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
nmap: fix build with new Darwin stdenv #61733
Conversation
fc512c4
to
983358a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm it builds on darwin
patches = ./zenmap.patch; | ||
patches = [ ./zenmap.patch ] | ||
++ optional stdenv.cc.isClang ( | ||
# https://github.com/nmap/nmap/pull/1363 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a comment about why this patch is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment here with the details of what the patch fixes. I don't know why it's only necessary in clang, since the code looks invalid in any c++11 environment. I'm trusting the description of upstream PR.
pkgs/tools/security/nmap/default.nix
Outdated
@@ -27,7 +27,14 @@ in stdenv.mkDerivation rec { | |||
sha256 = "063fg8adx23l4irrh5kn57hsmi1xvjkar4vm4k6g94ppan4hcyw4"; | |||
}; | |||
|
|||
patches = ./zenmap.patch; | |||
patches = [ ./zenmap.patch ] | |||
++ optional stdenv.cc.isClang ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minorly, use optionals
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thread isn't clear: is optionals [(fetchpatch {})]
preferred over optional (fetchpatch {})
due to the increased type safety?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for humor me, this is more of a personal preference, since it's easy to mix optional/optionals (e.g. #60123) I'm inclined to use optionals
even when is not necessary
983358a
to
5f783da
Compare
5f783da
to
6520451
Compare
Tested locally on Darwin |
Motivation for this change
Fixes #61595.
Brings in changes from unmerged upstream PR nmap/nmap#1363.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)