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

ipmitool: migrate to openssl 1.1 #80987

Merged
merged 3 commits into from Mar 1, 2020
Merged

ipmitool: migrate to openssl 1.1 #80987

merged 3 commits into from Mar 1, 2020

Conversation

andir
Copy link
Member

@andir andir commented Feb 24, 2020

Motivation for this change

This adds a patch from debian to switch ipmitool to openssl 1.1. Upstream seems to already carry a version of this but that is yet to be part of a release.

If this is accepted we should backport to 20.03.

cc #80746

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@andir andir requested a review from fpletz February 24, 2020 22:11
This adds a patch from debian to switch ipmitool to openssl 1.1.
Upstream seems to already carry a version of this but that is yet to be
part of a release.
@flokli
Copy link
Contributor

flokli commented Feb 25, 2020

Also, while at it, can you replace the configureFlagsArray hackery in preConfigure with just setting configureFlags, and using placeholder?

@flokli
Copy link
Contributor

flokli commented Feb 28, 2020

@vcunat, @andir I adressed the suggested changes, PTAL.

@flokli flokli requested a review from vcunat February 28, 2020 20:12
@vcunat
Copy link
Member

vcunat commented Feb 28, 2020

It seems like you care about darwin here, so what about adding it into meta.platforms?

url = "https://sources.debian.org/data/main/i/ipmitool/1.8.18-6/debian/patches/0120-openssl1.1.patch";
sha256 = "1xvsjxb782lzy72bnqqnsk3r5h4zl3na95s4pqn2qg7cic2mnbfk";
})
];
Copy link
Member

@vcunat vcunat Feb 28, 2020

Choose a reason for hiding this comment

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

Nitpick: this doesn't seem like dynamically generated patch, so I expect fetchurl should suffice, but I see no problem with using fetchpatch anyway. EDIT: this referred to the debian one – GitHub now shows the comment under the other one which is dynamically generated.

@worldofpeace
Copy link
Contributor

@GrahamcOfBorg eval

@vcunat vcunat added the 9.needs: port to stable A PR needs a backport to the stable release. label Feb 29, 2020
@flokli
Copy link
Contributor

flokli commented Mar 1, 2020

It seems like you care about darwin here, so what about adding it into meta.platforms?

I can't verify this works on Darwin, as I don't have the hardware around - I just realized the mentioned substitution became a no-op.

flokli and others added 2 commits March 1, 2020 08:49
* remove no-op substitution of s6_addr16 -> s6_addr

This string doesn't exist anymore in that file.

* clean up configureFlags
@worldofpeace
Copy link
Contributor

Just waiting on this to build on darwin now.
It appears @flokli and I have captured your PR @andir 🤣

@worldofpeace worldofpeace merged commit 851aac4 into NixOS:master Mar 1, 2020
@worldofpeace
Copy link
Contributor

backported to 20.03 in

@vcunat vcunat removed the 9.needs: port to stable A PR needs a backport to the stable release. label Mar 1, 2020
@andir
Copy link
Member Author

andir commented Mar 4, 2020 via email

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.

None yet

4 participants