-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Correct handling of tagged prereleases #16246
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
Changes from all commits
b53554f
6c068b3
15964ca
5c21b7f
2b950bb
a3874f0
af138bf
8fe0df0
8439e2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -182,17 +182,12 @@ commands: | |
| # -------------------------------------------------------------------------- | ||
| # Build Commands | ||
|
|
||
| setup_prerelease_commit_hash: | ||
| setup_prerelease: | ||
| steps: | ||
| - run: | ||
| name: Store commit hash and prerelease | ||
| name: Store prerelease suffix | ||
| command: | | ||
| if [[ $CIRCLE_BRANCH == release || -n $CIRCLE_TAG ]]; then | ||
| echo -n > prerelease.txt; | ||
| else | ||
| date -u +"nightly.%Y.%-m.%-d" > prerelease.txt; | ||
| fi | ||
| echo -n "$CIRCLE_SHA1" > commit_hash.txt | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not really sure if there was a reason we were creating |
||
| "scripts/prerelease_suffix.sh" nightly "$CIRCLE_TAG" > prerelease.txt | ||
|
|
||
| install_and_check_minimum_requirements: | ||
| parameters: | ||
|
|
@@ -1006,6 +1001,7 @@ jobs: | |
| <<: *base_ubuntu2404_xlarge | ||
| steps: | ||
| - build | ||
| - run: cat prerelease.txt | ||
|
|
||
| # x64 ASAN build, for testing for memory related bugs | ||
| b_ubu_asan: &b_ubu_asan | ||
|
|
@@ -1115,7 +1111,7 @@ jobs: | |
| <<: *base_ubuntu_clang_large | ||
| steps: | ||
| - checkout | ||
| - setup_prerelease_commit_hash | ||
| - setup_prerelease | ||
| - run_build_ossfuzz | ||
| - persist_ossfuzz_executables_to_workspace | ||
| - matrix_notify_failure_unless_pr | ||
|
|
@@ -1225,7 +1221,7 @@ jobs: | |
| <<: *base_ubuntu2404_small | ||
| steps: | ||
| - checkout | ||
| - setup_prerelease_commit_hash | ||
| - setup_prerelease | ||
| - run: | ||
| name: Install build system dependencies | ||
| command: | | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,10 +126,12 @@ def get_github_username_repo(url): | |
| with open('../CMakeLists.txt', 'r', encoding='utf8') as f: | ||
| version = re.search('PROJECT_VERSION "([^"]+)"', f.read()).group(1) | ||
| # The full version, including alpha/beta/rc tags. | ||
| if not os.path.isfile('../prerelease.txt') or os.path.getsize('../prerelease.txt') == 0: | ||
| if os.path.isfile('../prerelease.txt') and os.path.getsize('../prerelease.txt') == 0: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prerelease_file = '../prerelease.txt' would have been nice There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I intentionally didn't want to introduce new variables here since I'm not sure if these don't become visible as settings. |
||
| release = version | ||
| elif os.path.isfile('../prerelease.txt'): | ||
| with open('../prerelease.txt', 'r', encoding='utf8') as prerelease_file: | ||
| release = version + '-' + prerelease_file.read() | ||
| else: | ||
| # This is a prerelease version | ||
| release = version + '-develop' | ||
|
|
||
| # The language for content autogenerated by Sphinx. Refer to documentation | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| (( $# <= 2 )) || { >&2 echo "Usage: $0 [PRERELEASE_SOURCE] [GIT_TAG]"; exit 1; } | ||
| prerelease_source="${1:-nightly}" | ||
| git_tag="${2:-}" | ||
| FORCE_RELEASE="${FORCE_RELEASE:-}" | ||
|
|
||
| GNU_DATE="date" | ||
| if [[ "$OSTYPE" == "darwin"* ]]; then | ||
| GNU_DATE=gdate | ||
| fi | ||
|
|
||
| if [[ $FORCE_RELEASE != "" || $git_tag =~ ^v[0-9.]+$ ]]; then | ||
| echo -n | ||
| elif [[ $git_tag =~ ^v[0-9.]+-pre. ]]; then | ||
| echo -n "pre.${git_tag#*-pre.}" | ||
| else | ||
| # Use last commit date rather than build date to avoid ending up with builds for | ||
| # different platforms having different version strings (and therefore producing different bytecode) | ||
| # if the CI is triggered just before midnight. | ||
| # NOTE: The -local suffix makes git not use the timezone from the commit but instead convert to | ||
| # local one, which we explicitly set to UTC. | ||
| # NOTE: git --date is supposed to support the %-m/%-d format too, but it does not seem to | ||
| # work on Windows. Without it we get leading zeros for month and day. | ||
| last_commit_date=$(TZ=UTC git show --quiet --date="format-local:%Y-%m-%d" --format="%cd") | ||
| last_commit_date_stripped=$("$GNU_DATE" --date "$last_commit_date" "+%Y.%-m.%-d") | ||
| echo -n "${prerelease_source}.${last_commit_date_stripped}" | ||
| fi |
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.
Note that tagged prereleases will skip most of the CI in solc-bin, because
compare_bytecode_reports.shfilters out anything that's not a release. I think it's fine, because they are supposed to have passed CI checks in thesolidityrepo and messing this up is not as big of a deal as with a full release. Still, one thing that would be useful to keep is the sanity check that the filename matches the output ofsolc --version. This check now sits invalidate_reported_version()inbytecode_reports_for_modified_binaries.sh. We should extract it into a separate PR check that runs on all binaries.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.
Sounds to me like it's almost easier to adapt the regex to allow for prereleases as well:)
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.
Well, that's an option too.