{"thread":{"id":"56702","subject":"[PATCH] ci(windows): ensure that we do not pick up random executables","startedAt":"2021-10-13T12:55:53Z","lastAt":"2021-10-13T23:49:32Z","messageCount":4,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"438646","messageId":"pull.1057.git.1634129748874.gitgitgadget@gmail.com","threadId":"56702","inReplyTo":null,"subject":"[PATCH] ci(windows): ensure that we do not pick up random executables","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-10-13T12:55:48Z","receivedAt":"2021-10-13T12:55:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nOn the Windows build agents, a lot of programs are installed, and added\nto the PATH automatically.\n\nOne such program is Git for Windows, and due to the way it is set up,\nunfortunately its copy of `gpg.exe` is also reachable via the PATH.\n\nThis usually does not pose any problems. To the contrary, it even allows\nus to test the GPG parts of Git's test suite even if `gpg.exe` is not\ndelivered as part of `git-sdk-64-minimal`, the minimal subset of Git for\nWindows' SDK that we use in the CI builds to compile Git.\n\nHowever, every once in a while we build a new MSYS2 runtime, which means\nthat there is a mismatch between the copy in `git-sdk-64-minimal` and\nthe copy in C:\\Program Files\\Git\\usr\\bin. When that happens we hit the\ndreaded problem where only one `msys-2.0.dll` is expected to be in the\nPATH, and things start to fail.\n\nLet's avoid all of this by restricting the PATH to the minimal set. This\nis actually done by `git-sdk-64-minimal`'s `/etc/profile`, and we just\nhave to source this file manually (one would expect that it is sourced\nautomatically, but the Bash steps in Azure Pipelines/GitHub workflows\nare explicitly run using `--noprofile`, hence the need for doing this\nexplicitly).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    ci(windows): avoid using external gpg by mistake\n    \n    I stumbled over a CI failure this morning when I scrambled to tie up all\n    the loose ends for an unexpected v2.33.1 release. Turns out that the CI\n    picked up the installed Git for Windows' gpg.exe, and due to a (new, as\n    of this morning) mismatch in the MSYS2 runtime between the installed Git\n    for Windows and the subset of the Git for Windows SDK used for compiling\n    in the CI runs, it worked just enough to pass the prereq, but then\n    failed the tests.\n    \n    Seeing as essentially every single CI build will fail without this patch\n    right up until the point in time when Git for Windows is upgraded in the\n    build agents (to a version that does yet exist), it would be good to\n    fast-track this to maint.\n    \n    Note: I based this on the earliest topic where it would apply without\n    merge conflicts, js/ci-windows-update (which is unfortunately quite\n    recent, it is not reachable from any version older than v2.33.0).\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1057%2Fdscho%2Fwork-around-windows-ci-failures-with-gpg-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1057/dscho/work-around-windows-ci-failures-with-gpg-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1057\n\n .github/workflows/main.yml | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 0f7516c9ef3..2a6d68718ae 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -88,7 +88,7 @@ jobs:\n       env:\n         HOME: ${{runner.workspace}}\n         NO_PERL: 1\n-      run: ci/make-test-artifacts.sh artifacts\n+      run: . /etc/profile && ci/make-test-artifacts.sh artifacts\n     - name: zip up tracked files\n       run: git archive -o artifacts/tracked.tar.gz HEAD\n     - name: upload tracked files and build artifacts\n@@ -115,7 +115,7 @@ jobs:\n     - uses: git-for-windows/setup-git-for-windows-sdk@v1\n     - name: test\n       shell: bash\n-      run: ci/run-test-slice.sh ${{matrix.nr}} 10\n+      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n     - name: ci/print-test-failures.sh\n       if: failure()\n       shell: bash\n@@ -199,7 +199,7 @@ jobs:\n       env:\n         NO_SVN_TESTS: 1\n         GIT_TEST_SKIP_REBASE_P: 1\n-      run: ci/run-test-slice.sh ${{matrix.nr}} 10\n+      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n     - name: ci/print-test-failures.sh\n       if: failure()\n       shell: bash\n\nbase-commit: d681d0dc3a77016caa7e26abfe734afbdab44de5\n-- \ngitgitgadget\n"},{"id":"438669","messageId":"xmqqpms8hlop.fsf@gitster.g","threadId":"56702","inReplyTo":"pull.1057.git.1634129748874.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(windows): ensure that we do not pick up random executables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-13T17:35:50Z","receivedAt":"2021-10-13T17:35:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">     Note: I based this on the earliest topic where it would apply without\n>     merge conflicts, js/ci-windows-update (which is unfortunately quite\n>     recent, it is not reachable from any version older than v2.33.0).\n\nSo do you want this forked from v2.33.0 (and merged to 'maint' so\nthat v2.33.2, if we need to issue it, would have it)?\n\nhttps://github.com/git/git/actions/runs/1334961399 is the CI/PR run\non v2.33.1 from yesterday.  Our PATH wasn't contaminated and we\ndidn't see the problem you fixed here by mere luck, and you were\nunlucky when you ran the same for generating your release material?\n\nThanks.\n\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1057/dscho/work-around-windows-ci-failures-with-gpg-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1057\n>\n>  .github/workflows/main.yml | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 0f7516c9ef3..2a6d68718ae 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -88,7 +88,7 @@ jobs:\n>        env:\n>          HOME: ${{runner.workspace}}\n>          NO_PERL: 1\n> -      run: ci/make-test-artifacts.sh artifacts\n> +      run: . /etc/profile && ci/make-test-artifacts.sh artifacts\n>      - name: zip up tracked files\n>        run: git archive -o artifacts/tracked.tar.gz HEAD\n>      - name: upload tracked files and build artifacts\n> @@ -115,7 +115,7 @@ jobs:\n>      - uses: git-for-windows/setup-git-for-windows-sdk@v1\n>      - name: test\n>        shell: bash\n> -      run: ci/run-test-slice.sh ${{matrix.nr}} 10\n> +      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n>      - name: ci/print-test-failures.sh\n>        if: failure()\n>        shell: bash\n> @@ -199,7 +199,7 @@ jobs:\n>        env:\n>          NO_SVN_TESTS: 1\n>          GIT_TEST_SKIP_REBASE_P: 1\n> -      run: ci/run-test-slice.sh ${{matrix.nr}} 10\n> +      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n>      - name: ci/print-test-failures.sh\n>        if: failure()\n>        shell: bash\n>\n> base-commit: d681d0dc3a77016caa7e26abfe734afbdab44de5\n"},{"id":"438701","messageId":"nycvar.QRO.7.76.6.2110132353160.395@tvgsbejvaqbjf.bet","threadId":"56702","inReplyTo":"xmqqpms8hlop.fsf@gitster.g","subject":"Re: [PATCH] ci(windows): ensure that we do not pick up random executables","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-10-13T22:01:55Z","receivedAt":"2021-10-13T22:02:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 13 Oct 2021, Junio C Hamano wrote:\n\n> >     Note: I based this on the earliest topic where it would apply without\n> >     merge conflicts, js/ci-windows-update (which is unfortunately quite\n> >     recent, it is not reachable from any version older than v2.33.0).\n>\n> So do you want this forked from v2.33.0 (and merged to 'maint' so\n> that v2.33.2, if we need to issue it, would have it)?\n\nWell, I followed your custom and based it on the oldest applicable topic,\njs/ci-windows-update. My preference would be for this to be merged into\n`maint` rather sooner than later, so that GitGitGadget PRs could target\n`maint` and get a meaningful test coverage.\n\n> https://github.com/git/git/actions/runs/1334961399 is the CI/PR run\n> on v2.33.1 from yesterday.  Our PATH wasn't contaminated and we\n> didn't see the problem you fixed here by mere luck, and you were\n> unlucky when you ran the same for generating your release material?\n\nAt the time this ran, the `msys2-runtime` package was already upgraded to\na slightly newer version, and even deployed to Git for Windows' Pacman\nrepository. However, that change was not yet reflected in the `git-sdk-64`\nrepository (which is a Git mirror, tracking all the Pacman packages that\nmake up Git for Windows' SDK), as this repository is updated by a nightly\nautomated build.\n\nAs a consequence, the `git-sdk-64-minimal` build had not run, therefore\nyour build picked up the previous one, with the earlier `msys2-runtime`\nversion.\n\nThat means that the `msys-2.0.dll` file contained in `git-sdk-64-minimal`\nwas identical to the one in Git for Windows as installed on GitHub's build\nagents.\n\nSo yes, the PATH was \"contaminated\". Meaning: the test suite run did pick\nup the `gpg.exe` from Git for Windows. And since there was no\n`msys-2.0.dll` mismatch, it did its job as intended.\n\nThe problem arose when I scrambled to get all the things I wanted to wrap\nup before the next Git for Windows release, this past morning, at which\nstage the `git-sdk-64-minimal` had already picked up that new\n`msys2-runtime` version. As a consequence, the MSYS2 Bash we use to run\nGit's test suite in the CI builds had a `msys-2.0.dll` that was different\nfrom the one in `C:\\Program Files\\Git\\usr\\bin` (which lives next to the\n`gpg.exe` and is therefore used when running that program). That, in turn,\nprevented certain functionality from working. Which made the GPG-related\ntests fail (maybe not even all of them).\n\nCiao,\nDscho\n"},{"id":"438716","messageId":"xmqqily0eb93.fsf@gitster.g","threadId":"56702","inReplyTo":"nycvar.QRO.7.76.6.2110132353160.395@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci(windows): ensure that we do not pick up random executables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-13T23:49:28Z","receivedAt":"2021-10-13T23:49:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> So do you want this forked from v2.33.0 (and merged to 'maint' so\n>> that v2.33.2, if we need to issue it, would have it)?\n>\n> Well, I followed your custom and based it on the oldest applicable topic,\n> js/ci-windows-update. My preference would be for this to be merged into\n> `maint` rather sooner than later, so that GitGitGadget PRs could target\n> `maint` and get a meaningful test coverage.\n\nYes, will do.  Tonight's pushout did not have it in 'next', but I do\nnot think of a reason why we shouldn't fast-track the topic.\n\nThanks.\n"}]}