-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Make netcat-openbsd the default netcat #19982
Conversation
@sternenseemann, thanks for your PR! By analyzing the history of the files in this pull request, we identified @aszlig, @rycee and @edolstra to be potential reviewers. |
netcat = callPackage ../tools/networking/netcat { }; | ||
netcat = netcat-openbsd; | ||
|
||
netcat-gnu = callPackage ../tools/networking/gnu-netcat { }; |
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.
Nitpick: the naming is rather inconsistent. I think it would be better to use netcat-gnu
even for the directory name.
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.
Yeah, I had named it differently before, so a bit of regression, good point.
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.
Fixed
ffa7ee0
to
8f4f34b
Compare
@@ -36,7 +36,7 @@ import ./make-test.nix (pkgs: { | |||
$machine->waitForShutdown; | |||
$machine->start; | |||
$probe->waitForUnit("network.target"); | |||
$probe->waitUntilSucceeds("echo test | nc -c machine 4444"); | |||
$probe->waitUntilSucceeds("echo test | nc -q 0 machine 4444"); |
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.
Why is the timeout here 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.
Oh, it is not needed indeed, let me fix that.
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.
Done
The motivation for this change is the following: As gnu-netcat, e. g. does not support ipv6, it is not suitable as default netcat. This commit also fixes all obvious build issues caused by this change.
8f4f34b
to
bbe9159
Compare
\o/ |
Noticed that a call to "nixops ssh" did not work anymore after I upgraded the target host to the current 17.03 channel. Tracked the issue down to a change of the default netcat in the following PR: NixOS/nixpkgs#19982
Noticed that a call to "nixops ssh" did not work anymore after I upgraded the target host to the current 17.03 channel. Tracked the issue down to a change of the default netcat in the following PR: NixOS/nixpkgs#19982
I'm not sure why 024b501 used -q 0 because even netcat-openbsd has the -N flag which IMO is the better way to shutdown the socket on EOF. Our default netcat implementation has changed once again[1] in 3c3b822 and we're now using LibreSSL's implementation, which doesn't have a -q flag. See #39634 for the pull request introducing the switch. [1]: #19982 Signed-off-by: aszlig <aszlig@nix.build> Cc: @matthewbauer, @dtzWill, @Mic92
I'm not sure why 024b501 used -q 0 because even netcat-openbsd has the -N flag which IMO is the better way to shutdown the socket on EOF. Our default netcat implementation has changed once again[1] in 3c3b822 and we're now using LibreSSL's implementation, which doesn't have a -q flag. See NixOS#39634 for the pull request introducing the switch. [1]: NixOS#19982 Signed-off-by: aszlig <aszlig@nix.build> Cc: @matthewbauer, @dtzWill, @Mic92
Noticed that a call to "nixops ssh" did not work anymore after I upgraded the target host to the current 17.03 channel. Tracked the issue down to a change of the default netcat in the following PR: NixOS/nixpkgs#19982
Motivation for this change
See Issue #19411.
TL;DR: netcat-gnu does neither support IPv6 nor unix sockets and is therefore not a good idea for a default netcat.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nixos/tests/hibernate.nix
nixos/tests/virtualbox.nix
build-support/vm/windows
I am open for suggestions on, how to test the windows vm build-support with GNU netcat.
A problem I see here is, that there is no standard deprecation process in nixpkgs (#15357). This change would certainly benefit from a
warnSemanticChange
which I proposed in this PR: #19315.