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

openssl: better check that withPerl=false prevents perl reference #156776

Closed
wants to merge 1 commit into from

Conversation

thomasjm
Copy link
Contributor

Motivation for this change

This is a PR to fix #19965, making it possible to remove a large 50MB perl dependency from the closure of openssl.bin when using withPerl=false. The checks in openssl/default.nix weren't quite right before, and there was an unnecessarily restrictive assert preventing you from setting withPerl to false when not cross-compiling.

I wanted to do this so I could deploy nodejs-slim in a Docker container, without bringing in perl. I'll open a separate PR for that.

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.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@thomasjm thomasjm changed the title Openssl slim openssl.bin: remove perl dependency when withPerl=false Jan 25, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jan 25, 2022
@veprbl veprbl added the 6.topic: closure size The final size of a derivation, including its dependencies label Jan 26, 2022
@veprbl
Copy link
Member

veprbl commented Jan 26, 2022

So at this point I see

# head $(nix-build -E '((import ./. {}).openssl.override { withPerl = false; }).bin')/bin/c_rehash
#!/usr/bin/env perl

So there is no bug, other than the pesky assert (the withPerl option introduced in ef24a28 and immediately locked out for no apparent reason). We could just remove disallowedReferences and merge this to master.

@thomasjm
Copy link
Contributor Author

Why not keep the disallowedReferences to keep this from regressing again? Looking at #19965 it seems like it has in the past.

@veprbl
Copy link
Member

veprbl commented Jan 27, 2022

Why not keep the disallowedReferences to keep this from regressing again? Looking at #19965 it seems like it has in the past.

If the goal is to prevent dependency on buildPackages.perl spliced from the nativeBuildInputs, then it's probably need to be spelled out in disallowedReferences explicitly (not just perl) and unconditionally with respect to withPerl. At that point we might as well set strictDeps = true;.

@veprbl
Copy link
Member

veprbl commented Jan 27, 2022

Also can we organize the two changes into separate commits (or even better, separate PRs)?

@thomasjm
Copy link
Contributor Author

I only found a single passing reference to strictDeps in the nixpkgs manual which I don't fully understand, but happy to switch to it if you prefer. For now I just updated the disallowedReferences.

@veprbl veprbl changed the base branch from master to staging January 27, 2022 05:42
@veprbl veprbl requested a review from aanderse as a code owner January 27, 2022 05:42
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jan 13, 2024
Replaces perl based c_rehash script with shell script wrapping `openssl rehash`
with the same functionality.

Fixes: NixOS#19965
Supersedes: NixOS#156776, NixOS#83446
Possibly related to: NixOS#157093, NixOS#82924
tm-drtina pushed a commit to awakesecurity/nixpkgs that referenced this pull request Feb 27, 2024
Replaces perl based c_rehash script with shell script wrapping `openssl rehash`
with the same functionality.

Fixes: NixOS#19965
Supersedes: NixOS#156776, NixOS#83446
Possibly related to: NixOS#157093, NixOS#82924
tm-drtina pushed a commit to awakesecurity/nixpkgs that referenced this pull request Apr 8, 2024
Replaces perl based c_rehash script with shell script wrapping `openssl rehash`
with the same functionality.

Fixes: NixOS#19965
Supersedes: NixOS#156776, NixOS#83446
Possibly related to: NixOS#157093, NixOS#82924
tm-drtina pushed a commit to awakesecurity/nixpkgs that referenced this pull request May 16, 2024
Replaces perl based c_rehash script with shell script wrapping `openssl rehash`
with the same functionality.

Fixes: NixOS#19965
Supersedes: NixOS#156776, NixOS#83446
Possibly related to: NixOS#157093, NixOS#82924
tm-drtina pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jun 7, 2024
Replaces perl based c_rehash script with shell script wrapping `openssl rehash`
with the same functionality.

Fixes: NixOS#19965
Supersedes: NixOS#156776, NixOS#83446
Possibly related to: NixOS#157093, NixOS#82924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: closure size The final size of a derivation, including its dependencies 6.topic: cross-compilation Building packages on a different platform than they will be used on 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openssl.bin depends on perl because of c_rehash
7 participants