On Wed, Jan 30, 2019 at 12:56:24PM -0800, Jeff Hostetler via GitGitGadget wrote:
> Add optional platform-specific code to log information about
> the current process.
>
> On Windows, this includes whether git.exe is running under a
> debugger and information about the ancestors of the process.
>
> The purpose of this information is to help indicate if the
> process was launched interactively or in the background by
> an IDE, for example.
> diff --git a/trace2.h b/trace2.h
> index a0e99d9c26..cb11a46366 100644
> --- a/trace2.h
> +++ b/trace2.h
> @@ -363,4 +363,18 @@ __attribute__((format (printf, 1, 2)))
> void trace2_printf(const char *fmt, ...);
> #endif
>
> +/*
> + * Optional platform-specific code to dump information about the
> + * current and any parent process(es). This is intended to allow
> + * post-processors to know who spawned this git instance and anything
> + * else the platform may be able to tell us about the current process.
> + */
> +#if defined(GIT_WINDOWS_NATIVE)
> +void trace2_collect_process_info(void);
> +#else
> +#define trace2_collect_process_info() \
> + do { \
> + } while (0)
> +#endif
Please consider mentioning in the commit message that on other
platforms this is a noop. I was scrolling through the whole patch,
skipping over the Windows-specific parts, to see how you did it on
Linux, only to find the above do-nothing loop. It was anticlimactic :)
Why is it a noop on other platforms? I suspect that (since your main
focus is supporting Windows devs using Git on Windows) it's along the
lines of "I just didn't want to bother, and left it as future work for
anyone interested", which a perfectly valid reason in my book.
However, if you did look into it, and found some major difficulties or
downright showstoppers, then that might be worth mentioning.
(Portability? I would expect that it would need a bunch of '#elif
defined(...)')
