Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not rely on Python2 to install Cloud9 dependencies #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

andrewnester
Copy link

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

install.sh Outdated
echo "Python version 2.7 is required to install pty.js. Please install python 2.7 and try again. You can find more information on how to install Python in the docs: https://docs.aws.amazon.com/cloud9/latest/user-guide/ssh-settings.html#ssh-settings-requirements"
if ! type -P "$PYTHON" &> /dev/null; then
echo "Python version is required to install pty.js. Please install python and try again. You can find more information on how to install Python in the docs: https://docs.c9.io/ssh_workspaces.html"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version reads a bit awkward here (2nd word). Can we remove it? Also, I think we should keep the original url.

Suggested change
echo "Python version is required to install pty.js. Please install python and try again. You can find more information on how to install Python in the docs: https://docs.c9.io/ssh_workspaces.html"
echo "Python is required to install pty.js. Please install python and try again. You can find more information on how to install Python in the docs: https://docs.aws.amazon.com/cloud9/latest/user-guide/ssh-settings.html#ssh-settings-requirements"

@@ -36,8 +38,7 @@ if [ ! -d "$C9_DIR" ]; then
fi

VERSION=1
NODE_VERSION=v6.3.1
NODE_VERSION_ARM_PI=v0.10.28
NODE_VERSION=v12.16.1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node 16 has known security issues—see https://nodejs.org/en/blog/release/v12.20.1/. Can we instead add 12.20.1 to our CDN and use that here?

Suggested change
NODE_VERSION=v12.16.1
NODE_VERSION=v12.20.1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm 12.20.1 has security issues too. Maybe it would be better to use 12.22.x. 12.22.9 is the version we current install at runtime.

Suggested change
NODE_VERSION=v12.16.1
NODE_VERSION=v12.22.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants