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

Jump to bottom

wireguard-tools 1.0.20191226 / wireguard 0.0.20191226 #76578

Merged
merged 2 commits into from Dec 30, 2019
Merged

wireguard-tools 1.0.20191226 / wireguard 0.0.20191226 #76578

merged 2 commits into from Dec 30, 2019

Conversation

d-xo
Copy link
Contributor

@d-xo d-xo commented Dec 27, 2019

Motivation for this change

There has been a wireguard repo reorg as a result of it's acceptance into net-next. More details here.

The kernel module and userspace tools now live in separate repos and are versioned separately.

The announcements for the module and tools releases are here:

https://lists.zx2c4.com/pipermail/wireguard/2019-December/004796.html
https://lists.zx2c4.com/pipermail/wireguard/2019-December/004787.html

There should be no functional changes as a part of these releases.

I have also added myself as a package maintainer :).

tests performed

  • nix-build nixos/tests/wireguard/* passes
  • nix-build nixos/tests/systemd-networkd-wireguard.nix passes
  • nix-review rev 6332c4ac92f151a9cff77c6596470fff1b332069 passes
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.
Notify maintainers

cc @elseym @ericsagnes @Mic92 @zx2c4 @globin @Ma27

@d-xo d-xo changed the title Wireguard tools 1.0.20191226 wireguard-tools 1.0.20191226 / wireguard 0.0.20191226 Dec 27, 2019
@zx2c4
Copy link
Contributor

zx2c4 commented Dec 27, 2019

Thanks for taking care of this. Please merge this 👍.

@Ma27 Ma27 self-requested a review December 27, 2019 16:49
@Ma27
Copy link
Member

Ma27 commented Dec 27, 2019

@GrahamcOfBorg test wireguard wireguard-generated

@Ma27 Ma27 added the 9.needs: port to stable A PR needs a backport to the stable release. label Dec 27, 2019
@d-xo
Copy link
Contributor Author

d-xo commented Dec 27, 2019

@Ma27 it would also be good to run the wireguard-namespaces and systemd-networkd-wireguard tests for this change.

@zx2c4
Copy link
Contributor

zx2c4 commented Dec 27, 2019

@Ma27 it would also be good to run the wireguard-namespaces and systemd-networkd-wireguard tests for this change.

At the source level, the only change to the wg(8) command is:

diff --git a/src/wg.c b/src/wg.c
index 7b5d3af6..dc6dda4b 100644
--- a/src/wg.c
+++ b/src/wg.c
@@ -8,6 +8,7 @@
 #include <string.h>
 
 #include "subcommands.h"
+#include "version.h"
 
 const char *PROG_NAME;
 
@@ -40,6 +41,10 @@ int main(int argc, char *argv[])
 {
 	PROG_NAME = argv[0];
 
+	if (argc == 2 && (!strcmp(argv[1], "-v") || !strcmp(argv[1], "--version") || !strcmp(argv[1], "version"))) {
+		printf("wireguard-tools v%s - https://git.zx2c4.com/wireguard-tools/\n", WIREGUARD_TOOLS_VERSION);
+		return 0;
+	}
 	if (argc == 2 && (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help") || !strcmp(argv[1], "help"))) {
 		show_usage(stdout);
 		return 0;

@ofborg ofborg bot requested review from Mic92, elseym and globin December 27, 2019 16:59
@d-xo
wireguard: 0.0.20191219 -> 0.0.20191226
9681bc7 
the wireguard kernel module has been pulled out into it's own repo as
announced in this mailing list post:

https://lists.zx2c4.com/pipermail/wireguard/2019-December/004796.html
@d-xo
wireguard-tools: 0.0.20191219 -> 1.0.20191226
d6be252 
the userspace wireguard tools have been pulled out into their own repo as
announced in this mailing list post:

https://lists.zx2c4.com/pipermail/wireguard/2019-December/004787.html
@d-xo d-xo force-pushed the wireguard-tools-1.0.20191226 branch from 6332c4a to d6be252 Compare December 27, 2019 20:03
@ofborg ofborg bot requested a review from Ma27 December 27, 2019 20:26
zx2c4
zx2c4 reviewed Dec 27, 2019
View reviewed changes
pkgs/os-specific/linux/wireguard/default.nix

# module requires Linux >= 3.10 https://www.wireguard.io/install/#kernel-requirements
assert stdenv.lib.versionAtLeast kernel.version "3.10";
# wireguard upstreamed since 5.6 https://lists.zx2c4.com/pipermail/wireguard/2019-December/004704.html
assert stdenv.lib.versionOlder kernel.version "5.6";
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "assert" the logic we actually want? In 5.6, the module will already be provided. Maybe the best action in that case is to make the install/build part of this a no-op, or similar?

zx2c4
zx2c4 approved these changes Dec 29, 2019
View reviewed changes
Copy link
Contributor

@zx2c4 zx2c4 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@d-xo d-xo force-pushed the wireguard-tools-1.0.20191226 branch from 4f07282 to d6be252 Compare December 29, 2019 21:49
@ofborg ofborg bot requested a review from Ma27 December 29, 2019 22:21
Ma27
Ma27 approved these changes Dec 30, 2019
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

Changes are overall fine. @zx2c4 and I had a short chat in #nixos where we agreed that the question whether or not to use an assertion for linux 5.6 isn't a blocker here (sorry for delaying the merge btw!).

The kernel isn't even released yet, so if people prefer a different solution, it can be still changed later.

@Ma27 Ma27 merged commit 646c641 into NixOS:master Dec 30, 2019
@Ma27
Copy link
Member

Ma27 commented Dec 30, 2019

Thanks! Backported as 143b2ff, 3fdb468.

@Ma27 Ma27 added 8.has: port to stable A PR already has a backport to the stable release. and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Dec 30, 2019
@d-xo
Copy link
Contributor Author

d-xo commented Dec 30, 2019 •
edited

Hey! Apologies for the delay, I've been traveling the last few days with very patchy internet.

Looking at how the linux modules get added in all-packages.nix (with linuxPackagesFor), I think the assert will probably end up breaking stuff when 5.6 actually lands, so @zx2c4's suggestion of a noop makes a lot of sense. @Ma27 If you agree I'll open a new PR in a day or two addressing that.

@Ma27
Copy link
Member

Ma27 commented Dec 30, 2019

After looking at the expression: the standard pattern for that seems to be sth. like this:

{
    wireguard = if stdenv.lib.versionOlder kernel.version "5.6" then callPackage ../os-specific/linux/wireguard { } else null;
}

@d-xo
Copy link
Contributor Author

d-xo commented Dec 30, 2019

Thanks!

@d-xo d-xo mentioned this pull request Dec 30, 2019
Merged
10 tasks
@d-xo
Copy link
Contributor Author

d-xo commented Dec 30, 2019

Done in #76722

@d-xo d-xo deleted the wireguard-tools-1.0.20191226 branch January 8, 2020 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zx2c4 zx2c4 zx2c4 approved these changes

@Ma27 Ma27 Ma27 approved these changes

@Mic92 Mic92 Awaiting requested review from Mic92

@elseym elseym Awaiting requested review from elseym

@globin globin Awaiting requested review from globin

Assignees
No one assigned
Labels
8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@d-xo @zx2c4 @Ma27