NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.29k stars 13.54k forks source link

Build failure: diffoscope #328350

Closed panchoh closed 1 month ago

panchoh commented 1 month ago

Steps To Reproduce

Steps to reproduce the behavior:

  1. build diffoscope

Build log

Full log

❯ nix build .#diffoscope
error: builder for '/nix/store/gy75y3hyqci2cwr1xzvbp230fwabwzb3-diffoscope-271.drv' failed with exit code 1;
       last 10 log lines:
       > tests/comparators/test_jpeg_image.py:87: AssertionError
       > ----------------------------- Captured stderr call -----------------------------
       > WARNING: The convert command is deprecated in IMv7, use "magick" instead of "convert" or "magick convert"
       >
       > =========================== short test summary info ============================
       > FAILED tests/comparators/test_jpeg_image.py::test_has_visuals - assert 1 == 2
       >  +  where 1 = len([<Difference Image content -- Image content []>])
       >  +    where [<Difference Image content -- Image content []>] = <Difference /build/diffoscope-271/tests/data/test1.jpg -- /build/diffoscope-271/tests/data/test2.jpg [<Difference Image content -- Image content []>]>.details
       > = 1 failed, 679 passed, 38 skipped, 5 deselected, 1 xfailed, 1 xpassed in 52.00s =
       > /nix/store/5r0df66ikad3xw06azlqvswcvncll8wa-stdenv-linux/setup: line 1641: pop_var_context: head of shell_variables not a function context
       For full logs, run 'nix log /nix/store/gy75y3hyqci2cwr1xzvbp230fwabwzb3-diffoscope-271.drv'.

Additional context

From what I read in the build log, it seems that ImageMagick v7 has changed its cli interface a bit. Maybe we can depend on imagemagick6 until upstream catches up.

Notify maintainers

@dezgeg @danielfullmer @raitobezarius

Metadata

Build failure still present at c3e7bd26c8e281a677d444913663e1244d5e9d66.

❯ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.10.0, NixOS, 24.11 (Vicuna), 24.11.20240716.ad0b5ee`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.5`
 - nixpkgs: `/etc/nix/inputs/nixpkgs`

Add a :+1: reaction to issues you find important.

s0me1newithhand7s commented 1 month ago

seems like easy fix

s0me1newithhand7s commented 1 month ago

as i found, this error isn't nixpkgs related as o found "convert" option here and error is pointing on both WARNING and FAILED while diffoscope is running test_jpeg_image.py. i feel like this issue needs to be created inside diffoscope repository on salsa.

panchoh commented 1 month ago

I'll report this issue upstream. Currently waiting for the salsa admins to approve my request to create an account there.

I also tried the quick fix of replacing imagemagick with imagemagick6 in the derivation (1, 2), but it seems that there is a whole bunch of vulnerabilities in the legacy ImageMagick v6 series.

nh2 commented 1 month ago

as i found, this error isn't nixpkgs related as o found "convert" option here

Stable link: https://salsa.debian.org/reproducible-builds/diffoscope/-/blob/82bbacbf0060a1b89cde5b5c950f754935757605/tests/comparators/test_jpeg_image.py#L83

bjornfor commented 1 month ago

Ugh, this breakage also hit nixos-24.05 branch (the stable channel).

panchoh commented 1 month ago

I've just reported this upstream. Thanks for the feedback, folks.

s0me1newithhand7s commented 1 month ago

I've just reported this upstream. Thanks for the feedback, folks.

np!

panchoh commented 1 month ago

Chris Lamb has fixed the issue (haven't tested it yet, though).

bjornfor commented 1 month ago

Ugh, this breakage also hit nixos-24.05 branch (the stable channel).

Oh, it was a very small (imagemagic) update too: d35a2e8d0297d8cc30d1b3a6d2c6a5b66a3cb044 (imagemagick: 7.1.1-34 -> 7.1.1-35)

panchoh commented 1 month ago

Ugh, this breakage also hit nixos-24.05 branch (the stable channel).

Oh, it was a very small (imagemagic) update too: d35a2e8 (magemagick: 7.1.1-34 -> 7.1.1-35)

I find it a bit confusing since they state that they follow SemVer (can be seen at the top of the changelog you shared), and yet, most of their releases fall in the "prerelease" category (X.Y.Z-N) according to SemVer, which indicates that (and I quote): "A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version."

So, it follows that we cannot freely track changes to ImageMagick, or else we might break stuff. Not very SemVer friendly, IMHO.

panchoh commented 1 month ago

Fixed by https://github.com/NixOS/nixpkgs/pull/324888, yay!, so closing.

bjornfor commented 1 month ago

Fixed by https://github.com/NixOS/nixpkgs/pull/324888, yay!, so closing.

Yay!

Technically it was commit 9eae9c20350ad5b133bdfd63d27a2029a5734a79 that fixed the build failure (by skipping the broken test). We should revert that hotfix now that diffoscope is updated.

bjornfor commented 1 month ago

The hotfix (disable broken test) is already backported to NixOS 24.05: https://github.com/NixOS/nixpkgs/pull/330982

bjornfor commented 1 month ago

Technically it was commit 9eae9c2 that fixed the build failure (by skipping the broken test). We should revert that hotfix now that diffoscope is updated.

Reverting hotfix: https://github.com/NixOS/nixpkgs/pull/331487.