Subject: Re: [PATCH v2] tr2: log parent process name

From: Emily Shaffer <[email-masked]>
Date: Thu, 20 May 2021 16:23:45 -0700

On Thu, May 20, 2021 at 05:36:25PM -0400, Randall S. Becker wrote:
> 
> On May 20, 2021 5:06 PM, Emily Shaffer wrote:
> >To: [email-masked]
> >Cc: Emily Shaffer <[email-masked]>; Ævar Arnfjörð Bjarmason <[email-masked]>; Junio C Hamano <[email-masked]>;
> >Jeff Hostetler <[email-masked]>; Bagas Sanjaya <[email-masked]>
> >Subject: [PATCH v2] tr2: log parent process name
> >
> >It can be useful to tell who invoked Git - was it invoked manually by a user via CLI or script? By an IDE?  In some cases - like 'repo' tool -
> >we can influence the source code and set the GIT_TRACE2_PARENT_SID environment variable from the caller process. In 'repo''s case,
> >that parent SID is manipulated to include the string "repo", which means we can positively identify when Git was invoked by 'repo' tool.
> >However, identifying parents that way requires both that we know which tools invoke Git and that we have the ability to modify the source
> >code of those tools. It cannot scale to keep up with the various IDEs and wrappers which use Git, most of which we don't know about.
> >Learning which tools and wrappers invoke Git, and how, would give us insight to decide where to improve Git's usability and performance.
> >
> >Unfortunately, there's no cross-platform reliable way to gather the name of the parent process. If procfs is present, we can use that;
> >otherwise we will need to discover the name another way. However, the process ID should be sufficient regardless of platform.
> 
> I like this idea, but there are some platforms where this is unlikely to work. NonStop, in particular, can initiate git - and I frequently do - from a non-POSIX environment where process name is entirely different. In fact, it is something like $ABC (always beginning with a $, which makes life very difficult for shell scripts and screws up GIT_SSH_COMMAND, but I digress). I'm going to need to plug in something very platform-specific to make this work. getppid() always returns 1 in this situation, which is extraordinarily meaningless on the platform and does not represent the actual parent.

Ok. It sounds like you're saying I should be more conservative in the
commit message as well as in the #ifdef scope? Do you think this needs a
reroll to made the #ifdef more aggressive, or would you rather get to it
when you get to it?

It looks like the change in config.mak.uname won't affect NonStop; I
think also the compat/procinfo.c is probably indicative enough of "this
stuff is for procfs" that it won't look like it *should* work for
NonStop, which means that you should still get the stub for
'trace2_collect_process_info()'. But if you think the guards aren't
readable enough I can try to move them around a little more.

 - Emily