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.

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

libidn2: hack to avoid referencing bootstrap tools #175785

Merged
merged 1 commit into from
Jun 17, 2022

Conversation

vcunat
Copy link
Member

@vcunat vcunat commented Jun 1, 2022

Due to bootstrap tools getting purged from closure of libidn2.dev,
a very large rebuild is caused.


Fixes #175693

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.

@vcunat vcunat added the 6.topic: stdenv Standard environment label Jun 1, 2022
@vcunat
Copy link
Member Author

vcunat commented Jun 1, 2022

Eh, well, the conflict is trivial to resolve, now I rather want this to be testable without a stdenv rebuild.

This approach is the best I could come with, though perhaps someone will have a better idea or some nits to improve in this PR.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 1, 2022
buildInputs = [ patchelf ];

passthru = {
inherit (libidn2) out info devdoc; # no need to touch these store paths
Copy link
Member

Choose a reason for hiding this comment

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

This leads to unexpected results when requesting libidn2 from the CLI, which generally works with drvPath and its libstore-level outputs, rather than the outputs attribute. I'm proposing to redefine "package" in Nix here NixOS/nix#6507.

Copy link
Member

Choose a reason for hiding this comment

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

Presumably it works fine when used in expressions though.

It is only a bit of a problem when doing nix-build -A libidn2.

Copy link
Member Author

Choose a reason for hiding this comment

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

What are you getting from that command? I'm getting the right -bin output, i.e. the one without boostrap tools in closure. (also with nix build)

Copy link
Member Author

Choose a reason for hiding this comment

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

libidn2.all is a bit confusing, though I don't think it has so much usage. I also tried to make that not too bad... so you're missing info and devdoc, and out only differs by adding a symlink layer.

Copy link
Member

Choose a reason for hiding this comment

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

Right, it's more of a problem with the new CLI's output selector syntax, which operates at the store level (via drvPath) rather than the expression level.

Copy link
Member

Choose a reason for hiding this comment

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

$ nix-build -A libidn2.info
/nix/store/klbv14mif5q34nqj04v2bgv9d7qv2pk6-libidn2-2.3.2-info

$ nix build .#libidn2^info --print-out-paths 
warning: Git tree '/home/user/h/nixpkgs' is dirty
error: derivation '/nix/store/zy3r4mqqb8wsljg1cfdnw02mqplaqw8n-libidn2-2.3.2.drv' does not have wanted outputs 'info'

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I forgot to mention that stuff like libidn2.info.bin is also broken by such overrides, and I don't know a nice way of fixing that. (But again, I think it's rarely used.)

@bobby285271 bobby285271 added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 3, 2022
Due to bootstrap tools getting purged from closure of libidn2.dev,
a very large rebuild is caused.
@bobby285271 bobby285271 removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 5, 2022
@vcunat vcunat merged commit 5ffd19d into NixOS:staging Jun 17, 2022
@vcunat
Copy link
Member Author

vcunat commented Jun 17, 2022

I'll assume that we take this until a better proposal appears.

@vcunat vcunat deleted the p/libidn2-bin branch June 17, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: stdenv Standard environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants