diff mbox series

[OpenWrt-Devel,v2] wireguard-tools: fix version indicator

Message ID 20200511113702.10975-1-fe@dev.tdt.de
State Rejected
Delegated to: Petr Štetiar
Headers show
Series [OpenWrt-Devel,v2] wireguard-tools: fix version indicator | expand

Commit Message

Florian Eckert May 11, 2020, 11:37 a.m. UTC
If we execute `wg --version` we get a diffrent version string that does
not match with the version string in the openwrt makefile.

Current version string:
`wireguard-tools vreboot-13159-gac5caa2718 -https://git.zx2c4.com/wireguard-tools/`

Corrected versions string:
`wireguard-tools v1.0.20200319 - https://git.zx2c4.com/wireguard-tools/`

This had already led to confusion! To fix this, the version string of
the wireguard source code include file 'src/version.h' must be used. This
is achieved by removing version string generation from the
wireguard-tools src/Makefile during build.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---

v2: Update commit message

 package/network/utils/wireguard-tools/Makefile      |  2 +-
 .../patches/0001-wireguard-tools-fix-version.patch  | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 package/network/utils/wireguard-tools/patches/0001-wireguard-tools-fix-version.patch

Comments

Petr Štetiar May 11, 2020, 2:25 p.m. UTC | #1
Florian Eckert <fe@dev.tdt.de> [2020-05-11 13:37:02]:

Hi,

> If we execute `wg --version` we get a diffrent version string that does
> not match with the version string in the openwrt makefile.
> 
> Current version string:
> `wireguard-tools vreboot-13159-gac5caa2718 -https://git.zx2c4.com/wireguard-tools/`
> 
> Corrected versions string:
> `wireguard-tools v1.0.20200319 - https://git.zx2c4.com/wireguard-tools/`

ok, but I fail to see why this patch should be maintained by OpenWrt, this
looks like it should be fixed upstream. Thanks.

-- ynezz
Paul Fertser May 11, 2020, 2:43 p.m. UTC | #2
Hello,

I'm not talking about this specific patch but rather about the general
problem I'm aware of, in this specific case it might be not
applicable.

On Mon, May 11, 2020 at 04:25:42PM +0200, Petr Štetiar wrote:
> > If we execute `wg --version` we get a diffrent version string that does
> > not match with the version string in the openwrt makefile.
> >
> > Current version string:
> > `wireguard-tools vreboot-13159-gac5caa2718 -https://git.zx2c4.com/wireguard-tools/`

This is clearly the git tag from OpenWrt tree itself.

> ok, but I fail to see why this patch should be maintained by OpenWrt, this
> looks like it should be fixed upstream. Thanks.

Many projects when cloned from a git tree use some variation of "git
describe" to embed a meaningful version in the compiled
binary. OpenWrt kind of breaks the assumptions because it first does a
git clone but then packs an archive with all the sources but not the
.git directory. Of course it makes sense to save space on servers and
buildbots and whereever the downloads are stored. However, without
.git directory the command run by a particular build system will often
traverse to the upper directories until it finds the OpenWrt tree
.git.

Probably the right way to solve this would be to have means to
override the default git describe behaviour (and force specific
version string instead) by a configure (or similar) flag. That would
require collaboration with upstream but will also need some additional
tweaks to the OpenWrt package Makefile.

--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com
Paul Fertser May 11, 2020, 3:53 p.m. UTC | #3
On Mon, May 11, 2020 at 05:43:56PM +0300, Paul Fertser wrote:
> Probably the right way to solve this would be to have means to
> override the default git describe behaviour (and force specific
> version string instead) by a configure (or similar) flag.

Another (probably sick) idea: after cloning from git OpenWrt can
remove original .git/ and add to the archive a fake one with just a
single object and tag. I've just checked and it seems to have "git
describe --tags" working it's enough to add just three files:

refs/tags/<versionstring>
HEAD
objects/xx/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

First two files with identical content, the last should be a valid
object of type "commit" (no need to store a tree object even though
it's referenced from it).

To make "git rev-parse --verify --short HEAD" work as expected too one
can preserve just the HEAD commit object (not a tree). This way the
space requirements to store additional data inside the source archives
will be really small. This would require moving and unpacking the pack
that's usually created by git fetch --depth=1 and then removing all
the objects but one. Does this sound like a plan?
Florian Eckert May 12, 2020, 11:22 a.m. UTC | #4
Hello Paul

Thank you for your thoughts.

>> ok, but I fail to see why this patch should be maintained by OpenWrt, 
>> this
>> looks like it should be fixed upstream. Thanks.

I think this is not possible every project does have his own rules.
I only noticed this with wireguard so I will send the changes upstream 
to wireguard.

> Probably the right way to solve this would be to have means to
> override the default git describe behaviour (and force specific
> version string instead) by a configure (or similar) flag. That would
> require collaboration with upstream but will also need some additional
> tweaks to the OpenWrt package Makefile.

My v3 patch does fix this in the wireguard-tools,
but I think we could export this variable during building of an package 
on Openwrt.
For example `export GIT_DIR="$(PKG_BUILD_DIR)/.git" in 
"include/package.mk" to pin the git dir for this package.

Regards

Florian
Petr Štetiar May 12, 2020, 4:39 p.m. UTC | #5
Paul Fertser <fercerpav@gmail.com> [2020-05-11 17:43:56]:

Hi,

> On Mon, May 11, 2020 at 04:25:42PM +0200, Petr Štetiar wrote:
> > > If we execute `wg --version` we get a diffrent version string that does
> > > not match with the version string in the openwrt makefile.
> > >
> > > Current version string:
> > > `wireguard-tools vreboot-13159-gac5caa2718 -https://git.zx2c4.com/wireguard-tools/`
> 
> This is clearly the git tag from OpenWrt tree itself.

Indeed.

> > ok, but I fail to see why this patch should be maintained by OpenWrt, this
> > looks like it should be fixed upstream. Thanks.
> 
> Many projects when cloned from a git tree use some variation of "git
> describe" to embed a meaningful version in the compiled binary. 

OpenWrt is using WireGuard release tarballs.

-- ynezz
diff mbox series

Patch

diff --git a/package/network/utils/wireguard-tools/Makefile b/package/network/utils/wireguard-tools/Makefile
index 549329509a..4ccb6dd2cf 100644
--- a/package/network/utils/wireguard-tools/Makefile
+++ b/package/network/utils/wireguard-tools/Makefile
@@ -12,7 +12,7 @@  include $(INCLUDE_DIR)/kernel.mk
 PKG_NAME:=wireguard-tools
 
 PKG_VERSION:=1.0.20200319
-PKG_RELEASE:=1
+PKG_RELEASE:=2
 
 PKG_SOURCE:=wireguard-tools-$(PKG_VERSION).tar.xz
 PKG_SOURCE_URL:=https://git.zx2c4.com/wireguard-tools/snapshot/
diff --git a/package/network/utils/wireguard-tools/patches/0001-wireguard-tools-fix-version.patch b/package/network/utils/wireguard-tools/patches/0001-wireguard-tools-fix-version.patch
new file mode 100644
index 0000000000..b5ee60ac6d
--- /dev/null
+++ b/package/network/utils/wireguard-tools/patches/0001-wireguard-tools-fix-version.patch
@@ -0,0 +1,13 @@ 
+--- a/src/Makefile
++++ b/src/Makefile
+@@ -46,10 +46,6 @@ CFLAGS += -DRUNSTATEDIR="\"$(RUNSTATEDIR
+ ifeq ($(DEBUG),yes)
+ CFLAGS += -g
+ endif
+-WIREGUARD_TOOLS_VERSION = $(patsubst v%,%,$(shell GIT_CEILING_DIRECTORIES="$(PWD)/../.." git describe --dirty 2>/dev/null))
+-ifneq ($(WIREGUARD_TOOLS_VERSION),)
+-CFLAGS += -D'WIREGUARD_TOOLS_VERSION="$(WIREGUARD_TOOLS_VERSION)"'
+-endif
+ ifeq ($(PLATFORM),haiku)
+ LDLIBS += -lnetwork -lbsd
+ endif