Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Jeff King <peff@peff.net>
Cc: "Taylor Blau" <me@ttaylorr.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] ci: avoid unnecessary builds
Date: Tue, 8 Nov 2022 10:16:15 +0100 (CET)	[thread overview]
Message-ID: <oo9ssp5n-6ors-n309-2r2n-3q43rq7pn89q@tzk.qr> (raw)
In-Reply-To: <Y2SFGmQnx7CXtTEI@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 5487 bytes --]

Hi Peff,

On Thu, 3 Nov 2022, Jeff King wrote:

> On Thu, Nov 03, 2022 at 10:23:56PM -0400, Taylor Blau wrote:
>
> > But I think you make a compelling point (which doesn't match my own
> > workflow, but I can see the utility of nonetheless).
> >
> > I was thinking that we could rely on something similar to the ci-config
> > ref stuff from back in e76eec35540 (ci: allow per-branch config for
> > GitHub Actions, 2020-05-07), but it looks like it'll be a little
> > trickier than that, maybe impossible.
> >
> > We need to know about the state of the ci-config branch before we set
> > the concurrency bits. So I think you *could* do something like:
>
> As an aside, I wish there was a way to interpret per-repo environment
> variables in the actual action config.

There kind of is. "Kind of" because it is not _really_ a per-repo variable
(those do not exist on GitHub), but there are topics you can set. These
are relatively free-form labels you can attach to _your_ fork, and these
labels show up below the "About" section and the link to the home page (if
any) on the right side of your respository. AFAICT these topics are not
inherited automatically when forking a repository, which is precisely what
we want. See
https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/classifying-your-repository-with-topics
for more details on that.

A GitHub workflow can be made conditional on the presence of such a topic.
I use this, for example, for an opt-in build of the InnoSetup installer:
https://github.com/jrsoftware/issrc/blob/is-6_2_1/.github/workflows/build.yml#L11-L12
The build is opt-in because it requires a non-free build environment,
configured via two repository secrets containing a URL pointing to a
`.zip` file and a password to extract said `.zip`. As you say, I cannot
make the build opt-in based on the presence of secrets, but I can use the
presence a repository topic as the knob to enable the workflow.

> The current ci-config stuff works, but it's pretty horrible because (if
> I understand correctly) it spins up a VM just to evaluate a glob and say
> "nope, no CI needed on this branch". So:
>
>   1. It's wasteful of resources, compared to a system where the Actions
>      parser can evaluate a variable.

Indeed. It might look like the job only takes a few seconds (at least that
was the argument that got the `ci-config` patch accepted), but that misses
the queue time, which turns this more into several dozens of seconds, and
the recorded total duration is much longer than that. In
https://github.com/gitgitgadget/git/actions/runs/3412982102 for example,
the `ci-config` job only took 6 seconds, according to the page, but the
total duration of the build was 6 minutes and 56 seconds.

>   2. It makes the Actions results page for a repo ugly, because skipped
>      branches clutter the output with "yes, I passed CI" even though all
>      they passed was a trivial job to say "don't bother running more
>      CI".
>
>   3. The problem you mention: it happens too late to affect the overall
>      Actions flow, and instead individual jobs have to take it into
>      account.

And

    4. The way `ci-config` is configured is sufficiently "magic" that it
       only benefits very, very few users, at the price of adding to
       everybody's build time. To see what I mean, look at
       https://github.com/gitgitgadget/git/actions/runs/3416084640/jobs/5685867765#step:1:1
       and at
       https://github.com/gitgitgadget/git/actions/runs/3416084640/jobs/5685879914#step:1:1
       turning on the timestamps (i.e. click on the sprocket wheel on the
       upper right side of the log and select "Show timestamps"). You will
       see that the `ci-config` job started at 3:22:05 UTC and the next
       job, `win-build`, started only at 4:16:03 UTC.

    5. There is official support for the desired behavior that comes
       without any magic branch with special content that users somehow
       need to learn about: If you push a branch with commit messages that
       contain `[skip ci]`, the build will be skipped, and no time is
       wasted on running any job. For full details, see
       https://github.blog/changelog/2021-02-08-github-actions-skip-pull-request-and-push-workflows-with-skip-ci/

Having said that, the `ci-config` job is used for something I find much
more useful than that magic `ci-config` branch handling: to skip builds
when there are already successful builds of the same commit or at least
tree. Sadly, that logic fails sometimes because of an unresilient REST API
call: in case of network errors, we fall back to not skipping the build
_even if_ there has been a previously-successful build of that tree.

If it was up to me, I'd simply rip out the `ci-config` branch (`ci/config`
file) handling and tell users to mark up branches that need to be skipped
with `[skip ci]`. That has a further advantage of being actually portable
across a wider range of CI systems (for example, CircleCI supports it,
too: https://circleci.com/docs/skip-build/).

But then, it would not save us a whole lot because the `ci-config` job
_still_ does something useful (i.e. checking for previously successful
builds of the same tree), so that time is still spent. But how knows,
maybe there will be out-of-the-box support for that at some stage. 🤷

Ciao,
Dscho

  reply	other threads:[~2022-11-08  9:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-03 13:34 [PATCH] ci: avoid unnecessary builds Johannes Schindelin via GitGitGadget
2022-11-04  1:46 ` Ævar Arnfjörð Bjarmason
2022-11-04  2:23   ` Taylor Blau
2022-11-04  3:20     ` Jeff King
2022-11-08  9:16       ` Johannes Schindelin [this message]
2022-11-09 14:00         ` Jeff King
2022-11-10  2:40           ` Taylor Blau
2022-11-04  2:09 ` Taylor Blau
2022-11-07 19:45 ` Derrick Stolee
2022-11-07 19:53   ` Taylor Blau
2022-11-07 20:08     ` Derrick Stolee
2022-11-07 21:03       ` Ævar Arnfjörð Bjarmason
2022-11-07 21:59         ` Derrick Stolee
2022-11-07 22:44           ` Taylor Blau
2022-11-08  8:18             ` Johannes Schindelin
2022-11-08 18:30               ` Taylor Blau
2022-11-07 22:56           ` Ævar Arnfjörð Bjarmason
2022-11-08  0:02             ` Derrick Stolee
2022-11-08  0:31   ` Junio C Hamano
2022-11-08  9:51     ` Johannes Schindelin

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=oo9ssp5n-6ors-n309-2r2n-3q43rq7pn89q@tzk.qr \
    --to=johannes.schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).