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.
next prev parent 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 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).