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

Cleanup anr scripts #1714

Merged
merged 11 commits into from
Jul 18, 2023
Merged

Cleanup anr scripts #1714

merged 11 commits into from
Jul 18, 2023

Conversation

ceyonur
Copy link
Contributor

@ceyonur ceyonur commented Jul 14, 2023

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.

@ceyonur ceyonur added the cleanup Code quality improvement label Jul 14, 2023
@ceyonur ceyonur self-assigned this Jul 14, 2023
#
# 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__"
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@StephenButtolph StephenButtolph changed the title refactor anr scripts Cleanup anr scripts Jul 18, 2023
@StephenButtolph StephenButtolph added testing This primarily focuses on testing ci This focuses on changes to the CI process labels Jul 18, 2023
exit 255
fi

# Set the CGO flags to use the portable version of BLST
Copy link
Contributor

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?

Copy link
Contributor

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

@StephenButtolph StephenButtolph merged commit 5e1e571 into dev Jul 18, 2023
13 checks passed
@StephenButtolph StephenButtolph deleted the e2e-anr-scripts branch July 18, 2023 18:41
@StephenButtolph StephenButtolph added this to the v1.10.5 milestone Jul 18, 2023
marun added a commit that referenced this pull request Jul 19, 2023
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.
marun added a commit that referenced this pull request Jul 19, 2023
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.
marun added a commit that referenced this pull request Jul 19, 2023
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.
marun added a commit that referenced this pull request Jul 19, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci This focuses on changes to the CI process cleanup Code quality improvement testing This primarily focuses on testing
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants