OpenVPN / tap-windows6

Windows TAP driver (NDIS 6)
Other
765 stars 238 forks source link

MSM: Fix WiX compilation dependency #131

Closed rozmansi closed 3 years ago

rozmansi commented 3 years ago

As installer.wix is using $(var.PRODUCT_...) properties defined in config.props which is auto-generated from config.props.in, the installer.wix MUST be recompiled each time config.props is updated (e.g. version change).

As installer.wix is using $(var.INSTALLERLIBRARY...) properties calculated at runtime based on installer.dll, the installer.wix MUST be recompiled each time installer.dll is updated.

lstipakov commented 3 years ago

Just discovered the same - updating "version.m4" didn't trigger MSM rebuild:

WixCompile:
Skipping target "WixCompile" because all output files are up-to-date with respect to the input files.

Adding config.props to Inputs fixes the problem:

WixCompile:
  "C:\Program Files (x86)\WiX Toolset v3.11\bin\candle.exe"  -dPRODUCT_NAME="TAP-Windows" -dPRODUCT_PUBLISHER="OpenVPN Technologies, Inc." -dPRODUCT_VERSION="9.24.66" -dPRODUCT_TAP_WIN_COMPONENT_ID="tap0901" -nologo -arch x86 -dPLATFORM
  =Win32 -sw1086 -dINSTALLER_LIBRARY_HASH=A01853A7B8A24F05A9323684DDBD996DC94A6680F55446A4FC70EFDC5ACA1D25 -dINSTALLER_LIBRARY_TIME=637371438956372306 -out "Release\installer.wixobj" installer.wxs
  installer.wxs
rozmansi commented 3 years ago

@mattock: I suggest you bump the build number to 602 and repackage with buildtap.py -m [--sdk=wdk].

selvanair commented 3 years ago

@mattock: I suggest you bump the build number to 602 and repackage with buildtap.py -m [--sdk=wdk].

The version that is compared in Newer() in msi.c appears to be the one taken from PRODUCT_VERSION which does not include the build number. We've to either include the build number into PRODUCT_VERSION or construct the msm version as major.minor.revision.build.

lstipakov commented 3 years ago

Cannot we just bump revision?

diff --git a/version.m4 b/version.m4
index 39ff337..b23b851 100644
--- a/version.m4
+++ b/version.m4
@@ -1,12 +1,12 @@
 dnl define the TAP version
 define([PRODUCT_NAME], [TAP-Windows])
 define([PRODUCT_PUBLISHER], [OpenVPN Technologies, Inc.])
-define([PRODUCT_VERSION], [9.24.5])
-define([PRODUCT_VERSION_RESOURCE], [9,24,5,601])
+define([PRODUCT_VERSION], [9.24.6])
+define([PRODUCT_VERSION_RESOURCE], [9,24,6,601])
 define([PRODUCT_TAP_WIN_COMPONENT_ID], [tap0901])
 define([PRODUCT_TAP_WIN_MAJOR], [9])
 define([PRODUCT_TAP_WIN_MINOR], [24])
-define([PRODUCT_TAP_WIN_REVISION], [5])
+define([PRODUCT_TAP_WIN_REVISION], [6])
 define([PRODUCT_TAP_WIN_BUILD], [601])
 define([PRODUCT_TAP_WIN_PROVIDER], [TAP-Windows Provider V9])
 define([PRODUCT_TAP_WIN_CHARACTERISTICS], [0x1])
selvanair commented 3 years ago

We can but anything we do here will make the version look inconsistent with the version in the driver. I'm assuming mattock doesn't want to go through resigning the driver which now has version=9.24.5.601. Changing 601 to 602 without re-building the driver looks like a better compromise than changing 5 to 6.

selvanair commented 3 years ago

We can but anything we do here will make the version look inconsistent with the version in the driver. I'm assuming mattock doesn't want to go through resigning the driver which now has version=9.24.5.601. Changing 601 to 602 without re-building the driver looks like a better compromise than changing 5 to 6.

Actually, why increase the version number at all? Rebuilding msm will get 9.24.5 which is higher than the wrong 9.24.4 in rc2 and the driver already has the right version. Only the installer filename needs to be updated.

rozmansi commented 3 years ago

Actually, why increase the version number at all? Rebuilding msm will get 9.24.5 which is higher than the wrong 9.24.4 in rc2 and the driver already has the right version. Only the installer filename needs to be updated.

True. But then you get an inconsistency with the installer filename and its content.

What's the deal with https://build.openvpn.net/downloads/releases/ download repo? Can you just replace existing MSMs? After all, building form a clean tree would produce exactly the same MSMs as they would be now that this dependency was fixed.

mattock commented 3 years ago

While MSM build uses build.openvpn.net (no caching), I'd need to update the MSMs on the main download server as well, which is S3 with two layers of caching in front (Cloudflare and Cloudfront). Those two caches for all the changed files and their GPG signatures, which is painful, error-prone and annoying. The cache clearing also propagates slowly to Cloudflare regional endpoints, so people may, for a time, get different mismatching versions of the MSIs and/or their GPG signatures. Then I get the complaints when somebody for whatever reason downloads the MSM from the main download server and it does not match our expectations.

I'm inclined to just add "-2" or something to the MSM filename to avoid the above issues and the slow and painful resigning process.