{"thread":{"id":"66317","subject":"[PATCH] ci: only warn about perforce/git-lfs/JGit on platforms that need them","startedAt":"2026-09-12T14:38:05Z","lastAt":"2026-09-28T16:13:39Z","messageCount":4,"participants":["Harald Nordgren via GitGitGadget","Junio C Hamano","Patrick Steinhardt","Harald Nordgren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"552622","messageId":"pull.2403.git.git.1789223882471.gitgitgadget@gmail.com","threadId":"66317","inReplyTo":null,"subject":"[PATCH] ci: only warn about perforce/git-lfs/JGit on platforms that need them","fromName":"Harald Nordgren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-12T14:38:02Z","receivedAt":"2026-09-12T14:38:05Z","isPatch":true,"body":"From: Harald Nordgren <haraldnordgren@gmail.com>\n\nperforce, git-lfs, and JGit test git's own interop code, not anything\nplatform-specific, so installing them once on ubuntu-* (all three)\nand macos-* (perforce) is enough coverage. debian, i386/ubuntu,\nalpine, fedora and almalinux never install them, yet the presence\ncheck at the end of the script warned on all of them anyway.\n\nScope each check to the platforms that attempt the install, so a\nwarning means one actually failed.\n\nSigned-off-by: Harald Nordgren <haraldnordgren@gmail.com>\n---\n    ci: only warn about perforce/git-lfs/JGit on platforms that need them\n    \n    Only warn about a missing perforce/git-lfs/JGit install on the platforms\n    that actually need and attempt them (ubuntu-*, plus macOS for perforce),\n    since every other platform never installs them and was warning\n    regardless.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2403%2FHaraldNordgren%2Fci-scope-optional-tool-warnings-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2403/HaraldNordgren/ci-scope-optional-tool-warnings-v1\nPull-Request: https://github.com/git/git/pull/2403\n\n ci/install-dependencies.sh | 54 ++++++++++++++++++++++----------------\n 1 file changed, 31 insertions(+), 23 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 2f61fbb07c..a68cec64b4 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -171,30 +171,38 @@ Documentation)\n \t;;\n esac\n \n-if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n-then\n-\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n-\tp4d -V\n-\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n-\tp4 -V\n-else\n-\techo >&2 \"::warning:: perforce wasn't installed, see above for clues why\"\n-fi\n+case \"$distro\" in\n+ubuntu-*|macos-*)\n+\tif type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n+\tthen\n+\t\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n+\t\tp4d -V\n+\t\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n+\t\tp4 -V\n+\telse\n+\t\techo >&2 \"::warning:: perforce wasn't installed, see above for clues why\"\n+\tfi\n+\t;;\n+esac\n \n-if type git-lfs >/dev/null 2>&1\n-then\n-\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n-\tgit-lfs version\n-else\n-\techo >&2 \"::warning:: git-lfs wasn't installed, see above for clues why\"\n-fi\n+case \"$distro\" in\n+ubuntu-*)\n+\tif type git-lfs >/dev/null 2>&1\n+\tthen\n+\t\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n+\t\tgit-lfs version\n+\telse\n+\t\techo >&2 \"::warning:: git-lfs wasn't installed, see above for clues why\"\n+\tfi\n \n-if type jgit >/dev/null 2>&1\n-then\n-\techo \"$(tput setaf 6)JGit Version$(tput sgr0)\"\n-\tjgit version\n-else\n-\techo >&2 \"::warning:: JGit wasn't installed, see above for clues why\"\n-fi\n+\tif type jgit >/dev/null 2>&1\n+\tthen\n+\t\techo \"$(tput setaf 6)JGit Version$(tput sgr0)\"\n+\t\tjgit version\n+\telse\n+\t\techo >&2 \"::warning:: JGit wasn't installed, see above for clues why\"\n+\tfi\n+\t;;\n+esac\n \n end_group \"Install dependencies\"\n\nbase-commit: 47ce80527c56f462cb97db4ca8125342204d3783\n-- \ngitgitgadget\n"},{"id":"553311","messageId":"xmqq1pahupca.fsf@gitster.g","threadId":"66317","inReplyTo":"pull.2403.git.git.1789223882471.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: only warn about perforce/git-lfs/JGit on platforms that need them","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-25T19:02:13Z","receivedAt":"2026-09-25T19:02:16Z","isPatch":true,"body":"\"Harald Nordgren via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Harald Nordgren <haraldnordgren@gmail.com>\n>\n> perforce, git-lfs, and JGit test git's own interop code, not anything\n> platform-specific, so installing them once on ubuntu-* (all three)\n> and macos-* (perforce) is enough coverage. debian, i386/ubuntu,\n> alpine, fedora and almalinux never install them, yet the presence\n> check at the end of the script warned on all of them anyway.\n>\n> Scope each check to the platforms that attempt the install, so a\n> warning means one actually failed.\n>\n> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>\n> ---\n>     ci: only warn about perforce/git-lfs/JGit on platforms that need them\n>     \n>     Only warn about a missing perforce/git-lfs/JGit install on the platforms\n>     that actually need and attempt them (ubuntu-*, plus macOS for perforce),\n>     since every other platform never installs them and was warning\n>     regardless.\n\nIt is curious that nobody seems to have looked at this patch, as my\ncursory look suggests it would be a no brainer to check correctness\nof this under the assumption that nothing will change externally.\n\nThe maintenance to keep this in sync with what exactly are tested in\neach platforms will be made more costly with this change, but I do\nnot know by how much.  If somebody wants to start testing p4 on a\ndifferent platform, for example, as I think t98xx will just punt\nwithout failing if p4 is not available, it will probably be a while\nuntil they eventually notice that their test do not run due to lack\nof p4 and then they have to add their platform to the logic added by\nthis patch.  I think jgit is also the same; silent success when JGIT\nprerequisite is not met.  But if they are motivated enough to add\ntests, they will eventually notice when the tests they wanted to run\nwere not running, so it is a reasonably low risk.  If somebody wants\nto drop testing jgit on a platform, we may still install jgit even\nthough we do not run tests that require jgit, which may take longer\nfor us to notice, but the result is just as bad at most as the state\nwithout this change, so overall I think it makes sense.\n\nAny volunteers to offer a second pair of eyes?\n\nThanks.\n\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2403%2FHaraldNordgren%2Fci-scope-optional-tool-warnings-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2403/HaraldNordgren/ci-scope-optional-tool-warnings-v1\n> Pull-Request: https://github.com/git/git/pull/2403\n>\n>  ci/install-dependencies.sh | 54 ++++++++++++++++++++++----------------\n>  1 file changed, 31 insertions(+), 23 deletions(-)\n>\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 2f61fbb07c..a68cec64b4 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -171,30 +171,38 @@ Documentation)\n>  \t;;\n>  esac\n>  \n> -if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n> -then\n> -\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> -\tp4d -V\n> -\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> -\tp4 -V\n> -else\n> -\techo >&2 \"::warning:: perforce wasn't installed, see above for clues why\"\n> -fi\n> +case \"$distro\" in\n> +ubuntu-*|macos-*)\n> +\tif type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> +\t\tp4d -V\n> +\t\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> +\t\tp4 -V\n> +\telse\n> +\t\techo >&2 \"::warning:: perforce wasn't installed, see above for clues why\"\n> +\tfi\n> +\t;;\n> +esac\n>  \n> -if type git-lfs >/dev/null 2>&1\n> -then\n> -\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n> -\tgit-lfs version\n> -else\n> -\techo >&2 \"::warning:: git-lfs wasn't installed, see above for clues why\"\n> -fi\n> +case \"$distro\" in\n> +ubuntu-*)\n> +\tif type git-lfs >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n> +\t\tgit-lfs version\n> +\telse\n> +\t\techo >&2 \"::warning:: git-lfs wasn't installed, see above for clues why\"\n> +\tfi\n>  \n> -if type jgit >/dev/null 2>&1\n> -then\n> -\techo \"$(tput setaf 6)JGit Version$(tput sgr0)\"\n> -\tjgit version\n> -else\n> -\techo >&2 \"::warning:: JGit wasn't installed, see above for clues why\"\n> -fi\n> +\tif type jgit >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$(tput setaf 6)JGit Version$(tput sgr0)\"\n> +\t\tjgit version\n> +\telse\n> +\t\techo >&2 \"::warning:: JGit wasn't installed, see above for clues why\"\n> +\tfi\n> +\t;;\n> +esac\n>  \n>  end_group \"Install dependencies\"\n>\n> base-commit: 47ce80527c56f462cb97db4ca8125342204d3783\n"},{"id":"553456","messageId":"arpeDzeXlnZRwj30@pks.im","threadId":"66317","inReplyTo":"pull.2403.git.git.1789223882471.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: only warn about perforce/git-lfs/JGit on platforms that need them","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T12:31:11Z","receivedAt":"2026-09-28T12:31:17Z","isPatch":true,"body":"On Sat, Sep 12, 2026 at 02:38:02PM +0000, Harald Nordgren via GitGitGadget wrote:\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 2f61fbb07c..a68cec64b4 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -171,30 +171,38 @@ Documentation)\n>  \t;;\n>  esac\n>  \n> -if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n> -then\n> -\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> -\tp4d -V\n> -\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> -\tp4 -V\n> -else\n> -\techo >&2 \"::warning:: perforce wasn't installed, see above for clues why\"\n> -fi\n> +case \"$distro\" in\n> +ubuntu-*|macos-*)\n> +\tif type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> +\t\tp4d -V\n> +\t\techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> +\t\tp4 -V\n> +\telse\n> +\t\techo >&2 \"::warning:: perforce wasn't installed, see above for clues why\"\n> +\tfi\n> +\t;;\n> +esac\n>  \n> -if type git-lfs >/dev/null 2>&1\n> -then\n> -\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n> -\tgit-lfs version\n> -else\n> -\techo >&2 \"::warning:: git-lfs wasn't installed, see above for clues why\"\n> -fi\n> +case \"$distro\" in\n> +ubuntu-*)\n> +\tif type git-lfs >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n> +\t\tgit-lfs version\n> +\telse\n> +\t\techo >&2 \"::warning:: git-lfs wasn't installed, see above for clues why\"\n> +\tfi\n>  \n> -if type jgit >/dev/null 2>&1\n> -then\n> -\techo \"$(tput setaf 6)JGit Version$(tput sgr0)\"\n> -\tjgit version\n> -else\n> -\techo >&2 \"::warning:: JGit wasn't installed, see above for clues why\"\n> -fi\n> +\tif type jgit >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$(tput setaf 6)JGit Version$(tput sgr0)\"\n> +\t\tjgit version\n> +\telse\n> +\t\techo >&2 \"::warning:: JGit wasn't installed, see above for clues why\"\n> +\tfi\n> +\t;;\n> +esac\n\nI wonder whether it makes sense to have these warnings in the first\nplace.\n\nPart of the reason why we have these checks is that we allow the\ninstallation of these tools to fail, and if so we know to gracefully\ncontinue anyway. Tests will be skipped, and the pipeline will be green\nin such a case. But is that even a safe thing to do? I strongly doubt\nthat we'd start to notice such failures anytime soon, so it very much\ngives us a false sense of confidence.\n\nSo I'd suggest that instead of warning, we should make the whole build\nfail outright if we fail to install any of those tools. And once we do,\nthese warnings here become quite useless, because we know that the build\nwould fail on platforms where we expect the tools to be present. And on\nplatforms where we don't, the warning is pointless anyway.\n\nThanks!\n\nPatrick\n"},{"id":"553504","messageId":"CAHwyqnXJqABaN1JvfF7R0P9FbK1ptt66kaO98=sJDw-p3x=nXg@mail.gmail.com","threadId":"66317","inReplyTo":"arpeDzeXlnZRwj30@pks.im","subject":"Re: [PATCH] ci: only warn about perforce/git-lfs/JGit on platforms that need them","fromName":"Harald Nordgren","fromEmail":"haraldnordgren@gmail.com","sentAt":"2026-09-28T16:13:00Z","receivedAt":"2026-09-28T16:13:39Z","isPatch":true,"body":"> I wonder whether it makes sense to have these warnings in the first\n> place.\n>\n> Part of the reason why we have these checks is that we allow the\n> installation of these tools to fail, and if so we know to gracefully\n> continue anyway. Tests will be skipped, and the pipeline will be green\n> in such a case. But is that even a safe thing to do? I strongly doubt\n> that we'd start to notice such failures anytime soon, so it very much\n> gives us a false sense of confidence.\n>\n> So I'd suggest that instead of warning, we should make the whole build\n> fail outright if we fail to install any of those tools. And once we do,\n> these warnings here become quite useless, because we know that the build\n> would fail on platforms where we expect the tools to be present. And on\n> platforms where we don't, the warning is pointless anyway.\n\nFine by me!\n\n\nHarald\n"}]}