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
 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...
___
WireGuard mailing list
WireGuard@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/wireguard

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

[Daniel Kahn Gillmor]

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


signature.asc
Description: PGP signature
___
WireGuard mailing list
WireGuard@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/wireguard

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?
___
WireGuard mailing list
WireGuard@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/wireguard

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

[Kalin KOZHUHAROV]

Hi Jason,

On Mon, Nov 14, 2016 at 11:28 AM, John Huttley  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 
 #include 

+#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
. 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
. 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 ");
 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.
___
WireGuard mailing list
WireGuard@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/wireguard