From: Jeff Hostetler <git@jeffhostetler.com>
To: Josh Steadmon <steadmon@google.com>,
Junio C Hamano <gitster@pobox.com>,
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: Re: Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file)
Date: Mon, 14 Aug 2023 09:09:14 -0400 [thread overview]
Message-ID: <2a347112-aaa8-d8ef-5919-dc66730af593@jeffhostetler.com> (raw)
In-Reply-To: <ZLHcaFvvZh88TrLb@google.com>
On 7/14/23 7:38 PM, Josh Steadmon wrote:
> 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.
I missed the original discussion and am definitely late to the party,
but an FYI there is also a `wmain()` in `compat/mingw.c` that is used
for MSVC builds on Windows. It sets up some OS process stuff before
calling the actual `main()`.
Jeff
next prev parent reply other threads:[~2023-08-14 13:10 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 ` Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file) Josh Steadmon
2023-07-15 0:34 ` Splitting common-main Junio C Hamano
2023-08-14 13:09 ` Jeff Hostetler [this message]
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=2a347112-aaa8-d8ef-5919-dc66730af593@jeffhostetler.com \
--to=git@jeffhostetler.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=steadmon@google.com \
--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).