From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] ci: skip unavailable external software
Date: Fri, 25 Apr 2025 07:39:37 -0700 [thread overview]
Message-ID: <xmqqv7qsuwye.fsf@gitster.g> (raw)
In-Reply-To: <aAtdsURaaxYO7pVt@pks.im> (Patrick Steinhardt's message of "Fri, 25 Apr 2025 12:02:25 +0200")
Patrick Steinhardt <ps@pks.im> writes:
> 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.
next prev parent reply other threads:[~2025-04-25 14:39 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-24 22:13 repo.eclipse.org outage breaking all our linux CI jobs Junio C Hamano
2025-04-24 23:10 ` [PATCH] ci: skip unavailable external software Junio C Hamano
2025-04-25 4:19 ` Patrick Steinhardt
2025-04-25 9:49 ` Junio C Hamano
2025-04-25 10:02 ` Patrick Steinhardt
2025-04-25 14:39 ` Junio C Hamano [this message]
2025-04-25 12:01 ` Johannes Schindelin
2025-04-25 14:41 ` Junio C Hamano
2025-04-25 15:38 ` [PATCH 0/2] ci: update unavailable external software handling Junio C Hamano
2025-04-25 15:38 ` [PATCH 1/2] ci: update the message for unavailble third-party software Junio C Hamano
2025-04-25 15:38 ` [PATCH 2/2] ci: download JGit from maven, not eclipse.org Junio C Hamano
2025-04-25 14:57 ` repo.eclipse.org outage breaking all our linux CI jobs shejialuo
2025-04-25 15:20 ` Junio C Hamano
2025-04-26 14:12 ` shejialuo
2025-04-28 6:49 ` Patrick Steinhardt
2025-04-28 10:30 ` shejialuo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqv7qsuwye.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).