From: Josh Steadmon <steadmon@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, calvinwan@google.com, szeder.dev@gmail.com,
phillip.wood123@gmail.com, chooglen@google.com, avarab@gmail.com,
sandals@crustytoothpaste.net
Subject: Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file)
Date: Fri, 14 Jul 2023 16:38:16 -0700 [thread overview]
Message-ID: <ZLHcaFvvZh88TrLb@google.com> (raw)
In-Reply-To: <xmqqcz2xtv83.fsf@gitster.g>
Hi, I'd like to revisit this as it's also relevant to a non-unit-test
issue (`make fuzz-all` is currently broken). I have some questions
inline below:
On 2023.05.18 10:17, Junio C Hamano wrote:
> steadmon@google.com writes:
>
> > It is convenient to have common_exit() in its own object file so that
> > standalone programs may link to it (and any other object files that
> > depend on it) while still having their own independent main() function.
>
> I am not so sure if this is a good direction to go in, though. The
> common_exit() function does two things that are very specific to and
> dependent on what Git runtime has supposed to have done, like
> initializing trace2 subsystem and linking with usage.c to make
> bug_called_must_BUG exist.
True. We won't call common_exit() unless we're trying to exit() from a
file that also includes git-compat-util.h, but I guess that's not a
guarantee that trace2 is initialized or that usage.o is linked.
> I understand that a third-party or standalone non-Git programs may
> want to do _more_ than what our main() does when starting up, but it
> should be doable if make our main() call out to a hook function,
> whose definition in Git is a no-op, that can be replaced by their
> own implementation to do whatever they want to happen in main(), no?
>
> The reason why I am not comfortable with this patch is because I
> cannot say why this split is better than other possible split. For
> example, we could instead split only our 'main' out to a separate
> file, say "main.c", and put main.o together with common-main.o to
> libgit.a to be found by the linker, and that arrangement will also
> help your "standalone programs" having their own main() function.
> Now with these two possible ways to split (and there may be other
> split that may be even more convenient; I simply do not know), which
> one is better, and what's the argument for each approach?
Sorry, I don't think I'm understanding your proposal here properly,
please let me know where I'm going wrong: isn't this functionally
equivalent to my patch, just with different filenames? Now main() would
live in main.c (vs. my common-main.c), while check_bug_if_BUG() and
common_exit() would live in common-main.c (now a misnomer, vs. my
common-exit.c). I'm not following how that changes anything so I'm
pretty sure I've misunderstood.
The issue I was trying to solve (whether for a unit-test framework or
for the fuzzing engine) is that we don't have direct control over their
main(), and so we can't rename it to avoid conflicts with our main().
I guess there may be some linker magic we could do to avoid the conflict
and have (our) main() call (their, renamed) main()? I don't know offhand
if that's actually possible, just speculating. Even if possible, it
feels more complicated to me, but again that may just be due to my lack
of linker knowledge.
next prev parent reply other threads:[~2023-07-14 23:38 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-17 23:56 [PATCH RFC v2 0/4] Add an external testing library for unit tests steadmon
2023-05-17 23:56 ` [PATCH RFC v2 1/4] common-main: split common_exit() into a new file steadmon
2023-05-18 17:17 ` Junio C Hamano
2023-07-14 23:38 ` Josh Steadmon [this message]
2023-07-15 0:34 ` Splitting common-main Junio C Hamano
2023-08-14 13:09 ` Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file) Jeff Hostetler
2023-05-17 23:56 ` [PATCH RFC v2 2/4] unit tests: Add a project plan document steadmon
2023-05-18 13:13 ` Phillip Wood
2023-05-18 20:15 ` Glen Choo
2023-05-24 17:40 ` Josh Steadmon
2023-06-01 9:19 ` Phillip Wood
2023-05-17 23:56 ` [PATCH RFC v2 3/4] Add C TAP harness steadmon
2023-05-18 13:15 ` Phillip Wood
2023-05-18 20:50 ` Josh Steadmon
2023-05-17 23:56 ` [PATCH RFC v2 4/4] unit test: add basic example and build rules steadmon
2023-05-18 13:32 ` Phillip Wood
2023-06-09 23:25 ` [RFC PATCH v3 0/1] Add a project document for adding unit tests Josh Steadmon
2023-06-09 23:25 ` [RFC PATCH v3 1/1] unit tests: Add a project plan document Josh Steadmon
2023-06-13 22:30 ` Junio C Hamano
2023-06-30 22:18 ` Josh Steadmon
2023-06-29 19:42 ` Linus Arver
2023-06-29 20:48 ` Josh Steadmon
2023-06-30 19:31 ` Linus Arver
2023-07-06 18:24 ` Glen Choo
2023-07-06 19:02 ` Junio C Hamano
2023-07-06 22:48 ` Glen Choo
2023-06-30 21:33 ` Josh Steadmon
2023-06-29 21:21 ` Junio C Hamano
2023-06-30 0:11 ` Linus Arver
2023-06-30 14:07 ` Phillip Wood
2023-06-30 18:47 ` K Wan
2023-06-30 22:35 ` Josh Steadmon
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=ZLHcaFvvZh88TrLb@google.com \
--to=steadmon@google.com \
--cc=avarab@gmail.com \
--cc=calvinwan@google.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood123@gmail.com \
--cc=sandals@crustytoothpaste.net \
--cc=szeder.dev@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 read-only IMAP folder(s) and NNTP newsgroup(s).