Re: [PATCH] ci: skip unavailable external software

> 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@pobox.com>
> > ---
> >  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.