On Thu, Apr 24, 2025 at 04:10:47PM -0700, Junio C Hamano wrote:
> The ci/install-dependencies.sh script used in a very early phase of
> our CI jobs downloads Perforce, Git-LFS, and JGit, used for running
> the test scripts. The test framework is prepared to properly skip
> the tests that depend on these external software, but the CI script
> is unnecessarily strict (due to its use of "set -e" in ci/lib.sh)
> and fails the entire CI run before even starting to test the rest of
> the system.
>
> Notice a failure to download to any of these external software, but
> keep going. We need to be careful about cleaning after a failed
> wget, as a later part of the script that does:
>
> if type jgit >/dev/null 2>&1
> then
> echo "$(tput setaf 6)JGit Version$(tput sgr0)"
> jgit version
> else
> echo >&2 "WARNING: JGit wasn't installed, see above for clues why"
> fi
>
> will (surprise!) succeed running "type jgit", and then fail with
> "jgit version", taking the whole thing down due to "set -e".
Yeah, I think this is a sensible direction to go. It is unfortunate that
this may lead to silent breakage of these dependencies unless somebody
explicitly looks for those warnings. But that feels like the lesser evil
compared to failing the whole pipeline.
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> ci/install-dependencies.sh | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 0df74610d0..e51304c3b0 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -66,16 +66,29 @@ ubuntu-*|i386/ubuntu-*|debian-*)
> mkdir --parents "$CUSTOM_PATH"
>
> wget --quiet --directory-prefix="$CUSTOM_PATH" \
> - "$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
> - chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
> -
> - wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> + "$P4WHENCE/bin.linux26x86_64/p4d" \
> + "$P4WHENCE/bin.linux26x86_64/p4" &&
> + chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4" || {
> + rm -f "$CUSTOM_PATH/p4"
> + rm -f "$CUSTOM_PATH/p4d"
> + echo >&2 "P4 download (optional) failed"
> + }
I think it would be preferable to only handle failure of wget as chmod
shouldn't ever fail if wget was successful. The same is true for the
other downloads -- let's be as strict as possible but allow failure of
those actions that depend on the network.
Patrick
