-
Notifications
You must be signed in to change notification settings - Fork 741
Cleanup anr scripts #1714
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
Cleanup anr scripts #1714
Conversation
This reverts commit 76945e8.
…o e2e-anr-scripts
# | ||
# We use "export" here instead of just setting a bash variable because we need | ||
# to pass this flag to all child processes spawned by the shell. | ||
export CGO_CFLAGS="-O -D__BLST_PORTABLE__" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
exit 255 | ||
fi | ||
|
||
# Set the CGO flags to use the portable version of BLST |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we removing this? Don't we need it to ensure this works locally on MacOS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
install_anr.sh
includes: source "$AVALANCHE_PATH"/scripts/constants.sh
which already includes:
# Set the CGO flags to use the portable version of BLST
#
# We use "export" here instead of just setting a bash variable because we need
# to pass this flag to all child processes spawned by the shell.
export CGO_CFLAGS="-O -D__BLST_PORTABLE__"
# While CGO_ENABLED doesn't need to be explicitly set, it produces a much more
# clear error due to the default value change in go1.20.
export CGO_ENABLED=1
PR #1714 factored ANR installation into a common script, but in the process removed the CGO configuration for BLST from tests.e2e.sh. The result was sporadic e2e job failure whenever the github worker vm was missing the instructions non-portable BLST required. Ensuring CGO is explicitly configured for portable BLST in tests.e2e.sh should ensure the e2e test binary can once again be built reliably.
PR #1714 factored ANR installation into a common script, but in the process removed the CGO configuration for BLST from tests.e2e.sh. The result was sporadic e2e job failure whenever the github worker vm was missing the instructions non-portable BLST required. Ensuring CGO is explicitly configured for portable BLST in tests.e2e.sh should ensure the e2e test binary can once again be built reliably.
PR #1714 factored ANR installation into a common script, but in the process removed the CGO configuration for BLST from tests.e2e.sh. The result was sporadic e2e job failure whenever the github worker vm was missing the instructions non-portable BLST required. Ensuring CGO is explicitly configured for portable BLST in tests.e2e.sh should ensure the e2e test binary can once again be built reliably.
PR #1714 factored ANR installation into a common script, but in the process removed the CGO configuration for BLST from tests.e2e.sh. The result was sporadic e2e job failure whenever the github worker vm was missing the instructions non-portable BLST required. Ensuring CGO is explicitly configured for portable BLST in tests.e2e.sh should ensure the e2e test binary can once again be built reliably.
Why this should be merged
We currently have a duplicated code to download & install ANR.
How this works
Creates a new script to install ANR and uses it in test scripts.
How this was tested
Tests should pass.