git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all"
Date: Tue, 23 Feb 2021 13:28:54 -0500	[thread overview]
Message-ID: <YDVJZnmTiBYZ4iPM@coredump.intra.peff.net> (raw)
In-Reply-To: <20210223114132.24345-7-avarab@gmail.com>

On Tue, Feb 23, 2021 at 12:41:32PM +0100, Ævar Arnfjörð Bjarmason wrote:

> Adding $(FUZZ_OBJS) as a dependency on "all" was intentionally done in
> 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).
> 
> Rather than needlessly build these objects which aren't required for
> the build every time we make "all", let's instead move them to be
> built by the CI jobs.
> 
> The goal is to make sure that we don't inadvertently break these, we
> can accomplish that goal by building them in CI, rather than slowing
> down every build of git for everyone everywhere.

The current state is that regular devs are responsible for avoiding
compile breakages in the fuzz objects, even if they don't care
themselves. Your earlier patches turned this into: regular devs are not
on the hook for breaking fuzz objects; they are the responsibility of
fuzz people. I'm OK with either of those, but this approach seems to me
like the worst of both worlds. ;)

If you do a refactor, you are still on the hook for breaking the fuzz
objects because CI will fail (and you have to investigate it, and fix it
for CI to remain a useful tool). But instead of finding out about the
problem quickly as you're working, instead you push up what you think is
a finished result, and then from minutes to hours later you get a
notification telling you that oops, you missed a spot. I find that the
shorter the error-fix-compile cycle is, the less time I waste waiting or
context-switching.

If we had a ton of fuzz object files that took forever to build, the
savings on each build might be worth it. But AFAICT (from timing "make
clean; make -j1" before and after), we are saving less than 1% of the
build time (which is way less than the run-to-run noise).

It doesn't seem like the right tradeoff to me. (Likewise, if other
CI-only checks we have, like coccinelle, could be run at a similar cost,
I'd recommend sticking them into the default developer build).

One thing we _could_ do is stop building fuzz objects as part of "all",
but include them for DEVELOPER=1 builds (which includes CI). That keeps
them from hurting normal users (who don't actually need them), but
prevents bitrot. It doesn't address your original motivation though (you
as a developer would probably still be building them).

-Peff

  reply	other threads:[~2021-02-23 18:32 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-26 16:07 [PATCH 0/4] Makefile: micro-optimize light non-test builds Ævar Arnfjörð Bjarmason
2021-01-26 16:07 ` [PATCH 1/4] Makefile: refactor assignment for subsequent change Ævar Arnfjörð Bjarmason
2021-01-27  1:29   ` Junio C Hamano
2021-01-26 16:07 ` [PATCH 2/4] Makefile: refactor " Ævar Arnfjörð Bjarmason
2021-01-26 16:07 ` [PATCH 3/4] Makefile: add a NO_TEST_TOOLS flag Ævar Arnfjörð Bjarmason
2021-01-26 16:07 ` [PATCH 4/4] Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target Ævar Arnfjörð Bjarmason
2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King
2021-01-27  1:38   ` Junio C Hamano
2021-01-27  4:34     ` Jeff King
2021-01-27  6:07       ` Junio C Hamano
2021-01-28 18:23   ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason
2021-02-01 11:17     ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2021-02-03  1:11       ` Junio C Hamano
2021-02-04  7:06         ` Jeff King
2021-02-04 17:49           ` Junio C Hamano
2021-02-23 11:41       ` [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs " Ævar Arnfjörð Bjarmason
2021-02-23 17:57         ` Junio C Hamano
2021-02-23 18:31         ` Jeff King
2021-02-23 11:41       ` [PATCH v3 1/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason
2021-02-23 11:41       ` [PATCH v3 2/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason
2021-02-23 11:41       ` [PATCH v3 3/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason
2021-02-23 11:41       ` [PATCH v3 4/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason
2021-02-23 11:41       ` [PATCH v3 5/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets Ævar Arnfjörð Bjarmason
2021-02-23 11:41       ` [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" Ævar Arnfjörð Bjarmason
2021-02-23 18:28         ` Jeff King [this message]
2021-02-23 19:19           ` Junio C Hamano
2021-02-28 20:13           ` Ævar Arnfjörð Bjarmason
2021-03-01  9:39             ` Jeff King
2021-02-01 11:17     ` [PATCH v2 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason
2021-02-04  6:51       ` Jeff King
2021-02-01 11:17     ` [PATCH v2 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason
2021-02-01 11:17     ` [PATCH v2 3/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason
2021-02-01 11:17     ` [PATCH v2 4/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason
2021-02-01 11:17     ` [PATCH v2 5/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason
2021-02-01 11:17     ` [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason
2021-02-01 22:30       ` Junio C Hamano
2021-01-28 18:23   ` [PATCH 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason
2021-01-28 18:23   ` [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason
2021-01-29  7:49     ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=YDVJZnmTiBYZ4iPM@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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