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
imv: 4.1.0 -> 4.2.0 #108309
imv: 4.1.0 -> 4.2.0 #108309
Conversation
* switch build system to wayland * enable all backends by adding the following to buildInputs as meson autodetects which backends are available. * libtiff * libheif * libpng Open questions: * imv prints a warning from the tiff backend everytime a non tiff file is opened: Is this normal? Seems harmless enough though. * Should we make backends configurable / optional? I readded some backends which apparently were removed, but still given as an argument to the derivation. Resolves NixOS#108185.
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.
LGTM.
$ nixpkgs-review pr 108309
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/108309/head:refs/nixpkgs-review/1
remote: Enumerating objects: 10, done.
remote: Counting objects: 100% (10/10), done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 12 (delta 5), reused 5 (delta 5), pack-reused 2
Unpacking objects: 100% (12/12), 13.63 KiB | 4.54 MiB/s, done.
From ssh://github.com/NixOS/nixpkgs
4d0e996b71d..712830d101d master -> refs/nixpkgs-review/0
+ 9d0164d53d4...a1e9a94785c refs/pull/108309/head -> refs/nixpkgs-review/1 (forced update)
$ git worktree add /home/matt/.cache/nixpkgs-review/pr-108309/nixpkgs 712830d101d94ae29a2310d91b4d54c0132f5716
Preparing worktree (detached HEAD 712830d101d)
HEAD is now at 712830d101d Merge pull request #108310 from mdlayher/mdl-bump-corerad
$ git merge --no-commit a1e9a94785cf885487d823790aa82dc82fe5924c
Automatic merge went well; stopped before committing as requested
$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/matt/.cache/nixpkgs-review/pr-108309/build.nix
[3 built, 21 copied (127.9 MiB), 22.4 MiB DL]
https://github.com/NixOS/nixpkgs/pull/108309
1 package built:
imv
$ nix-shell /home/matt/.cache/nixpkgs-review/pr-108309/shell.nix
You beat me to it: I have just been working on this: rnhmjoj@f288f7ac25e.
It doesn't in 4.1.0, it's probably a bug but imv seems to work just fine.
I would, but It doesn't look like they can be configured at all: the readme says most are optional but meson seems to require them all: maybe it's a bug in the build system. In any case, I would make X11/wayland configurable: you can pick from my commit if you want. |
imv 4.1.0 is compiled without |
There's an upstream issue for it: eXeC64/imv#252 |
Result of 1 package marked as broken and skipped:
|
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package built:
|
@rnhmjoj made imv (very) configurable via overrides in e635fca398bafbfe4f4f7cf4771d474c8e5b29b9 |
5d57a6b
to
3408d19
Compare
Why once of the sudden do we need to disable backends? Can we just build them all and be done? The package is really overengineered right now. Options are great if you want to disable options by default or there is a reason to disable them but here I can see none. |
Closure size, for instance: disabling GIF and SVG, which are rarely used, saves about 20MB, TIFF saves 40MB, and more for others. imv is a very minimal image viewer and was designed to have optional backends, so it's should definitely be a feature of the package. |
3408d19
to
47c4881
Compare
The following new derivation inputs are added: * withBackends: a list of all backends to enable. The valid names are the same as specified in imv's meson_options.txt. Default is to enable all 7 backends: freeimage, libtiff, libjpeg(_turbo), libpng, librsvg, libnsgif and libheif * withWindowSystem: either all, x11 or wayland to describe window system support.
47c4881
to
4f927ee
Compare
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.
Looks all good. Thank you!
Open questions:
is opened: Is this normal? Seems harmless enough though.
Should we make backends configurable / optional? I readded somebackends which apparently were removed, but still given as an argument
to the derivation.
Resolves #108185.
cc @markus1189 @rnhmjoj
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
) (only wayland)nix path-info -S
before and after)