All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Arthur Chan via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Arthur Chan <arthur.chan@adalogics.com>
Subject: Re: [PATCH v3] fuzz: reorganise the path for existing oss-fuzz fuzzers
Date: Mon, 19 Sep 2022 13:10:04 +0200	[thread overview]
Message-ID: <220919.86r10762bg.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <pull.1353.v3.git.1663542495094.gitgitgadget@gmail.com>


On Sun, Sep 18 2022, Arthur Chan via GitGitGadget wrote:

> From: Arthur Chan <arthur.chan@adalogics.com>
>
> This patch is aimed to provide a better organisation for oss-fuzz
> fuzzers, allowing more fuzzers for the git project to be added
> in later development.

I don't see any problem with this change per-se, but this rationale
really doesn't explain anything in the end to the reader. How does just
having x/y-*.c files rather than x-y-*.c allow for more fuzzers to be
added? We could also add new fuzzers to the top-level now, why does this
change help us to do so.

I suspect the unstated reason is just "adding a lot more would make the
top-level cluttered", or perhaps some design reason you hinted at in
https://lore.kernel.org/git/2405897f-a774-e0d3-99bb-2185dcbb5432@adalogics.com/
(but I haven't taken the time to fully understand).

So, I'm fine with this v3 as-is, but also wouldn't mind a v4 with an
updated commit message to address the above confusion.

> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,7 +1,7 @@
> -/fuzz-commit-graph
> +/oss-fuzz/fuzz-commit-graph
>  /fuzz_corpora
> -/fuzz-pack-headers
> -/fuzz-pack-idx
> +/oss-fuzz/fuzz-pack-headers
> +/oss-fuzz/fuzz-pack-idx
>  /GIT-BUILD-OPTIONS
>  /GIT-CFLAGS
>  /GIT-LDFLAGS

Speaking of clutter, a much better change here IMO would be to create a
/oss-fuzz/.gitignore file, and then move these there. For prior art see:

	git ls-files '**/.gitignore'

Even better (but I'm not sure how this is all used in the end), can we
perhaps build those in a .gitignore'd oss-fuzz/.build/.

But maybe not, and in any case that would be a larger change to the
Makefile logic, so we can leave it for now, but I do think it makes
sense to create a oss-fuzz/.gitignore in a v4.

  reply	other threads:[~2022-09-19 11:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-16 19:03 [PATCH] fuzz: reorganise the path for existing oss-fuzz fuzzers Arthur Chan via GitGitGadget
2022-09-16 21:55 ` Junio C Hamano
2022-09-17  0:45   ` Arthur Chan
2022-09-17 23:28 ` [PATCH v2] " Arthur Chan via GitGitGadget
2022-09-18 23:08   ` [PATCH v3] " Arthur Chan via GitGitGadget
2022-09-19 11:10     ` Ævar Arnfjörð Bjarmason [this message]
2022-09-19 14:36     ` [PATCH v4] " Arthur Chan via GitGitGadget
2022-09-19 16:38       ` Junio C Hamano
2022-09-19 16:40       ` 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

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

  git send-email \
    --in-reply-to=220919.86r10762bg.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=arthur.chan@adalogics.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.