[WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[Kalin KOZHUHAROV]

Hi Jason,

On Mon, Nov 14, 2016 at 11:28 AM, John Huttley <john@mib-infotech.co.nz> wrote:
> RFE: when the module loads and prints its test at startup, please print its
> version and compile flags as well.
>

I second that! There is not (yet) a notion of VERSION in the code,
better not wait till 1.0 to put it :-)

One can use the commit from git, but that assumes always build from git sources.
Examples:
GIT_COMMIT := $(shell git log -n 1 --pretty=format:"%h" HEAD 2>/dev/null)
GIT_TAG := $(shell git tag --points HEAD 2>/dev/null)

( NOTE: GIT_TAG is only available if we have a tag defined at current HEAD, e.g.

$ git checkout bebcae1
HEAD is now at bebcae1... chacha20poly1305: cleanup magic constants

$ git tag --points HEAD
experimental-0.0.20161105
)

I'd rather see a VERSION.txt file, or a `VERSION := 0.0.something`
inside Makefile.

Or even directly hardcoded in main.c:

diff --git a/src/main.c b/src/main.c
index e381d09..4991812 100644
--- a/src/main.c
+++ b/src/main.c
@@ -13,6 +13,9 @@
 #include <linux/module.h>
 #include <net/rtnetlink.h>

+#define DRV_VERSION "0.0.20161105"
+const char wireguard_driver_version[] = DRV_VERSION;
+
 static int __init mod_init(void)
 {
        int ret = 0;
@@ -29,25 +32,31 @@ static int __init mod_init(void)
        chacha20poly1305_init();
        noise_init();

+#ifdef CONFIG_WIREGUARD_PARALLEL
        ret = packet_init_data_caches();
        if (ret < 0)
                return ret;
+#endif

        ret = device_init();
        if (ret < 0) {
+#ifdef CONFIG_WIREGUARD_PARALLEL
                packet_deinit_data_caches();
+#endif
                return ret;
        }

-       pr_info("WireGuard loaded. See www.wireguard.io for information.\n");
-       pr_info("(C) Copyright 2015-2016 Jason A. Donenfeld
<Jason@zx2c4.com>. All Rights Reserved.\n");
+       pr_info("WireGuard %s loaded. See www.wireguard.io for
information.\n", wireguard_driver_version);
+       pr_info("Copyright (c) 2015-2016 Jason A. Donenfeld
<Jason@zx2c4.com>. All rights reserved.\n");
        return ret;
 }

 static void __exit mod_exit(void)
 {
        device_uninit();
+#ifdef CONFIG_WIREGUARD_PARALLEL
        packet_deinit_data_caches();
+#endif
        pr_debug("WireGuard has been unloaded\n");
 }

@@ -57,3 +66,4 @@ MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Simple, secure, and speedy VPN tunnel");
 MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
 MODULE_ALIAS_RTNL_LINK(KBUILD_MODNAME);
+MODULE_VERSION(DRV_VERSION);


Not sure if const char casting is needed (was copying from e1000e driver) ;-/

I changed slightly the Copyright pr_info, actually I guess you should
remove "All  rights reserved." altogether from it (but keep it in the
source).

Cheers,
Kalin.

Re: [WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[Jason A. Donenfeld]

The best I could come with:

> WIREGUARD_VERSION := $(shell parent_name=$$(readlink -f .. | sed -n 's:.*/[wW]ire[Gg]uard[a-z-]*-\([0-9.]\+\)$$:\1:p');  if [ -d ../.git ]; then echo "git-$$(git rev-parse --shor
t HEAD)"; elif [ -n $parent_name ]; then echo "$$parent_name"; else
echo "unknown"; fi)

Is this reliable, or is this garbage?

Re: [WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[Jason A. Donenfeld]

Trying again with no line breaks:

> WIREGUARD_VERSION :=3D $(shell parent_name=3D$$(readlink -f .. | sed -n '=
s:.*/[wW]ire[Gg]uard[a-z-]*-\([0-9.]\+\)$$:\1:p');  if [ -d ../.git ]; then=
 echo "git-$$(git rev-parse --short HEAD)"; elif [ -n $parent_name ]; then =
echo "$$parent_name"; else echo "unknown"; fi)

Re: [WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[Kalin KOZHUHAROV]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

On Thu, Nov 17, 2016 at 2:40 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Trying again with no line breaks:
>
>> WIREGUARD_VERSION := $(shell parent_name=$$(readlink -f .. | sed -n 's:.*/[wW]ire[Gg]uard[a-z-]*-\([0-9.]\+\)$$:\1:p');  if [ -d ../.git ]; then echo "git-$$(git rev-parse --short HEAD)"; elif [ -n $parent_name ]; then echo "$$parent_name"; else echo "unknown"; fi)
>

I couldn't make it work as meant to, fails (=prints empty string) when
I rename .git to .git- for example.

Here is it on 2 lines.

[-- Attachment #2: Makefile --]
[-- Type: application/octet-stream, Size: 314 bytes --]

parent_name := $(readlink -f .. | sed -n 's:.*/[wW]ire[Gg]uard[a-z-]*-\([0-9.]\+\)$$:\1:p')
WIREGUARD_VERSION := $(shell if [ -d ../.git ]; then echo "git-$$(git rev-parse --short HEAD)"; elif [ "x$(parent_name)" == "x" ]; then echo "$(parent_name)"; else echo "unknown"; fi)

version:
	@echo $(WIREGUARD_VERSION)

Re: [WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

On Thu 2016-11-17 02:40:30 +0900, Jason A. Donenfeld wrote:
> Trying again with no line breaks:
>
>> WIREGUARD_VERSION := $(shell parent_name=$$(readlink -f .. | sed -n 's:.*/[wW]ire[Gg]uard[a-z-]*-\([0-9.]\+\)$$:\1:p');  if [ -d ../.git ]; then echo "git-$$(git rev-parse --short HEAD)"; elif [ -n $parent_name ]; then echo "$$parent_name"; else echo "unknown"; fi)

Please don't assume that the source code is built from a git repository.
On debian, we build from the tarball, which is extracted from the git
repo, and we have nothing for "git rev-parse" to draw from.

For the cost of one extra commit just before tagging, you could populate
a version.txt file and this then becomes $(shell cat version.txt).

simplicity, simplicity :)

            --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

Re: [WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[Jason A. Donenfeld]

On Wed, Nov 16, 2016 at 10:01 PM, Daniel Kahn Gillmor
<dkg@fifthhorseman.net> wrote:
> On Thu 2016-11-17 02:40:30 +0900, Jason A. Donenfeld wrote:
>> Trying again with no line breaks:
>>
> Please don't assume that the source code is built from a git repository.
> On debian, we build from the tarball, which is extracted from the git
> repo, and we have nothing for "git rev-parse" to draw from.

That's why my horrible one liner falls back on getting the version
from the parent directory... Not pretty.

>
> For the cost of one extra commit just before tagging, you could populate
> a version.txt file and this then becomes $(shell cat version.txt).

Yea most likely this is what will wind up happening...

Re: [WireGuard] RFE: A notion of VERSION (was: Debugging AllowedIps)

[Jason A. Donenfeld]

https://git.zx2c4.com/WireGuard/commit/?id=258d0b33a5087558447b319273c37c89445d3ae0

There we go. My release script looks like this:

zx2c4@thinkpad ~/Projects/WireGuard $ cat maint/release.sh
#!/bin/sh
set -ex
cd "$(dirname "$(readlink -f "$0")")/.."
date="$(date +%Y%m%d)"
tag="experimental-0.0.$date"
git reset --hard
sed -i "s/\".*\"/\"$tag\"/" src/version.h
git update-index --no-assume-unchanged src/version.h
git add src/version.h
git commit -m "version: bump snapshot"
git tag "$tag" --sign -m "Experimental snapshot $date"