From edd1c43f8b4cdf0c8d758287fabdfdab00f4d082 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 27 Jul 2024 13:54:55 -0400 Subject: [PATCH 01/16] Use bash for everything on all platforms in release workflow The way environment variables are being set and accessed assumes a Bourne-style (POSIX-like) shell, but PowerShell was being used on Windows for some steps where this was being assumed. It's possible to set bash only for some steps, but that is more complex to understand than using it for everything. --- .github/workflows/release.yml | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 06e73353cfd..d2429e04cc5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,11 +12,6 @@ name: release -env: - RUST_BACKTRACE: 1 - CARGO_TERM_COLOR: always - CLICOLOR: 1 - on: workflow_dispatch: push: @@ -25,6 +20,16 @@ on: # - fix-releases tags: - "v.*" + +env: + RUST_BACKTRACE: 1 + CARGO_TERM_COLOR: always + CLICOLOR: 1 + +defaults: + run: + shell: bash + jobs: create-release: name: create-release @@ -167,7 +172,6 @@ jobs: path: artifacts - name: Set release upload URL and release version - shell: bash run: | release_upload_url="$(cat artifacts/release-upload-url)" echo "RELEASE_UPLOAD_URL=$release_upload_url" >> $GITHUB_ENV @@ -193,7 +197,6 @@ jobs: /target/arm-unknown-linux-gnueabihf/release/${{ env.EXE_NAME }} \ /target/arm-unknown-linux-gnueabihf/release/gix - name: Build archive - shell: bash run: | staging="gitoxide-${{ matrix.feature }}-${{ env.RELEASE_VERSION }}-${{ matrix.target }}" mkdir -p "$staging" From 3e779d988d82d2b2b8aed20f9f302c1218a857bf Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 27 Jul 2024 13:58:02 -0400 Subject: [PATCH 02/16] Remove some commented-out code in the workflow --- .github/workflows/release.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d2429e04cc5..54e893e93e4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -152,7 +152,6 @@ jobs: targets: ${{ matrix.target }} - name: Use Cross - # if: matrix.os != 'windows-latest' run: | cargo install cross echo "CARGO=cross" >> $GITHUB_ENV From d7e9269a61c1cbf2083baa039cde82b4feef577f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 27 Jul 2024 14:00:53 -0400 Subject: [PATCH 03/16] Remove confusing EXE_NAME environment variable This removes the `EXE_NAME` variable in the release workflow, replacing all uses of it with an literal `ein`, which was always its value. The rationale is that this variable was confusing in two ways: - The release workflow builds and publishes two executables, for both the ein and gix commands. The executable for the ein command was specified through this environment variable, while the executable for the gix command was already given literally. Besides improving consistency, removing the environment variable makes clear what is actually being specified. - The name of the executable that provides the ein command is `ein` on most operating systems but `ein.exe` on Windows. But the `EXE_NAME` variable always held the value `ein` on all systems, with a `.exe` suffix concatenated to it on Windows. This was unintuitive because `EXE_NAME` didn't always name an executable. --- .github/workflows/release.yml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 54e893e93e4..5c0d78ab597 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -83,8 +83,6 @@ jobs: TARGET_DIR: ./target # Emit backtraces on panics. RUST_BACKTRACE: 1 - # The name of the executable to expect - EXE_NAME: ein strategy: matrix: build: [ linux, linux-arm, macos, win-msvc, win-gnu, win32-msvc ] @@ -184,7 +182,7 @@ jobs: - name: Strip release binary (linux and macos) if: matrix.build == 'linux' || matrix.build == 'macos' - run: strip target/${{ matrix.target }}/release/${{ env.EXE_NAME }} target/${{ matrix.target }}/release/gix + run: strip target/${{ matrix.target }}/release/ein target/${{ matrix.target }}/release/gix - name: Strip release binary (arm) if: matrix.build == 'linux-arm' @@ -193,7 +191,7 @@ jobs: "$PWD/target:/target:Z" \ rustembedded/cross:arm-unknown-linux-gnueabihf \ arm-linux-gnueabihf-strip \ - /target/arm-unknown-linux-gnueabihf/release/${{ env.EXE_NAME }} \ + /target/arm-unknown-linux-gnueabihf/release/ein \ /target/arm-unknown-linux-gnueabihf/release/gix - name: Build archive run: | @@ -203,11 +201,11 @@ jobs: cp {README.md,LICENSE-*,CHANGELOG.md} "$staging/" if [ "${{ matrix.os }}" = "windows-latest" ]; then - cp target/release/${{ env.EXE_NAME }}.exe target/release/gix.exe "$staging/" + cp target/release/ein.exe target/release/gix.exe "$staging/" 7z a "$staging.zip" "$staging" echo "ASSET=$staging.zip" >> $GITHUB_ENV else - cp target/${{ matrix.target }}/release/${{ env.EXE_NAME }} target/${{ matrix.target }}/release/gix "$staging/" + cp target/${{ matrix.target }}/release/ein target/${{ matrix.target }}/release/gix "$staging/" tar czf "$staging.tar.gz" "$staging" echo "ASSET=$staging.tar.gz" >> $GITHUB_ENV fi From 564bdc97252d38de8004e2ae35499560ea6e1340 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 27 Jul 2024 14:08:57 -0400 Subject: [PATCH 04/16] Copy from target-specific build directories even on Windows Changing the shell to bash *may* have been sufficient to cause these directories to be used by successfully allowing environment variables to be set and accessed using POSIX shell syntax. If so, then this change together with that one *may* be enough either to make it so that the 32-bit release archives really get 32-bit executables on Windows or to reveal further reasons they do not. --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5c0d78ab597..d508b3ee359 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -201,7 +201,7 @@ jobs: cp {README.md,LICENSE-*,CHANGELOG.md} "$staging/" if [ "${{ matrix.os }}" = "windows-latest" ]; then - cp target/release/ein.exe target/release/gix.exe "$staging/" + cp target/${{ matrix.target }}/release/ein.exe target/${{ matrix.target }}/release/gix.exe "$staging/" 7z a "$staging.zip" "$staging" echo "ASSET=$staging.zip" >> $GITHUB_ENV else From 418d71ffc7253686a320542f31f9d73394fa87e6 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 27 Jul 2024 14:12:43 -0400 Subject: [PATCH 05/16] Reveal what kind of file the executables are when making archives --- .github/workflows/release.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d508b3ee359..48c323e1f0f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -201,10 +201,12 @@ jobs: cp {README.md,LICENSE-*,CHANGELOG.md} "$staging/" if [ "${{ matrix.os }}" = "windows-latest" ]; then + file target/${{ matrix.target }}/release/ein.exe target/${{ matrix.target }}/release/gix.exe cp target/${{ matrix.target }}/release/ein.exe target/${{ matrix.target }}/release/gix.exe "$staging/" 7z a "$staging.zip" "$staging" echo "ASSET=$staging.zip" >> $GITHUB_ENV else + file target/${{ matrix.target }}/release/ein target/${{ matrix.target }}/release/gix cp target/${{ matrix.target }}/release/ein target/${{ matrix.target }}/release/gix "$staging/" tar czf "$staging.tar.gz" "$staging" echo "ASSET=$staging.tar.gz" >> $GITHUB_ENV From 286e388700ce3967603403cbe8cffd8429a24c8c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 18:31:08 -0400 Subject: [PATCH 06/16] Minor changes from ripgrep workflow + simplify + clarify quoting The release.yml workflow adapts from the corresponding workflow in the ripgrep project, but drawing from multiple revisions of it and also including substantial gitoxide-specific changes. ripgrep's licensing permits this even with respect to any material that is original to that project, since one of the licenses ripgrep is offered under is the Unlicense, which allows arbitrary use and adaptation without requiring even attribution. This commit makes these changes related to that relationship: - Note the relationship, linking to the ripgrep workflow. - Bring in the new ripgrep comment explaining the workflow, which, as there, is shorter and just above the `create-release` job, not at the top. This makes a slight change, so as not to refer to a version tag scheme that differs and that we do not enforce. - Renames both *_VERSION variables to just `VERSION`, as they now are in the ripgrep workflow. These variables have conceptually the same meaning, always take the same value, and do not clash because each job in the workflow has exactly one of them. - Bring in the simplification of using `github.ref_name` to get the tag name to use as the version string. There remain other significant changes to that workflow that can and should be applied or adapted into this one, but which are not included in this commit. This commit also makes some other changes for simplification and stylistic clarity: - Change the trigger tag pattern to the glob-like `v*`, which seems to be what the GHA docs say will make it work (though this may have been tried unsuccessfully before). - Don't specify the fetch depth, because `actions-checkout` defaults to a depth of 1 already. - Remove ad-hoc echos that inadvertently performed empty parameter expansion because they used a variable that had just been written in `$GITHUB_ENV`. Writes to `$GITHUB_ENV` only affect the environment of subsequent steps, not of subsequent commands run as part of the same script step. These could have been shown in a different way, or by expanding them straightforwardly but in a subsequent script step (like the existing "Show command used for Cargo" step), but the removed echos did not seem very important and the information they were showing is mostly available by detailed log inspection. - Simplify some commands whose complexity was just to support those ineffective echos. - Use the bash `$(< path)` syntax where applicable. This is a more efficient alternative to `$(cat path)`. It's a bash-ism, but bash is now being used to run all steps on all platforms. - Where both (a) quoting was clearly unnecessary because of the way YAML parses strings and (b) quoting was being used in some places and not analogous other places, so the style was inconsistent, this removes the quoting. (This does not remove quoting in other situations, not even when one but not the other condition holds.) - When quoting inline, this always uses the strongest form, of those that are readily available and easily readable. This applies both to the formation of YAML strings, which now prefer single quotes to double quotes, and to quotation marks that appear inside YAML strings that are executed by the shell. Now double quotes are only used when both (a) quoting for a shell rather than the YAML parser and (b) deliberately causing the shell to perform its own `$...` expansions. This is because double-quoted `${{ }}` interpolation can falsely appear to be a shell expansion, when really all `${{ }}` interpolation is done after YAML parsing but before the shell receives the code that it runs in a script step. - Double-quote `$GITHUB_ENV` (rather than not quoting it), so all shell parameter expansion is double quoted except where expansion that quoting would suppress is wanted (currently, nowhere). - When shell quoting is optional because the item being `${{ }}` interpolated is guaranteed to be parsed by the shell as a single argument, and where we *rely* on that effect, this quotes the argument. This is done more for consistency, in view of how it was already mostly being done, than robustness. It is still not truly robust because, although we never set such values, a `'` character, if present in the output of interpolation, would be (like all output of `${{ }}` interpolation) present literally in the script that the shell receives, and the shell would interpret it as a closing quotation mark. - Adjust spacing for consistency with the existing style. --- .github/workflows/release.yml | 86 +++++++++++++++-------------------- 1 file changed, 37 insertions(+), 49 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 48c323e1f0f..56bc2cced92 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,14 +1,5 @@ -# The way this works is a little weird. But basically, the create-release job -# runs purely to initialize the GitHub release itself. Once done, the upload -# URL of the release is saved as an artifact. -# -# The build-release job runs only once create-release is finished. It gets -# the release upload URL by downloading the corresponding artifact (which was -# uploaded by create-release). It then builds the release executables for each -# supported platform and attaches them as release assets to the previously -# created release. -# -# The key here is that we create the release only once. +# This is largely adapted from past and recent versions of the ripgrep release workflow. +# https://github.com/BurntSushi/ripgrep/blob/master/.github/workflows/release.yml name: release @@ -19,7 +10,7 @@ on: # branches: # - fix-releases tags: - - "v.*" + - 'v*' env: RUST_BACKTRACE: 1 @@ -31,38 +22,39 @@ defaults: shell: bash jobs: + # The create-release job runs purely to initialize the GitHub release itself, + # and names the release after the version tag that was pushed. It's separate + # from building the release so that we only create the release once. create-release: name: create-release runs-on: ubuntu-latest # env: # # Set to force version number, e.g., when no tag exists. -# ARTIFACT_VERSION: TEST-0.0.0 +# VERSION: TEST-0.0.0 steps: - name: Create artifacts directory run: mkdir artifacts - name: Get the release version from the tag - if: env.ARTIFACT_VERSION == '' - run: | - echo "ARTIFACT_VERSION=${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV - echo "version is: ${{ env.ARTIFACT_VERSION }}" + if: env.VERSION == '' + run: echo 'VERSION=${{ github.ref_name }}' >> "$GITHUB_ENV" - name: Create GitHub release id: release uses: ncipollo/release-action@v1 with: - tag: ${{ env.ARTIFACT_VERSION }} - name: ${{ env.ARTIFACT_VERSION }} + tag: ${{ env.VERSION }} + name: ${{ env.VERSION }} allowUpdates: true omitBody: true omitPrereleaseDuringUpdate: true token: ${{ secrets.GITHUB_TOKEN }} - name: Save release upload URL to artifact - run: echo "${{ steps.release.outputs.upload_url }}" > artifacts/release-upload-url + run: echo '${{ steps.release.outputs.upload_url }}' > artifacts/release-upload-url - name: Save version number to artifact - run: echo "${{ env.ARTIFACT_VERSION }}" > artifacts/release-version + run: echo '${{ env.VERSION }}' > artifacts/release-version - name: Upload artifacts uses: actions/upload-artifact@v4 @@ -72,13 +64,13 @@ jobs: build-release: name: build-release - needs: [ "create-release" ] + needs: [ create-release ] env: # For some builds, we use cross to test on 32-bit and big-endian # systems. CARGO: cargo # When CARGO is set to CROSS, this is set to `--target matrix.target`. - TARGET_FLAGS: "" + TARGET_FLAGS: '' # When CARGO is set to CROSS, TARGET_DIR includes matrix.target. TARGET_DIR: ./target # Emit backtraces on panics. @@ -86,7 +78,7 @@ jobs: strategy: matrix: build: [ linux, linux-arm, macos, win-msvc, win-gnu, win32-msvc ] - feature: [ "small", "lean", "max", "max-pure" ] + feature: [ small, lean, max, max-pure ] include: - build: linux os: ubuntu-latest @@ -130,11 +122,10 @@ jobs: feature: max runs-on: ${{ matrix.os }} + steps: - name: Checkout repository uses: actions/checkout@v4 - with: - fetch-depth: 1 - name: Install packages (Ubuntu) # Because openssl doesn't work on musl by default, we resort to max-pure. And that won't need any dependency, so we can skip this.continue-on-error @@ -152,15 +143,15 @@ jobs: - name: Use Cross run: | cargo install cross - echo "CARGO=cross" >> $GITHUB_ENV - echo "TARGET_FLAGS=--target ${{ matrix.target }}" >> $GITHUB_ENV - echo "TARGET_DIR=./target/${{ matrix.target }}" >> $GITHUB_ENV + echo 'CARGO=cross' >> "$GITHUB_ENV" + echo 'TARGET_FLAGS=--target ${{ matrix.target }}' >> "$GITHUB_ENV" + echo 'TARGET_DIR=./target/${{ matrix.target }}' >> "$GITHUB_ENV" - name: Show command used for Cargo run: | - echo "cargo command is: ${{ env.CARGO }}" - echo "target flag is: ${{ env.TARGET_FLAGS }}" - echo "target dir is: ${{ env.TARGET_DIR }}" + echo 'cargo command is: ${{ env.CARGO }}' + echo 'target flag is: ${{ env.TARGET_FLAGS }}' + echo 'target dir is: ${{ env.TARGET_DIR }}' - name: Get release download URL uses: actions/download-artifact@v4 @@ -170,19 +161,15 @@ jobs: - name: Set release upload URL and release version run: | - release_upload_url="$(cat artifacts/release-upload-url)" - echo "RELEASE_UPLOAD_URL=$release_upload_url" >> $GITHUB_ENV - echo "release upload url: $RELEASE_UPLOAD_URL" - release_version="$(cat artifacts/release-version)" - echo "RELEASE_VERSION=$release_version" >> $GITHUB_ENV - echo "release version: $RELEASE_VERSION" + echo "UPLOAD_URL=$(< artifacts/release-upload-url)" >> "$GITHUB_ENV" + echo "VERSION=$(< artifacts/release-version)" >> "$GITHUB_ENV" - name: Build release binary run: ${{ env.CARGO }} build --verbose --release ${{ env.TARGET_FLAGS }} --no-default-features --features ${{ matrix.feature }} - name: Strip release binary (linux and macos) if: matrix.build == 'linux' || matrix.build == 'macos' - run: strip target/${{ matrix.target }}/release/ein target/${{ matrix.target }}/release/gix + run: strip 'target/${{ matrix.target }}/release/ein' 'target/${{ matrix.target }}/release/gix' - name: Strip release binary (arm) if: matrix.build == 'linux-arm' @@ -193,23 +180,24 @@ jobs: arm-linux-gnueabihf-strip \ /target/arm-unknown-linux-gnueabihf/release/ein \ /target/arm-unknown-linux-gnueabihf/release/gix + - name: Build archive run: | - staging="gitoxide-${{ matrix.feature }}-${{ env.RELEASE_VERSION }}-${{ matrix.target }}" - mkdir -p "$staging" + staging='gitoxide-${{ matrix.feature }}-${{ env.VERSION }}-${{ matrix.target }}' + mkdir -p -- "$staging" cp {README.md,LICENSE-*,CHANGELOG.md} "$staging/" - if [ "${{ matrix.os }}" = "windows-latest" ]; then - file target/${{ matrix.target }}/release/ein.exe target/${{ matrix.target }}/release/gix.exe - cp target/${{ matrix.target }}/release/ein.exe target/${{ matrix.target }}/release/gix.exe "$staging/" + if [ '${{ matrix.os }}' = 'windows-latest' ]; then + file 'target/${{ matrix.target }}/release/ein.exe' 'target/${{ matrix.target }}/release/gix.exe' + cp 'target/${{ matrix.target }}/release/ein.exe' 'target/${{ matrix.target }}/release/gix.exe' "$staging/" 7z a "$staging.zip" "$staging" - echo "ASSET=$staging.zip" >> $GITHUB_ENV + echo "ASSET=$staging.zip" >> "$GITHUB_ENV" else - file target/${{ matrix.target }}/release/ein target/${{ matrix.target }}/release/gix - cp target/${{ matrix.target }}/release/ein target/${{ matrix.target }}/release/gix "$staging/" + file 'target/${{ matrix.target }}/release/ein' 'target/${{ matrix.target }}/release/gix' + cp 'target/${{ matrix.target }}/release/ein' 'target/${{ matrix.target }}/release/gix' "$staging/" tar czf "$staging.tar.gz" "$staging" - echo "ASSET=$staging.tar.gz" >> $GITHUB_ENV + echo "ASSET=$staging.tar.gz" >> "$GITHUB_ENV" fi - name: Upload release archive @@ -217,7 +205,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: - upload_url: ${{ env.RELEASE_UPLOAD_URL }} + upload_url: ${{ env.UPLOAD_URL }} asset_path: ${{ env.ASSET }} asset_name: ${{ env.ASSET }} asset_content_type: application/octet-stream From a4b5cb93791c391c7db9c1337a085c0db772338e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 18:40:57 -0400 Subject: [PATCH 07/16] Only use `cross` on Linux This adds the `if:` key from the ripgrep workflow to the "Use Cross" step, so that we only use `cross` instead of `cargo` when both of the following hold: (a) We are on a Linux runner. (b) It is possible that we are cross-compiling. Of those conditions, (a) is more important than (b), since `cross` can be used even when not cross-compiling. However, on Windows, it fails to create the environment, because there is no Docker image available for it, and also it is a Windows environment so Windows containers would have to be used, which are not available. This would also not currently be able to work on Windows hosted GHA runners, because they don't support nested virtualization. Although using `cross` on Windows in this workflow had never worked properly, there were two different incorrect behaviors: - It was previously not actually being used at all, because PowerShell rather than bash was being used as the shell, and the attempt to use syntax like `>> $GITHUB_ENV` in PowerShell did not succeed at setting the `CARGO` environment variable (nor others). Although this is *conceptually* related to the bug where the 32-bit (i686) Windows release archives actually contained 64-bit binaries (#1472), it does not seem to have been the direct cause, since cross-compiling with `cargo` should have worked. But the related failure to set the `$TARGET_FLAGS` and `$TARGET_DIR` environment variables was an important factor. This was one factor responsible for the bug where the 32-bit - With bash used for all script steps on all operating systems, it failed early with an error from `docker` about not finding an image on ghcr.io corresponding to the target. For some further details, see: https://github.com/Byron/gitoxide/issues/1472#issuecomment-2254244666 --- .github/workflows/release.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 56bc2cced92..d9a0f6076ae 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -141,6 +141,7 @@ jobs: targets: ${{ matrix.target }} - name: Use Cross + if: matrix.os == 'ubuntu-latest' && matrix.target != '' run: | cargo install cross echo 'CARGO=cross' >> "$GITHUB_ENV" From f6a71f04df4f90a786fb0e2074f97d053cc6fdc7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 19:04:16 -0400 Subject: [PATCH 08/16] Set $TARGET_FLAGS and $TARGET_DIR even without `cross` This adds that step from the revised ripgrep workflow, moving the commands into it. The new step is the same as in ripgrep except: - Without `shell: bash`, because we set that for the whole job and do so on all operating systems. - With a different quoting convention. The related change of taking the executables from the specific target directory has already been made. This is part of what is needed to make that actually work. --- .github/workflows/release.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d9a0f6076ae..7528f3caf28 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -145,6 +145,9 @@ jobs: run: | cargo install cross echo 'CARGO=cross' >> "$GITHUB_ENV" + + - name: Set target variables + run: | echo 'TARGET_FLAGS=--target ${{ matrix.target }}' >> "$GITHUB_ENV" echo 'TARGET_DIR=./target/${{ matrix.target }}' >> "$GITHUB_ENV" From cf379f65596375582da79c01627149d6d30cfe3a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 20:57:20 -0400 Subject: [PATCH 09/16] Update and clarify comments on job-level env.TARGET_* --- .github/workflows/release.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7528f3caf28..6ec33333132 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -69,9 +69,9 @@ jobs: # For some builds, we use cross to test on 32-bit and big-endian # systems. CARGO: cargo - # When CARGO is set to CROSS, this is set to `--target matrix.target`. + # Whether CARGO is `cargo` or `cross`, TARGET_FLAGS will be set to `--target matrix.target`. TARGET_FLAGS: '' - # When CARGO is set to CROSS, TARGET_DIR includes matrix.target. + # Whether CARGO is `cargo` or `cross`, TARGET_DIR will include matrix.target. TARGET_DIR: ./target # Emit backtraces on panics. RUST_BACKTRACE: 1 From cb10aaef2ed53191fbfb1ab5ea8f18e38364e39e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 21:47:49 -0400 Subject: [PATCH 10/16] Don't check matrix.target for cross + do related prep All our jobs have matrix.target set, so that check isn't needed. The intent was to avoid using `cross` when not cross-compiling, but treating an empty value to mean native compilation wouldn't have had any effect. This also does some preparation for related forthcoming changes, moving the job-level `env` below `strategy` (so it remains easy to understand even once it gains matrix-influenced values) and fixing up some wording to reflect that we don't currently build for any bigendian architectures such as s390x. --- .github/workflows/release.yml | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6ec33333132..1a75c88771a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -64,17 +64,9 @@ jobs: build-release: name: build-release + needs: [ create-release ] - env: - # For some builds, we use cross to test on 32-bit and big-endian - # systems. - CARGO: cargo - # Whether CARGO is `cargo` or `cross`, TARGET_FLAGS will be set to `--target matrix.target`. - TARGET_FLAGS: '' - # Whether CARGO is `cargo` or `cross`, TARGET_DIR will include matrix.target. - TARGET_DIR: ./target - # Emit backtraces on panics. - RUST_BACKTRACE: 1 + strategy: matrix: build: [ linux, linux-arm, macos, win-msvc, win-gnu, win32-msvc ] @@ -123,6 +115,16 @@ jobs: runs-on: ${{ matrix.os }} + env: + # For some builds, we use cross to test on 32-bit, and maybe later big-endian, systems. + CARGO: cargo + # Whether CARGO is `cargo` or `cross`, TARGET_FLAGS will be set to `--target matrix.target`. + TARGET_FLAGS: '' + # Whether CARGO is `cargo` or `cross`, TARGET_DIR will include matrix.target. + TARGET_DIR: ./target + # Emit backtraces on panics. + RUST_BACKTRACE: 1 + steps: - name: Checkout repository uses: actions/checkout@v4 @@ -141,7 +143,7 @@ jobs: targets: ${{ matrix.target }} - name: Use Cross - if: matrix.os == 'ubuntu-latest' && matrix.target != '' + if: matrix.os == 'ubuntu-latest' run: | cargo install cross echo 'CARGO=cross' >> "$GITHUB_ENV" From 8f43a921be3745a97a5b13a23f16745a4f866441 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 23:10:39 -0400 Subject: [PATCH 11/16] Set $TARGET_FLAGS and $TARGET_DIR from matrix Since they are produced via `${{ }}` interpolation using values from the matrix. Matrix values cannot be formed from job-level `env`, but job-level `env` can be formed form matrix values, as this does. --- .github/workflows/release.yml | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1a75c88771a..152596645f4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -119,9 +119,9 @@ jobs: # For some builds, we use cross to test on 32-bit, and maybe later big-endian, systems. CARGO: cargo # Whether CARGO is `cargo` or `cross`, TARGET_FLAGS will be set to `--target matrix.target`. - TARGET_FLAGS: '' + TARGET_FLAGS: --target=${{ matrix.target }} # Whether CARGO is `cargo` or `cross`, TARGET_DIR will include matrix.target. - TARGET_DIR: ./target + TARGET_DIR: ./target/${{ matrix.target }} # Emit backtraces on panics. RUST_BACKTRACE: 1 @@ -148,11 +148,6 @@ jobs: cargo install cross echo 'CARGO=cross' >> "$GITHUB_ENV" - - name: Set target variables - run: | - echo 'TARGET_FLAGS=--target ${{ matrix.target }}' >> "$GITHUB_ENV" - echo 'TARGET_DIR=./target/${{ matrix.target }}' >> "$GITHUB_ENV" - - name: Show command used for Cargo run: | echo 'cargo command is: ${{ env.CARGO }}' From fbafff3e9c93c313ac08846189b66828ba7693cf Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 23:22:58 -0400 Subject: [PATCH 12/16] Really use `cross` only when cross-compiling on Linux --- .github/workflows/release.yml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 152596645f4..d4ca08dee23 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -75,26 +75,32 @@ jobs: - build: linux os: ubuntu-latest rust: stable + cargo: cargo target: x86_64-unknown-linux-musl - build: linux-arm os: ubuntu-latest rust: nightly + cargo: cross target: arm-unknown-linux-gnueabihf - build: macos os: macos-latest rust: stable + cargo: cargo target: x86_64-apple-darwin - build: win-msvc os: windows-latest rust: nightly + cargo: cargo target: x86_64-pc-windows-msvc - build: win-gnu os: windows-latest rust: nightly-x86_64-gnu + cargo: cargo target: x86_64-pc-windows-gnu - build: win32-msvc os: windows-latest rust: nightly + cargo: cargo target: i686-pc-windows-msvc # on linux we build with musl which causes trouble with open-ssl. For now, just build max-pure there # even though we could also build with `--features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/fast-sha1` for fast hashing. @@ -116,14 +122,10 @@ jobs: runs-on: ${{ matrix.os }} env: - # For some builds, we use cross to test on 32-bit, and maybe later big-endian, systems. - CARGO: cargo - # Whether CARGO is `cargo` or `cross`, TARGET_FLAGS will be set to `--target matrix.target`. + CARGO: ${{ matrix.cargo }} TARGET_FLAGS: --target=${{ matrix.target }} - # Whether CARGO is `cargo` or `cross`, TARGET_DIR will include matrix.target. TARGET_DIR: ./target/${{ matrix.target }} - # Emit backtraces on panics. - RUST_BACKTRACE: 1 + RUST_BACKTRACE: 1 # Emit backtraces on panics. steps: - name: Checkout repository @@ -142,11 +144,9 @@ jobs: toolchain: ${{ matrix.rust }} targets: ${{ matrix.target }} - - name: Use Cross - if: matrix.os == 'ubuntu-latest' - run: | - cargo install cross - echo 'CARGO=cross' >> "$GITHUB_ENV" + - name: Install Cross + if: matrix.cargo == 'cross' + run: cargo install cross - name: Show command used for Cargo run: | From 79f9ec0d46c6dbfbc9c5a60eb87f2b96a2c5990b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 23:27:46 -0400 Subject: [PATCH 13/16] Don't set CARGO=cross too early Because `cargo` can also use this environment variable. --- .github/workflows/release.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d4ca08dee23..ef45f2dbbca 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -122,7 +122,7 @@ jobs: runs-on: ${{ matrix.os }} env: - CARGO: ${{ matrix.cargo }} + CARGO: cargo # Sometimes changes to `cross` later (such as when building linux-arm). TARGET_FLAGS: --target=${{ matrix.target }} TARGET_DIR: ./target/${{ matrix.target }} RUST_BACKTRACE: 1 # Emit backtraces on panics. @@ -144,9 +144,11 @@ jobs: toolchain: ${{ matrix.rust }} targets: ${{ matrix.target }} - - name: Install Cross + - name: Use Cross if: matrix.cargo == 'cross' - run: cargo install cross + run: | + cargo install cross + echo 'CARGO=cross' >> "$GITHUB_ENV" - name: Show command used for Cargo run: | From d6769a6b6d3dd51a6e8c50fdb48750fac4af2071 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 28 Jul 2024 23:39:55 -0400 Subject: [PATCH 14/16] Use shell expansions for env vars where clearer This uses parameter expansion, and sometimes also brace expansion, to make some commands that use environment variables shorter and easier to read. I did not apply brace expansion in the strip command run in docker, since the shell running that may be sh rather than bash. --- .github/workflows/release.yml | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ef45f2dbbca..d070c31f013 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -54,7 +54,7 @@ jobs: run: echo '${{ steps.release.outputs.upload_url }}' > artifacts/release-upload-url - name: Save version number to artifact - run: echo '${{ env.VERSION }}' > artifacts/release-version + run: echo "$VERSION" > artifacts/release-version - name: Upload artifacts uses: actions/upload-artifact@v4 @@ -152,9 +152,9 @@ jobs: - name: Show command used for Cargo run: | - echo 'cargo command is: ${{ env.CARGO }}' - echo 'target flag is: ${{ env.TARGET_FLAGS }}' - echo 'target dir is: ${{ env.TARGET_DIR }}' + echo "cargo command is: $CARGO" + echo "target flag is: $TARGET_FLAGS" + echo "target dir is: $TARGET_DIR" - name: Get release download URL uses: actions/download-artifact@v4 @@ -168,11 +168,12 @@ jobs: echo "VERSION=$(< artifacts/release-version)" >> "$GITHUB_ENV" - name: Build release binary - run: ${{ env.CARGO }} build --verbose --release ${{ env.TARGET_FLAGS }} --no-default-features --features ${{ matrix.feature }} + run: | + "$CARGO" build --verbose --release "$TARGET_FLAGS" --no-default-features --features ${{ matrix.feature }} - name: Strip release binary (linux and macos) if: matrix.build == 'linux' || matrix.build == 'macos' - run: strip 'target/${{ matrix.target }}/release/ein' 'target/${{ matrix.target }}/release/gix' + run: strip -- "$TARGET_DIR"/release/{ein,gix} - name: Strip release binary (arm) if: matrix.build == 'linux-arm' @@ -192,13 +193,13 @@ jobs: cp {README.md,LICENSE-*,CHANGELOG.md} "$staging/" if [ '${{ matrix.os }}' = 'windows-latest' ]; then - file 'target/${{ matrix.target }}/release/ein.exe' 'target/${{ matrix.target }}/release/gix.exe' - cp 'target/${{ matrix.target }}/release/ein.exe' 'target/${{ matrix.target }}/release/gix.exe' "$staging/" + file -- "$TARGET_DIR"/release/{ein,gix}.exe + cp -- "$TARGET_DIR"/release/{ein,gix}.exe "$staging/" 7z a "$staging.zip" "$staging" echo "ASSET=$staging.zip" >> "$GITHUB_ENV" else - file 'target/${{ matrix.target }}/release/ein' 'target/${{ matrix.target }}/release/gix' - cp 'target/${{ matrix.target }}/release/ein' 'target/${{ matrix.target }}/release/gix' "$staging/" + file -- "$TARGET_DIR"/release/{ein,gix} + cp -- "$TARGET_DIR"/release/{ein,gix} "$staging/" tar czf "$staging.tar.gz" "$staging" echo "ASSET=$staging.tar.gz" >> "$GITHUB_ENV" fi From a64759870a46d8ac8346f8294c087fd55c1df6d4 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 29 Jul 2024 00:17:42 -0400 Subject: [PATCH 15/16] Don't use `--` argument to `strip` Unlike many utilities, the `strip` utility is not required by POSIX to follow the XBD 12.2 Utility Syntax Guidelines, so need not support `--`. On macOS, and possibly some other systems, it does not, and `--` causes an error. Fortunately, it's okay to just not pass it, because the value of `$TARGET_DIR` is known not to start with `-`. --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d070c31f013..69d3d4e5ca8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -173,7 +173,7 @@ jobs: - name: Strip release binary (linux and macos) if: matrix.build == 'linux' || matrix.build == 'macos' - run: strip -- "$TARGET_DIR"/release/{ein,gix} + run: strip "$TARGET_DIR"/release/{ein,gix} - name: Strip release binary (arm) if: matrix.build == 'linux-arm' From 3c3a8315554c329a06ece543c856308dfbce7eb4 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 29 Jul 2024 00:26:49 -0400 Subject: [PATCH 16/16] Go back to always using `cross` on Linux Because technically even the "non-cross" compilation is cross compilation, as it is targeting `x86_64-unknown-linux-musl`. When `cargo` rather than `cross` is used in that build, `ring` fails to compile C code due to the `musl-gcc` command not being present. Very likely other dependencies would also fail without more tools installed on the system, and future features we wish to support in binary releases would run into this in further ways. An alternative approach here could be installing `musl-gcc` in a prior step, much as we could also install an ARM gcc suite and probably eliminate `cross` for linux-arm. For now, though, I'm putting this back to using `cross` for all Linux builds. --- .github/workflows/release.yml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 69d3d4e5ca8..68f8b6ea7bb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -75,32 +75,26 @@ jobs: - build: linux os: ubuntu-latest rust: stable - cargo: cargo target: x86_64-unknown-linux-musl - build: linux-arm os: ubuntu-latest rust: nightly - cargo: cross target: arm-unknown-linux-gnueabihf - build: macos os: macos-latest rust: stable - cargo: cargo target: x86_64-apple-darwin - build: win-msvc os: windows-latest rust: nightly - cargo: cargo target: x86_64-pc-windows-msvc - build: win-gnu os: windows-latest rust: nightly-x86_64-gnu - cargo: cargo target: x86_64-pc-windows-gnu - build: win32-msvc os: windows-latest rust: nightly - cargo: cargo target: i686-pc-windows-msvc # on linux we build with musl which causes trouble with open-ssl. For now, just build max-pure there # even though we could also build with `--features max-control,http-client-reqwest,gitoxide-core-blocking-client,gix-features/fast-sha1` for fast hashing. @@ -145,7 +139,7 @@ jobs: targets: ${{ matrix.target }} - name: Use Cross - if: matrix.cargo == 'cross' + if: matrix.os == 'ubuntu-latest' run: | cargo install cross echo 'CARGO=cross' >> "$GITHUB_ENV"