From 73cac88edb77a96c940f9ac008a30d04d7323e65 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 22 Aug 2024 10:15:45 +0200 Subject: [PATCH 1/6] ci: verify `git update-git-for-windows`'s version comparison The script has an option to run a self-check; Let's make this part of the CI build. Signed-off-by: Johannes Schindelin --- .github/workflows/main.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7dfc07554b..0edeb182be 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -297,7 +297,8 @@ jobs: checklist=installer/run-checklist.sh && # cannot test SSH keys in read-only mode, skip test for now sed -i 's|git@ssh.dev.azure.com:v3/git-for-windows/git/git|https://github.com/git/git|' $checklist && - sh -x $checklist + sh -x $checklist && + git update-git-for-windows --test-version-compare check-for-missing-dlls: needs: determine-packages if: needs.determine-packages.outputs.test-sdk-artifacts == 'true' || needs.determine-packages.outputs.check-for-missing-dlls == 'true' From ce4d4e06563c3d82a70b5b1c7cff02bc18dae1d2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 22 Aug 2024 13:26:46 +0200 Subject: [PATCH 2/6] installer: extend `VersionCompare()` to handle -rc versions, too It is uncommon that somebody tries to downgrade to an -rc version, but it does happen from time to time. Signed-off-by: Johannes Schindelin --- installer/install.iss | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/installer/install.iss b/installer/install.iss index 00e7dac67d..2f2d0db366 100644 --- a/installer/install.iss +++ b/installer/install.iss @@ -917,6 +917,22 @@ begin Result:=-1; end; +function IsRCVersion(var Pos:Integer;Version:String):Boolean; +begin + if (Pos+2<=Length(Version)) and + ((Copy(Version,Pos,3)='-rc') or + (Copy(Version,Pos,3)='.rc')) then begin + Pos:=Pos+3; + Result:=True + end else if (Pos+1<=Length(Version)) and + ((Copy(Version,Pos,2)='rc') or + (Copy(Version,Pos,2)='rc')) then begin + Pos:=Pos+2; + Result:=True + end else + Result:=False; +end; + function VersionCompare(CurrentVersion,PreviousVersion:String):Integer; var i,j,Current,Previous:Integer; @@ -960,16 +976,31 @@ begin Exit; end; if j>Length(PreviousVersion) then begin - if i<=Length(CurrentVersion) then - Result:=+1; + if i<=Length(CurrentVersion) then begin + // an -rc version is considered "smaller" + if IsRCVersion(i,CurrentVersion) then + Result:=-1 + else + Result:=+1; + end; Exit; end; if i>Length(CurrentVersion) then begin - Result:=-1; + // an -rc version is considered "smaller" + if IsRCVersion(j,PreviousVersion) then + Result:=+1 + else + Result:=-1; Exit; end; if CurrentVersion[i]<>PreviousVersion[j] then begin - if PreviousVersion[j]='.' then + if IsRCVersion(i,CurrentVersion) then begin + if IsRCVersion(j,PreviousVersion) then + Continue; + Result:=-1 + end else if IsRCVersion(j,PreviousVersion) then + Result:=+1 + else if PreviousVersion[j]='.' then Result:=-1 else Result:=+1; From 9964d60264460893fd8fb9347fa89af64855603c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 22 Aug 2024 13:29:21 +0200 Subject: [PATCH 3/6] installer: implement a self-check Currently, this only validates that the `VersionCompare()` function works as intended. To that end, the test cases from the corresponding self-check in `git update-git-for-windows` have been transmogrified, and two new options have been added to `installer/release.sh`: --self-check This produces an installer that does not do anything but run the tests and exit with a non-zero exit code in case there were failures, otherwise exit code 0. --include-self-check This produces an installer that first runs the tests, and exits with exit code 1 if any test failed. If no test failed, it continues to install Git for Windows. This mode is useful for CI, where we want to run the self checks and also install Git for Windows in order to validate the installation. Signed-off-by: Johannes Schindelin --- installer/install.iss | 40 ++++++++++++++++++++++++++++++++++++++++ installer/release.sh | 11 +++++++++++ 2 files changed, 51 insertions(+) diff --git a/installer/install.iss b/installer/install.iss index 2f2d0db366..6368b61720 100644 --- a/installer/install.iss +++ b/installer/install.iss @@ -1128,11 +1128,51 @@ begin WizardForm.NextButton.Caption:=SetupMessage(msgButtonNext); end; +function SelfCheckVersionCompare(A,B:String;ExpectedOutcome:Integer):Boolean; +var + ActualOutcome:Integer; +begin + ActualOutcome:=VersionCompare(A,B); + if ((ActualOutcome=0) and (ExpectedOutcome=0)) or + ((ActualOutcome<0) and (ExpectedOutcome<0)) or + ((ActualOutcome>0) and (ExpectedOutcome>0)) then + Result:=True + else + LogError('VersionCompare('+A+','+B+')='+IntToStr(VersionCompare(A,B))+' but expected '+IntToStr(ExpectedOutcome)); +end; + +procedure SelfCheck; +var + Failed:Boolean; +begin + if not SelfCheckVersionCompare('2.32.0','2.32.1',-1) or + not SelfCheckVersionCompare('2.32.1','2.32.0',1) or + not SelfCheckVersionCompare('2.32.1.vfs.0.0','2.32.0',1) or + not SelfCheckVersionCompare('2.32.1.vfs.0.0','2.32.0.vfs.0.0',1) or + not SelfCheckVersionCompare('2.32.0.vfs.0.1','2.32.0.vfs.0.2',-1) or + not SelfCheckVersionCompare('2.32.0-rc0','2.31.1',1) or + not SelfCheckVersionCompare('2.31.1','2.32.0-rc0',-1) or + not SelfCheckVersionCompare('2.32.0-rc2','2.32.0',-1) or + not SelfCheckVersionCompare('2.32.0-rc2','2.32.0.0',-1) or + not SelfCheckVersionCompare('2.34.0.rc1','2.33.1',1) or + not SelfCheckVersionCompare('2.34.0.rc2','2.34.0',-1) then + Failed:=True; + + if Failed then + ExitProcess(1); +end; + function InitializeSetup:Boolean; var CurrentVersion,Msg:String; ErrorCode:Integer; begin +#ifdef INCLUDE_SELF_CHECK + SelfCheck; +#ifdef EXIT_AFTER_SELF_CHECK + ExitProcess(0); +#endif +#endif UpdateInfFilenames; #if BITNESS=='32' Result:=True; diff --git a/installer/release.sh b/installer/release.sh index f4caa3b4e4..0841fcbebb 100755 --- a/installer/release.sh +++ b/installer/release.sh @@ -69,6 +69,17 @@ do inno_defines="$inno_defines$LF#define OUTPUT_TO_TEMP ''" inno_defines="$inno_defines$LF#define DO_NOT_INSTALL 1" ;; + --include-self-check) + inno_defines="$inno_defines$LF#define INCLUDE_SELF_CHECK 1" + ;; + --self-check) + test_installer=t + skip_files=t + inno_defines="$inno_defines$LF#define OUTPUT_TO_TEMP ''" + inno_defines="$inno_defines$LF#define DO_NOT_INSTALL 1" + inno_defines="$inno_defines$LF#define INCLUDE_SELF_CHECK 1" + inno_defines="$inno_defines$LF#define EXIT_AFTER_SELF_CHECK 1" + ;; --output=*) output_directory="$(cygpath -m "${1#*=}")" || die "Directory inaccessible: '${1#*=}'" From fe6e8df7622131eea1c2a0e8fa19f97f30f95b74 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 22 Aug 2024 13:33:04 +0200 Subject: [PATCH 4/6] ci: run the installer's self-checks Now that we have a mode where internal validations can be run in the installer before installing anything, let's do precisely that in the CI builds (and fail the build if any tests fail). Signed-off-by: Johannes Schindelin --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0edeb182be..91eaa5e50a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -254,7 +254,7 @@ jobs: - name: build installer if: matrix.artifact == 'build-installers' shell: bash - run: ./installer/release.sh --output=$PWD/installer-${{ matrix.bitness }} 0-test + run: ./installer/release.sh --include-self-check --output=$PWD/installer-${{ matrix.bitness }} 0-test - uses: actions/upload-artifact@v4 if: matrix.artifact == 'build-installers' with: From 6973873071adcd78a4aded2506ab3900c7f7dbe1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 23 Aug 2024 10:53:18 +0200 Subject: [PATCH 5/6] installer: fix version comparison bug This is a left-over from long ago, introduced in 490887923b (installer: be prepared for Git tags when comparing versions, 2019-10-30). Reconstructing my memory from that commit, it looks as if my intention was to replace the faulty logic, but I left in that now-useless conditional assignment of `Result` which is immediately overridden. To help future me, let's just remove the unneeded code. Signed-off-by: Johannes Schindelin --- installer/install.iss | 2 -- 1 file changed, 2 deletions(-) diff --git a/installer/install.iss b/installer/install.iss index 6368b61720..e6879fbc81 100644 --- a/installer/install.iss +++ b/installer/install.iss @@ -952,8 +952,6 @@ begin Previous:=NextNumber(PreviousVersion,j); Current:=NextNumber(CurrentVersion,i); if Previous<0 then begin - if Current>=0 then - Result:=+1; Result:=Ord(CurrentVersion[i])-Ord(PreviousVersion[j]); if (Result=0) then begin // skip identical non-numerical characters From 95730846c362fb3da4f99f8197a52a94cfe54c35 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 23 Aug 2024 11:07:43 +0200 Subject: [PATCH 6/6] installer: special-case `.vfs.` versions Installing v2.46.0.vfs.0.0 while the official Git for Windows v2.46.0 is installed constitutes neither an upgrade nor a downgrade. So let's not complain that it is! Signed-off-by: Johannes Schindelin --- installer/install.iss | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/installer/install.iss b/installer/install.iss index e6879fbc81..ff658c3ed9 100644 --- a/installer/install.iss +++ b/installer/install.iss @@ -959,6 +959,9 @@ begin j:=j+1; Continue; end; + // special-case `.vfs.` versions; They are neither downgrades nor upgrades. + if (Copy(CurrentVersion,i,4)='vfs.') or (Copy(PreviousVersion,i,4)='vfs.') then + Result:=0; Exit; end; if Current<0 then begin @@ -1153,7 +1156,8 @@ begin not SelfCheckVersionCompare('2.32.0-rc2','2.32.0',-1) or not SelfCheckVersionCompare('2.32.0-rc2','2.32.0.0',-1) or not SelfCheckVersionCompare('2.34.0.rc1','2.33.1',1) or - not SelfCheckVersionCompare('2.34.0.rc2','2.34.0',-1) then + not SelfCheckVersionCompare('2.34.0.rc2','2.34.0',-1) or + not SelfCheckVersionCompare('git version 2.46.0.vfs.0.0','git version 2.46.0.windows.1',0) then Failed:=True; if Failed then