Merge branch 'master' into filter-png

.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'

.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

.github/workflows/oxipng.yml

@@ -2,100 +2,181 @@ 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
+          sudo sed -i "s/^deb mirror/deb [arch=$(dpkg-architecture -q DEB_HOST_ARCH)] mirror/" /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

Cargo.toml

@@ -86,6 +86,7 @@ opt-level = 2
 
 [profile.release]
 lto = "thin"
+strip = "symbols"
 
 [profile.dev.package.bitvec]
 opt-level = 3

src/lib.rs

@@ -180,7 +180,7 @@ pub struct Options {
     ///
     /// Default: `true`
     pub grayscale_reduction: bool,
-    /// Whether to perform IDAT recoding
+    /// Whether to perform recoding of IDAT and other compressed chunks
     ///
     /// If any type of reduction is performed, IDAT recoding will be performed
     /// regardless of this setting
@@ -904,23 +904,25 @@ fn postprocess_chunks(
                 None
             };
             // sRGB-like profile can be replaced with an sRGB chunk with the same rendering intent
-            // Otherwise try recompressing the profile
             if let Some(intent) = intent {
                 trace!("Replacing iCCP chunk with equivalent sRGB chunk");
                 png.aux_chunks[iccp_idx] = Chunk {
                     name: *b"sRGB",
                     data: vec![intent],
                 };
-            } else if let Ok(iccp) = construct_iccp(&icc, opts.deflate) {
-                let cur_len = png.aux_chunks[iccp_idx].data.len();
-                let new_len = iccp.data.len();
-                if new_len < cur_len {
-                    debug!(
-                        "Recompressed iCCP chunk: {} ({} bytes decrease)",
-                        new_len,
-                        cur_len - new_len
-                    );
-                    png.aux_chunks[iccp_idx] = iccp;
+            } else if opts.idat_recoding {
+                // Try recompressing the profile
+                if let Ok(iccp) = construct_iccp(&icc, opts.deflate) {
+                    let cur_len = png.aux_chunks[iccp_idx].data.len();
+                    let new_len = iccp.data.len();
+                    if new_len < cur_len {
+                        debug!(
+                            "Recompressed iCCP chunk: {} ({} bytes decrease)",
+                            new_len,
+                            cur_len - new_len
+                        );
+                        png.aux_chunks[iccp_idx] = iccp;
+                    }
                 }
             }
         }
@@ -950,7 +952,7 @@ fn postprocess_chunks(
         .iter_mut()
         .filter(|c| &c.name == b"fdAT")
         .collect();
-    if !fdat.is_empty() {
+    if opts.idat_recoding && !fdat.is_empty() {
         let buffer_size = orig_ihdr.raw_data_size();
         fdat.par_iter_mut()
             .with_max_len(1)

src/main.rs

@@ -233,13 +233,13 @@ fn main() {
         )
         .arg(
             Arg::new("no-reductions")
-                .help("No reductions")
+                .help("No reductions or deinterlacing")
                 .long("nx")
                 .action(ArgAction::SetTrue),
         )
         .arg(
             Arg::new("no-recoding")
-                .help("No IDAT recoding unless necessary")
+                .help("No recoding of IDAT or other compressed chunks unless necessary")
                 .long("nz")
                 .action(ArgAction::SetTrue),
         )
@@ -456,14 +456,6 @@ fn parse_opts_into_struct(
         Some(level) => Options::from_preset(level.parse::<u8>().unwrap()),
     };
 
-    if let Some(x) = matches.get_one::<String>("interlace") {
-        opts.interlace = if x == "keep" {
-            None
-        } else {
-            x.parse::<u8>().unwrap().try_into().ok()
-        };
-    }
-
     if let Some(x) = matches.get_one::<IndexSet<u8>>("filters") {
         opts.filter.clear();
         for &f in x {
@@ -532,10 +524,19 @@ fn parse_opts_into_struct(
         opts.color_type_reduction = false;
         opts.palette_reduction = false;
         opts.grayscale_reduction = false;
+        opts.interlace = None;
     }
 
     opts.idat_recoding = !matches.get_flag("no-recoding");
 
+    if let Some(x) = matches.get_one::<String>("interlace") {
+        opts.interlace = if x == "keep" {
+            None
+        } else {
+            x.parse::<u8>().unwrap().try_into().ok()
+        };
+    }
+
     if let Some(keep) = matches.get_one::<String>("keep") {
         let names = keep
             .split(',')

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;
                 }
             }

tests/flags.rs

@@ -48,7 +48,7 @@ fn test_it_converts_callbacks<CBPRE, CBPOST>(
     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<CBPRE, CBPOST>(
 
     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,

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();