Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update libjxl dependencies, fix static compilation #179102

Merged
merged 9 commits into from
Aug 16, 2022
Merged

Update libjxl dependencies, fix static compilation #179102

merged 9 commits into from
Aug 16, 2022

Conversation

dali99
Copy link
Member

@dali99 dali99 commented Jun 25, 2022 •
edited
Loading

Description of changes

Updates packages and adds special options to make them work with pkgsStatic

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.05 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@dali99 dali99 changed the title Libjxl static Update libjxl dependencies, fix static compilation Jun 25, 2022
@ofborg ofborg bot requested review from vcunat, paperdigits, codyopel and nh2 June 25, 2022 23:17
@dali99 dali99 requested a review from SuperSandro2000 June 26, 2022 00:17
@dali99 dali99 mentioned this pull request Jun 27, 2022
Merged
13 tasks
@mweinelt mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 27, 2022
@alyssais
Copy link
Member

The most objectionable thing here is probably removing gperftools in libjxl when statically compiling but it seems to work tm

I was able to get pkgsStatic.gperftools to build by removing the libunwind dependency, as is already done for ARM platforms. That should allow libjxl to have its gperftools dependency even when building statically.

@alyssais alyssais mentioned this pull request Jun 30, 2022
Merged
13 tasks
@alyssais alyssais added 6.topic: static 6.topic: musl Running or building packages with musl libc labels Jun 30, 2022
@dali99
Copy link
Member Author

dali99 commented Jul 2, 2022

While trying to apply what was said in #179102 (comment) I got some weird compilation errors I didn't see before (even without those changes). I dont have time to check out why right now, so please hold off on merging for a few more days. Converting to draft in the mean-time

@dali99 dali99 marked this pull request as draft July 2, 2022 01:14
@alyssais
Copy link
Member

alyssais commented Jul 2, 2022

I got some weird compilation errors I didn't see before (even without those changes). I dont have time to check out why right now

Can you share what they are? It's likely stuff that's just broken on staging in the meantime and is already a known issue — it's been quite volatile recently.

@dali99 dali99 force-pushed the libjxl_static branch 2 times, most recently from 69309ea to b59aa5a Compare July 2, 2022 13:40
@dali99 dali99 marked this pull request as ready for review July 2, 2022 13:41
@alyssais
Copy link
Member

alyssais commented Jul 3, 2022

The fix for pkgsStatic.libwebp (with TIFF support) is #180018.

@alyssais alyssais added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 3, 2022
@bobby285271 bobby285271 removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jul 4, 2022
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 12, 2022
@dali99 dali99 force-pushed the libjxl_static branch 2 times, most recently from 604a242 to bc6efcb Compare August 15, 2022 01:06
@alyssais
Copy link
Member

OfBorg failed to build graphviz, but it WFM on latest staging, so let's re-eval with a new merge and try again:

@ofborg eval

@alyssais
Copy link
Member

[nitpick] The commit summary "lib/systems: Add common static library extensions to extensions, and common attribute for static and shared libraries" is really long. A more reasonable one might be "lib/systems: add staticLibrary and library", with more explanation in the body. As it is, GitHub will truncate it, and so will git log unless somebody has a really wide terminal.

But I'm not going to hold up the PR any more for that, just something to be aware of in future.

SuperSandro2000
SuperSandro2000 reviewed Aug 15, 2022
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

otherwise LGTM
There are some ofborg failures. Please take a look at them.

pkgs/development/libraries/libhwy/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libhwy/default.nix Outdated Show resolved Hide resolved
dali99 and others added 7 commits August 15, 2022 15:19
@dali99
openexr: 2.5.7 -> 2.5.8
a144251
@dali99
openexr*: Fix statically building
28ced36
@dali99
gperftools: 2.9.1 -> 2.10
ac6e79e
@dali99 @alyssais
gperftools: remove libunwind when building statically
a2a8cf5 
Co-authored-by: Alyssa Ross <[email protected]>
@dali99
libwebp: don't patch shebangs
f2afc6c 
It isn't needed as none of the scripts are being ran during build.

See NixOS@2c7119d for discussion
@dali99
giflib: Remove xml dependencies
e468b31 
These should have been in nativeBuildInputs anyways.
But it seems the docbook generation isn't needed when downloading the tarball

It didn't change the output anyways.
So if there are any docs that were supposed to be built that needs to be fixed somehow.
@dali99
lib/systems: Add staticLibrary and library
de70971 
staticLibrary includes common extensions for static libraries
library is a new common attribute that includes both shared and static extensions
alyssais and others added 2 commits August 15, 2022 15:20
@alyssais @dali99
libhwy: fix tests when cross-compiling
7c86ef1 
It seems to need some help to find gtest when cross compiling.

(Only applies when cross-compiling to compatible architectures where
the tests are enabled, like pkgsStatic.)

Co-authored-by: Daniel Olsen <[email protected]>
@dali99 @alyssais
libjxl: Fix static compilation
49cc0e1 
Co-authored-by: Alyssa Ross <[email protected]>
@alyssais
Copy link
Member

@ofborg build pkgsStatic.libjxl

@alyssais alyssais merged commit 0628889 into NixOS:staging Aug 16, 2022
@roconnor-blockstream
Copy link
Contributor

@dali99 Is there some way to patch pprof shebangs so that it refers to a fixed perl derivation? I'd like to run pprof in a pure environment.

@dali99
Copy link
Member Author

dali99 commented Jan 6, 2024 •
edited
Loading

I don't see what pprof has to do with this PR, I'm not really an expert in perl and haven't used pprof. I'm also not sure which pprof shebangs you're asking about, so I don't think this is the right place to talk about this.

Sorry I can't be of anymore help, maybe you can try asking in #nix:nixos.org or on the discourse?

@roconnor-blockstream
Copy link
Contributor

Sorry you are right. This was the last PR to bump the version of gperftools. I didn't realize you were not the maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@vcunat vcunat Awaiting requested review from vcunat

@paperdigits paperdigits Awaiting requested review from paperdigits

@codyopel codyopel Awaiting requested review from codyopel

@nh2 nh2 Awaiting requested review from nh2

@ajs124 ajs124 Awaiting requested review from ajs124

@zhaofengli zhaofengli Awaiting requested review from zhaofengli

@alyssais alyssais Awaiting requested review from alyssais

@nbp nbp Awaiting requested review from nbp

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
6.topic: musl Running or building packages with musl libc 6.topic: static 10.rebuild-darwin: 501+ 10.rebuild-darwin: 2501-5000 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dali99 @alyssais @roconnor-blockstream @SuperSandro2000 @mweinelt @bobby285271