On Thu, Jul 01 2021, Jeff Hostetler via GitGitGadget wrote:
A general comment on this series (including previous patches). We've
usually tried to bend over backwards in git's codebase not to have big
ifdef blocks, so we compile most code the same everywhere. We waste a
bit of object code, but that's fine.
See 9c897c5c2ad (pack-objects: remove #ifdef NO_PTHREADS, 2018-11-03)
for a good exmaple of bad code being turned to good.
E.g. in this case:
> +#ifdef HAVE_FSMONITOR_DAEMON_BACKEND
> +
> +int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
> +{
> + const char *subcmd;
> +
> + struct option options[] = {
> + OPT_END()
> + };
> +
> + if (argc < 2)
> + usage_with_options(builtin_fsmonitor__daemon_usage, options);
> +
> + if (argc == 2 && !strcmp(argv[1], "-h"))
> + usage_with_options(builtin_fsmonitor__daemon_usage, options);
> +
> + git_config(git_default_config, NULL);
> +
> + subcmd = argv[1];
> + argv--;
> + argc++;
> +
> + argc = parse_options(argc, argv, prefix, options,
> + builtin_fsmonitor__daemon_usage, 0);
> +
> + die(_("Unhandled subcommand '%s'"), subcmd);
> +}
> +
> +#else
> +int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
> +{
> + struct option options[] = {
> + OPT_END()
> + };
> +
> + if (argc == 2 && !strcmp(argv[1], "-h"))
> + usage_with_options(builtin_fsmonitor__daemon_usage, options);
> +
> + die(_("fsmonitor--daemon not supported on this platform"));
> +}
> +#endif
This whole thing could really just be a
-DHAVE_FSMONITOR_DAEMON_BACKEND=1 or -DHAVE_FSMONITOR_DAEMON_BACKEND=0
somewhere (depending), and then somewhere in the middle of the first
function:
if (!HAVE_FSMONITOR_DAEMON_BACKEND)
die(_("fsmonitor--daemon not supported on this platform"));
