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] remove run-time dependency of perl due to c_rehash #219323

Closed

Conversation

adrian-gierakowski
Copy link
Contributor

Description of changes

Replaces perl based c_rehash script with shell script wrapping openssl rehash
with the same functionality.

Fixes: #19965
Supersedes: #156776, #83446
Possibly related to: #157093, #82924

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/)
  • 23.05 Release Notes (or backporting 22.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.

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
@adrian-gierakowski
Copy link
Contributor Author

as discussed here the withPerl flag could be removed altogether

happy to do this if there are no objections

@Izorkin
Copy link
Contributor

Izorkin commented Mar 3, 2023

Need to add these changes for quictls?

@adrian-gierakowski
Copy link
Contributor Author

Need to add these changes for quictls?

Was just going to ask 😄

Once the final decision is made regarding keeping\removing the withPerl flag I’ll apply the changes there as well

@adrian-gierakowski
Copy link
Contributor Author

@veprbl I saw you did code review on #156776

Do you have the power to merge this? What's your opinion regarding removing the withPerl flag? Thanks!

@Artturin
Copy link
Member

Artturin commented Mar 10, 2023

let's drop withPerl (assuming rehash is fully compatible)

@adrian-gierakowski adrian-gierakowski force-pushed the openssl.bin/remove-dep-on-perl branch from f6fdef7 to 2d7e0f0 Compare March 10, 2023 15:33
@adrian-gierakowski adrian-gierakowski force-pushed the openssl.bin/remove-dep-on-perl branch from 2d7e0f0 to b7a5ddb Compare March 10, 2023 16:52
@adrian-gierakowski
Copy link
Contributor Author

@Artturin removed withPerl and applied your improvement to the wrapper scripts
also applied the same changes to quictls

@Artturin
Copy link
Member

Needs to target staging and the commit msg's need to be changed to our style, instructions for both in CONTRIBUTING.md

@thomasjm
Copy link
Contributor

Hey, there's about a month until the 23.05 release and it would be awesome to get this merged before then. Are there any blockers remaining?

@Artturin
Copy link
Member

Artturin commented Apr 14, 2023

@ofborg build openssl quictls

@Artturin
Copy link
Member

Needs to target staging check CONTRIBUTING.md for how to rebase

@Artturin
Copy link
Member

oh already said that

@Artturin
Copy link
Member

Can't push to this PR ERROR: Permission to rhinofi/nixpkgs.git denied to Artturin. even though Maintainers are allowed to edit this pull request.

@Artturin
Copy link
Member

Artturin commented Apr 14, 2023

#226108 kept your authorship in the commits

@adrian-gierakowski
Copy link
Contributor Author

@Artturin thanks! I didn’t really have time to address your comment so it’s great to took care of it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openssl.bin depends on perl because of c_rehash
4 participants