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: Support no-deprecated & no-tests #180611

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

IceCodeNew
Copy link

Description of changes
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.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jul 8, 2022
@IceCodeNew
Copy link
Author

Re-open #180452

@bobby285271 bobby285271 added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jul 8, 2022
@IceCodeNew IceCodeNew changed the title openssl: Support disabling deprecated functions openssl: Support no-deprecated & no-tests Jul 9, 2022
@IceCodeNew
Copy link
Author

tested through commands

nix-channel --update
package_name=openssl
nix-build --no-out-link --expr "with import <nixpkgs> {}; (pkgsStatic.${package_name}).overrideAttrs (old: { configureFlags = (old.configureFlags or []) ++ [ \"no-tests\" \"no-deprecated\" ]; })"

@bobby285271 bobby285271 removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jul 9, 2022
@ajs124 ajs124 requested a review from alyssais July 9, 2022 14:43
@alyssais
Copy link
Member

Something has gone wrong with your "openssl: Support sskipskippingthe tests" commit message. What's the reason for disabling the tests, just a faster build?

@alyssais
Copy link
Member

Also, in your first commit you introduce the no-deprecated option at the end of the options list, but then in a later commit you move it. Could you please modify your commits so that it's introduced in your preferred location from the start? Otherwise it's pointlessly adding an extra cycle to somebody trying to use git blame in future.

@IceCodeNew
Copy link
Author

What's the reason for disabling the tests, just a faster build?

Yes.
May I ask if there is a standard for what options are encouraged to be included in the nix formula, and what are not?
As the upstream has already left those options, It is not strange that someday the nix users will have asked for every single option to be tunable in the nix formula, right?
But such a complicated formula does not sound right to me. Am I abusing the nixpkgs repo?

@IceCodeNew
Copy link
Author

Otherwise it's pointlessly adding an extra cycle to somebody trying to use git blame in future.

I was expecting this PR to be merge with the squash-ish way. Can we squash this PR when it is ready for merging?

@IceCodeNew IceCodeNew force-pushed the 4pr_openssl branch 2 times, most recently from 5363ef8 to 2c8eaca Compare July 11, 2022 16:00
@IceCodeNew
Copy link
Author

It's been a week ago since this PR got reviewed, I'm wondering if @alyssais could spend some time guiding me to consummate this PR. Thanks ;-)

@IceCodeNew
Copy link
Author

@alyssais @SuperSandro2000 I just resolved the conflict.
Should something is missing in getting this PR merged, would you please shed some light on me?

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 27, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 27, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 1, 2025
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 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants