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

Replace update_bazel_workspace.sh with a python script #1271

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Aug 23, 2023

This script is (IMO) more readable, but the real reason for this change is that it raises an error message when the binary package fails to download. (The shell script silently generated a bogus hash instead, because the shell's set -e builtin does not affect commands executing inside a $() context. It seemed just as easy to rewrite the script in Python as to fix that.)

This change also updates some outdated filename references.

This script is (IMO) more readable, but the real reason for this change is that
it raises an error message when the binary package fails to download. (The shell
script silently generated a bogus hash instead, because the shell's `set -e`
builtin does not affect commands executing inside a $() context.
It seemed just as easy to rewrite the script in Python as to fix that.

This change also updates some outdated filename references.
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me.

@dschuff dschuff merged commit 3391e03 into main Aug 24, 2023
@dschuff dschuff deleted the update_bazel_script branch August 24, 2023 17:10
shlomif pushed a commit to shlomif/emsdk that referenced this pull request Sep 29, 2023
…re#1271)

This script is (IMO) more readable, but the real reason for this change is that
it raises an error message when the binary package fails to download. (The shell
script silently generated a bogus hash instead, because the shell's `set -e`
builtin does not affect commands executing inside a $() context.
It seemed just as easy to rewrite the script in Python as to fix that.

This change also updates some outdated filename references.
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.

3 participants