Re: [PATCH] ci: skip unavailable external software

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 25, 2025, 14:39 UTC
Message-ID: <xmqqv7qsuwye.fsf@gitster.g>
In-Reply-To: <aAtdsURaaxYO7pVt@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 12 quoted lines

> But shouldn't the failing wget cause an error, too? So the `|| { }`
> cleanup branch would execute in that case and we can prune the empty
> file there. So in other words, shouldn't the following work alright?
>
>     if wget --output-document=...
>     then
>         massage output
>     else
>         rm output
>     fi
>
> Or am I still missing the obvious?

While the above "works", what is "obvious" is that it is way too verbose and the merit of going verbose is dubious, especially given that the reason that trigger the argument to favor the above construct over the more concise

	wget && massage || rm

is "massage part should never fail".

We do want to notice a failure in something, if that something is what should never fail, don't we? Not necessarily so!

Our CI jobs are not in the business of checking and ensuring wget or chmod keeps working. If either of them fail, the more important part is its practical impact to the rest of the CI job---resulting "jgit" file is harmful to be left on disk and needs to be removed.

Another to think about this is to imagine if we are having this conversation, had "wget" had (just like its --output-document argument) a "--chmod" argument. (wget && massage) as a unit is conceptually a single "download the jgit binary" in this case, and if either of them fail, we failed to download it.

So, yes, either would "work", but I do not think longhand is warranted in this case.