Skip to content

Comments

libjxl: 0.8.2 -> 0.9.1, libaom: remove butteraugli support#282472

Merged
h7x4 merged 1 commit intoNixOS:stagingfrom
simonhollingshead:libjxl091
Jan 24, 2024
Merged

libjxl: 0.8.2 -> 0.9.1, libaom: remove butteraugli support#282472
h7x4 merged 1 commit intoNixOS:stagingfrom
simonhollingshead:libjxl091

Conversation

@simonhollingshead
Copy link
Member

Description of changes

Bumps version of libjxl to 0.9.1. Changes can be seen at https://github.com/libjxl/libjxl/releases.

One of the major changes for 0.9.x is the removal of the butteraugli API.

I have had to remove the option from libaom to use the butteraugli API as a result.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Jan 21, 2024
@simonhollingshead simonhollingshead marked this pull request as ready for review January 21, 2024 03:25
@ofborg ofborg bot requested review from D4ndellion, kiloreux, nh2 and primeos January 21, 2024 06:03
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jan 21, 2024
Copy link
Member

@D4ndellion D4ndellion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR for the removal has some more info: libjxl/libjxl#2576

I don't find it likely libaom will ever enable butteraugli via some other api, and the feature never worked at 10bits which limited its usefulness anyways

@D4ndellion
Copy link
Member

D4ndellion commented Jan 21, 2024

buildDocs was added in fefca2f to avoid a circular dependency with libavif -> libaom -> libjxl -> graphviz -> gd -> libavif. Since libaom no longer depends on libjxl this flag can be removed

The override in top-level/all-packages.nix disabling building the docs for the libjxl in libaom also needs to be removed

Copy link
Member

@D4ndellion D4ndellion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I built this up to gimp, then successfully opened some jxls.

I also built libaom and did a test encode with aomenc

(on x86_64)

Thanks for your contribution!

@D4ndellion D4ndellion requested review from h7x4 and pbsds January 23, 2024 16:55
@delroth delroth added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jan 23, 2024
@delroth delroth added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jan 24, 2024
@delroth delroth removed the 12.approvals: 2 This PR was reviewed and approved by two persons. label Jan 24, 2024
@h7x4 h7x4 merged commit e672d52 into NixOS:staging Jan 24, 2024
@vcunat
Copy link
Member

vcunat commented Feb 9, 2024

I think this is what broke the build of krita:

/build/krita-5.2.2/plugins/impex/jxl/JPEGXLImport.cpp:514:55: error: cannot convert 'std::nullptr_t' to 'JxlColorProfileTarget'
  514 |                                                       nullptr,
      |                                                       ^~~~~~~
      |                                                       |
      |                                                       std::nullptr_t

https://hydra.nixos.org/build/248773601#tabs-buildsteps

/cc krita maintainers: @abbradar, @sifmelcara, @nek0 and the PR #285983

@sifmelcara
Copy link
Member

I think this is what broke the build of krita

Opened #287532

@simonhollingshead simonhollingshead deleted the libjxl091 branch February 11, 2024 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants