From 1f2e0f336a826bd578a49c1dd477fb38773dd6ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez?= <7822554+AlexTMjugador@users.noreply.github.com> Date: Sat, 2 Sep 2023 17:09:20 +0200 Subject: [PATCH] Revamp CI workflow to upload artifacts, cross-compile ARM64 binaries, and more (#534) As commented in issues https://github.com/shssoichiro/oxipng/issues/444 and https://github.com/shssoichiro/oxipng/issues/518, there is some user interest for distributing binaries for each unstable commit, and target ARM64 platforms. Personally, I think both suggestions are useful for the project, as uploading binary artifacts for each commit might help interested users to catch regressions and give feedback earlier, and powerful ARM64 platforms are becoming increasingly popular due to some cloud services (e.g., Amazon EC2, Azure VMs, Oracle Cloud) offering cheaper plans for this hardware, in addition to the well-known push for ARM by Apple with their custom M1 chips. These changes make the CI target ARM64 as a first-class citizen. Because the public GitHub actions runners can only be hosted on x64 for now, I resorted to cross-compilation, [Debian's multiarch](https://elinux.org/images/d/d8/Multiarch_and_Why_You_Should_Care-_Running%2C_Installing_and_Crossbuilding_With_Multiple_Architectures.pdf), and QEMU to build, get ARM64 C library dependencies, and run tests, respectively. When the CI workflow finishes, a release CLI binary artifact is now uploaded, which can be downloaded from the workflow run page on the GitHub web interface. In addition, these changes also introduce some cleanup and miscellaneous improvements and changes to the CI workflow: - Tests are run using [`nextest`](https://nexte.st/) instead of `cargo test`, which substantially speeds up their execution. (On my development workstation, `cargo test --release` takes around 10.67 s, while `cargo nextest run --release` takes around 6.02 s.) - The dependencies on unmaintained `actions-rs` actions were dropped in favor of running Cargo commands directly, or using `giraffate/clippy-action` for pretty inline annotations for Clippy. This gets rid of the deprecation warnings for each workflow run. - Most CI steps are run with a nightly Rust toolchain now, which allows to take advantage of the latest Clippy lints and codegen improvements. In my experience, when not relying on specific nightly features or compiler internals, Rust does a pretty good job at making it possible to rely on a rolling-release compiler for CI, as breakage is extremely rare and thus offset by the improved features. - The MSRV check was moved to a separate job with less steps, so that it takes less of a toll on total workflow run minutes. ## Pending tasks - [x] Generate universal macOS binaries with `lipo` (i.e., containing both `aarch64` and `x64` code) - [x] Tirelessly fix the stupid errors that tend to happen when deploying a new CI workflow for the first time - [x] Think what to do with the `deploy.yml` workflow. Should it fetch artifacts from the CI job instead of building them again? - [x] Maybe bring back 32-bit Windows binaries. Are they actually useful for somebody, or just a way to remember the good old days? --------- Co-authored-by: Josh Holmer --- .cargo/config.toml | 9 ++ .github/workflows/deploy.yml | 203 ++++++++++------------------------ .github/workflows/oxipng.yml | 208 ++++++++++++++++++++++++----------- Cargo.toml | 1 + src/sanity_checks.rs | 2 +- tests/flags.rs | 6 +- tests/raw.rs | 2 +- 7 files changed, 220 insertions(+), 211 deletions(-) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 00000000..82d1de25 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,9 @@ +# Remove this if targeting AArch64 from an AArch64 Linux box +[target.'cfg(all(target_os = "linux", target_arch = "aarch64"))'] +runner = 'qemu-aarch64' + +[target.aarch64-unknown-linux-gnu] +linker = 'aarch64-linux-gnu-gcc' + +[target.aarch64-unknown-linux-musl] +linker = 'aarch64-linux-musl-gcc' diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index ce1a9032..ea37f02a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -5,157 +5,76 @@ on: tags: - 'v*.*.*' +permissions: + actions: read + contents: write + jobs: - create-windows-binaries: - runs-on: windows-latest - if: github.repository == 'shssoichiro/oxipng' - - strategy: - matrix: - conf: [x86_64] -# Temporarily disable i686 binaries, they are failing on linking libdeflate -# and I don't have a Windows machine set up to experiment with fixing it. -# conf: [x86_64, i686] - - steps: - - uses: actions/checkout@v3 - with: - persist-credentials: false - - - name: Install stable - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - target: ${{ matrix.conf }}-pc-windows-msvc - override: true - - - name: Build oxipng - run: | - cargo build --release --target ${{ matrix.conf }}-pc-windows-msvc - - - name: Get the version - shell: bash - id: tagName - run: | - VERSION=$(cargo pkgid | cut -d# -f2 | cut -d: -f2) - echo "::set-output name=tag::$VERSION" - - - name: Build package - id: package - shell: bash - run: | - ARCHIVE_TARGET="${{ matrix.conf }}-pc-windows-msvc" - ARCHIVE_NAME="oxipng-${{ steps.tagName.outputs.tag }}-$ARCHIVE_TARGET" - ARCHIVE_FILE="${ARCHIVE_NAME}.zip" - mv LICENSE LICENSE.txt - 7z a ${ARCHIVE_FILE} \ - ./target/${{ matrix.conf }}-pc-windows-msvc/release/oxipng.exe \ - ./CHANGELOG.md ./LICENSE.txt ./README.md - echo "::set-output name=file::${ARCHIVE_FILE}" - echo "::set-output name=name::${ARCHIVE_NAME}.zip" - - - name: Upload artifacts - uses: actions/upload-artifact@v3 - with: - name: ${{ steps.package.outputs.name }} - path: ${{ steps.package.outputs.file }} - - create-unix-binaries: - runs-on: ${{ matrix.os }} - if: github.repository == 'shssoichiro/oxipng' - - strategy: - matrix: - os: [ubuntu-latest, macos-latest] - include: - - os: ubuntu-latest - target: x86_64-unknown-linux-musl - - os: macos-latest - target: x86_64-apple-darwin - - steps: - - uses: actions/checkout@v3 - with: - persist-credentials: false - - - name: Install Rust - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - target: ${{ matrix.target }} - override: true - - - name: Install musl - if: contains(matrix.target, 'linux-musl') - run: | - sudo apt-get install musl-tools - - - name: Build oxipng - run: | - cargo build --release --target ${{ matrix.target }} - - - name: Strip binary - run: | - strip target/${{ matrix.target }}/release/oxipng - - - name: Get the version - id: tagName - run: | - VERSION=$(cargo pkgid | cut -d# -f2 | cut -d: -f2) - echo "::set-output name=tag::$VERSION" - - - name: Build package - id: package - run: | - ARCHIVE_TARGET=${{ matrix.target }} - ARCHIVE_NAME="oxipng-${{ steps.tagName.outputs.tag }}-$ARCHIVE_TARGET" - ARCHIVE_FILE="${ARCHIVE_NAME}.tar.gz" - mkdir "/tmp/${ARCHIVE_NAME}" - cp README.md CHANGELOG.md LICENSE \ - target/${{ matrix.target }}/release/oxipng \ - /tmp/${ARCHIVE_NAME} - tar -czf ${PWD}/${ARCHIVE_FILE} -C /tmp/ ${ARCHIVE_NAME} - echo ::set-output "name=file::${ARCHIVE_FILE}" - echo ::set-output "name=name::${ARCHIVE_NAME}.tar.gz" - - - name: Upload artifacts - uses: actions/upload-artifact@v3 - with: - name: ${{ steps.package.outputs.name }} - path: ${{ steps.package.outputs.file }} - - deploy: + name: Deploy release + runs-on: ubuntu-latest - if: github.repository == 'shssoichiro/oxipng' - needs: [create-windows-binaries, create-unix-binaries] + timeout-minutes: 30 + + # Prevent job from running on forks + if: ${{ !github.event.repository.fork }} + + strategy: + matrix: + target: + - x86_64-unknown-linux-gnu + - x86_64-unknown-linux-musl + - aarch64-unknown-linux-gnu + - aarch64-unknown-linux-musl + - x86_64-pc-windows-msvc + - i686-pc-windows-msvc + - x86_64-apple-darwin + - aarch64-apple-darwin steps: - - uses: actions/checkout@v3 - with: - persist-credentials: false + - name: Checkout source + uses: actions/checkout@v3 - - name: Get version and release description - id: tagName + - name: Get the Oxipng version + id: oxipngMeta + run: echo "version=$(cargo metadata --format-version 1 --no-deps | jq -r '.packages[] | select(.name == "oxipng").version')" + >> "$GITHUB_OUTPUT" + + - name: Retrieve ${{ matrix.target }} binary + uses: dawidd6/action-download-artifact@v2 + with: + workflow: oxipng.yml + commit: ${{ env.GITHUB_SHA }} + name: Oxipng binary (${{ matrix.target }}) + path: target + + - name: Build archives + working-directory: target run: | - VERSION=$(cargo pkgid | cut -d# -f2 | cut -d: -f2) - tail -n +2 CHANGELOG.md | sed -e '/^$/,$d' > CHANGELOG.txt - echo "::set-output name=tag::$VERSION" + ARCHIVE_NAME="oxipng-${{ steps.oxipngMeta.outputs.version }}-${{ matrix.target }}" - - name: Download artifacts - uses: actions/download-artifact@v3 - with: - path: ./binaries + mkdir "$ARCHIVE_NAME" + cp ../CHANGELOG.md ../README.md "$ARCHIVE_NAME" - - name: Create a release + case '${{ matrix.target }}' in + *-windows-*) + cp ../LICENSE "$ARCHIVE_NAME/LICENSE.txt" + cp oxipng.exe "$ARCHIVE_NAME" + zip "${ARCHIVE_NAME}.zip" "$ARCHIVE_NAME"/*;; + *) + cp ../LICENSE "$ARCHIVE_NAME" + cp oxipng "$ARCHIVE_NAME" + tar -vczf "${ARCHIVE_NAME}.tar.gz" "$ARCHIVE_NAME"/*;; + esac + + - name: Create release notes + run: tail -n +2 CHANGELOG.md | sed -e '/^$/,$d' > RELEASE_NOTES.txt + + - name: Create release uses: softprops/action-gh-release@v1 with: - name: v${{ steps.tagName.outputs.tag }} - body_path: CHANGELOG.txt + name: v${{ steps.oxipngMeta.outputs.version }} + body_path: RELEASE_NOTES.txt files: | - ./binaries/**/*.zip - ./binaries/**/*.tar.gz - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + target/*.zip + target/*.tar.gz diff --git a/.github/workflows/oxipng.yml b/.github/workflows/oxipng.yml index a51fa5b8..cbe3cbbc 100644 --- a/.github/workflows/oxipng.yml +++ b/.github/workflows/oxipng.yml @@ -2,100 +2,180 @@ name: oxipng on: push: - branches: - - master pull_request: - branches: - - master + types: + - opened + - synchronize workflow_dispatch: jobs: - test: + ci: + name: CI + runs-on: ${{ matrix.os }} + timeout-minutes: 60 + + # Prevent tags and in-repo PRs from triggering this workflow more than once for a commit + if: github.ref_type != 'tag' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork) + strategy: fail-fast: false matrix: target: - x86_64-unknown-linux-gnu - x86_64-unknown-linux-musl + - aarch64-unknown-linux-gnu + - aarch64-unknown-linux-musl - x86_64-pc-windows-msvc + - i686-pc-windows-msvc - x86_64-apple-darwin - toolchain: - # Minimum stable - - "1.65.0" - - stable - - beta - - nightly + - aarch64-apple-darwin + include: - target: x86_64-unknown-linux-gnu - os: ubuntu-latest + os: ubuntu-22.04 + target-apt-arch: amd64 - target: x86_64-unknown-linux-musl - os: ubuntu-latest + os: ubuntu-22.04 + target-apt-arch: amd64 + - target: aarch64-unknown-linux-gnu + os: ubuntu-22.04 + target-apt-arch: arm64 + - target: aarch64-unknown-linux-musl + os: ubuntu-22.04 + target-apt-arch: arm64 - target: x86_64-pc-windows-msvc os: windows-latest + - target: i686-pc-windows-msvc + os: windows-latest - target: x86_64-apple-darwin - os: macOS-latest - exclude: - - target: x86_64-pc-windows-msvc - toolchain: beta - - target: x86_64-pc-windows-msvc - toolchain: nightly - - target: x86_64-apple-darwin - toolchain: beta - - target: x86_64-apple-darwin - toolchain: nightly - - target: x86_64-unknown-linux-musl - toolchain: beta - - target: x86_64-unknown-linux-musl - toolchain: nightly + os: macos-latest + - target: aarch64-apple-darwin + os: macos-latest + + env: + CARGO_BUILD_TARGET: ${{ matrix.target }} steps: - - uses: actions/checkout@v3 + - name: Checkout source + uses: actions/checkout@v3 with: persist-credentials: false - - name: Install musl tools - run: sudo apt-get install musl-tools - if: "contains(matrix.target, 'musl')" - - name: Cache cargo registry - uses: actions/cache@v3 + + - name: Set up Ubuntu multiarch + if: startsWith(matrix.os, 'ubuntu') && matrix.target-apt-arch != 'amd64' + run: | + readonly DISTRO_CODENAME=jammy + sudo dpkg --add-architecture "${{ matrix.target-apt-arch }}" + sudo sed -i "s/^deb http/deb [arch=$(dpkg-architecture -q DEB_HOST_ARCH)] http/" /etc/apt/sources.list + for suite in '' '-updates' '-backports' '-security'; do + echo "deb [arch=${{ matrix.target-apt-arch }}] http://ports.ubuntu.com/ $DISTRO_CODENAME$suite main universe multiverse" | \ + sudo tee -a /etc/apt/sources.list >/dev/null + done + + - name: Install musl development files + if: endsWith(matrix.target, '-musl') + run: | + sudo apt-get -yq update + sudo apt-get -yq install musl-tools musl-dev:${{ matrix.target-apt-arch }} + + - name: Install QEMU and AArch64 cross compiler + if: startsWith(matrix.target, 'aarch64-unknown-linux') + run: | + sudo apt-get -yq update + # libc6 must be present to run executables dynamically linked + # against glibc for the target architecture + sudo apt-get -yq install qemu-user gcc-aarch64-linux-gnu libc6:arm64 + + - name: Cache Cargo artifacts + uses: Swatinem/rust-cache@v2 + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@v1 with: - path: | - ~/.cargo/bin/ - ~/.cargo/registry/index/ - ~/.cargo/registry/cache/ - ~/.cargo/git/db/ - key: ${{ runner.os }}-${{ matrix.toolchain }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-${{ matrix.toolchain }}-cargo-registry- - - name: Install ${{ matrix.toolchain }} - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: ${{ matrix.toolchain }} - override: true + toolchain: nightly + targets: ${{ env.CARGO_BUILD_TARGET }} components: clippy, rustfmt + + - name: Install nextest + uses: taiki-e/install-action@nextest + - name: Run rustfmt - if: matrix.toolchain == 'stable' - uses: actions-rs/cargo@v1 + if: matrix.target == 'x86_64-unknown-linux-gnu' + run: cargo fmt --check + + - name: Run Clippy (no default features) + if: matrix.target == 'x86_64-unknown-linux-gnu' + uses: giraffate/clippy-action@v1 with: - command: fmt - args: -- --check - - name: Run clippy - if: matrix.toolchain == 'stable' - uses: actions-rs/clippy-check@v1 + clippy_flags: --no-deps --all-targets --no-default-features -- -D warnings + reporter: github-check + fail_on_error: true + + - name: Run Clippy (all features) + if: matrix.target == 'x86_64-unknown-linux-gnu' + uses: giraffate/clippy-action@v1 with: - token: ${{ secrets.GITHUB_TOKEN }} - args: -- -D warnings + clippy_flags: --no-deps --all-targets --all-features -- -D warnings + reporter: github-check + fail_on_error: true + + # There aren't good user-mode ARM64 emulators we can use on x64 macOS hosts. + # QEMU doesn't have any plans to add such support due to a lack of kernel + # syscall stability guarantees: https://gitlab.com/qemu-project/qemu/-/issues/1682 - name: Run tests - run: cargo test --features sanity-checks + if: matrix.target != 'aarch64-apple-darwin' + run: | + cargo nextest run --release --features sanity-checks + cargo test --doc --release --features sanity-checks + + - name: Build tests (ARM64 macOS only) + if: matrix.target == 'aarch64-apple-darwin' + run: cargo test --release --features sanity-checks --no-run + - name: Build benchmarks - if: matrix.toolchain == 'nightly' run: cargo bench --no-run + - name: Build docs - run: cargo doc --no-deps - - name: Check no default features - if: matrix.toolchain == 'stable' - uses: actions-rs/clippy-check@v1 + if: matrix.target == 'x86_64-unknown-linux-gnu' + run: cargo doc --release --no-deps + + - name: Build CLI binary + run: cargo build --release + + - name: Upload CLI binary as artifact + uses: actions/upload-artifact@v3 with: - token: ${{ secrets.GITHUB_TOKEN }} - args: --no-default-features -- -D warnings + name: Oxipng binary (${{ matrix.target }}) + path: | + target/${{ env.CARGO_BUILD_TARGET }}/release/oxipng + target/${{ env.CARGO_BUILD_TARGET }}/release/oxipng.exe + + msrv-check: + name: MSRV check + + runs-on: ubuntu-latest + timeout-minutes: 30 + + # Prevent tags and in-repo PRs from triggering this workflow more than once for a commit + if: github.ref_type != 'tag' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork) + + steps: + - name: Checkout source + uses: actions/checkout@v3 + with: + persist-credentials: false + + - name: Cache Cargo artifacts + uses: Swatinem/rust-cache@v2 + + - name: Install MSRV Rust toolchain + uses: dtolnay/rust-toolchain@1.65.0 + + - name: Install nextest + uses: taiki-e/install-action@nextest + + - name: Run tests + run: | + cargo nextest run --release --features sanity-checks + cargo test --doc --release --features sanity-checks diff --git a/Cargo.toml b/Cargo.toml index acca5133..be018b5f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,6 +86,7 @@ opt-level = 2 [profile.release] lto = "thin" +strip = "symbols" [profile.dev.package.bitvec] opt-level = 3 diff --git a/src/sanity_checks.rs b/src/sanity_checks.rs index 8150d72d..d9f1b474 100644 --- a/src/sanity_checks.rs +++ b/src/sanity_checks.rs @@ -23,7 +23,7 @@ pub fn validate_output(output: &[u8], original_data: &[u8]) -> bool { (Ok(new_frames), Ok(old_frames)) if new_frames.len() != old_frames.len() => false, (Ok(new_frames), Ok(old_frames)) => { for (a, b) in old_frames.iter().zip(new_frames) { - if !images_equal(&a, &b) { + if !images_equal(a, &b) { return false; } } diff --git a/tests/flags.rs b/tests/flags.rs index 7b69c99b..8b256953 100644 --- a/tests/flags.rs +++ b/tests/flags.rs @@ -48,7 +48,7 @@ fn test_it_converts_callbacks( CBPOST: FnMut(&Path), CBPRE: FnMut(&Path), { - let png = PngData::new(&input, &opts).unwrap(); + let png = PngData::new(&input, opts).unwrap(); assert_eq!(png.raw.ihdr.color_type.png_header_code(), color_type_in); assert_eq!(png.raw.ihdr.bit_depth, bit_depth_in); @@ -64,7 +64,7 @@ fn test_it_converts_callbacks( callback_post(output); - let png = match PngData::new(output, &opts) { + let png = match PngData::new(output, opts) { Ok(x) => x, Err(x) => { remove_file(output).ok(); @@ -196,7 +196,7 @@ fn verbose_mode() { for (i, log) in logs.into_iter().enumerate() { let expected_prefix = expected_prefixes[i]; assert!( - log.starts_with(&expected_prefix), + log.starts_with(expected_prefix), "logs[{}] = {:?} doesn't start with {:?}", i, log, diff --git a/tests/raw.rs b/tests/raw.rs index c3e16fb2..107bf5e5 100644 --- a/tests/raw.rs +++ b/tests/raw.rs @@ -15,7 +15,7 @@ fn test_it_converts(input: &str) { let input = PathBuf::from(input); let opts = get_opts(); - let original_data = PngData::read_file(&PathBuf::from(input)).unwrap(); + let original_data = PngData::read_file(&input).unwrap(); let image = PngData::from_slice(&original_data, &opts).unwrap(); let png = Arc::try_unwrap(image.raw).unwrap();