Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Jeff King <>
To: Johannes Schindelin <>
Cc: "Taylor Blau" <>,
	"Ævar Arnfjörð Bjarmason" <>,
	"Johannes Schindelin via GitGitGadget" <>,
Subject: Re: [PATCH] ci: avoid unnecessary builds
Date: Wed, 9 Nov 2022 09:00:33 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <oo9ssp5n-6ors-n309-2r2n-3q43rq7pn89q@tzk.qr>

On Tue, Nov 08, 2022 at 10:16:15AM +0100, Johannes Schindelin wrote:

> > 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
> for more details on that.

Ah, that's very clever, thank you!

For the original problem that motivated me adding ci-config in the first
place, branch selection, I think we could do this:

  if: |
    !contains(join(github.event.repository.topics), 'ci-only-') ||
    contains(github.event.repository.topics, format('ci-only-{0}', github.ref_name))

and then by default we'd continue to build for all pushes, but if you
add ci-only-foo as a repo topic, then we'd build only refs/heads/foo.

I may see if I can work this into our workflow file. Even if we can't
get rid of ci-config for the skip-successful-build feature, this would
still save CPU by dropping even ci-config when the branch should be
skipped entirely.

> > 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
> 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.

Yes, but I don't think that's wasting 6 minutes of resources if we're
just sitting in a queue. It may increase the end-to-end latency of
getting the CI result, of course.

>     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
>        and at
>        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.

Ouch. Though I wonder in practice how fast that would have gone without
ci-config. It is just asking for a generic ubuntu vm, which many of
other jobs would. Was there a shortage of those vms at 3:22, and by the
time it finally ran that shortage was over? Or is there constant
contention, and it is increasing the end-to-end latency by asking for a
queue slot, running, and then asking for more queue slots?

>     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

This existed back when I added ci-config originally, and I tried it, but
the results are quite painful to use. Because "skip ci" is really a
property of a branch that a commit is pushed to, and not a branch. So
you have to sprinkle these "skip ci" markers all over the branch tips,
but then remember to remove them when you actually want to do useful
things with the branches, like merge them or send them out.


  reply	other threads:[~2022-11-09 14:00 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
2022-11-09 14:00         ` Jeff King [this message]
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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \

* 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).