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

From: "Randall S. Becker" <[email-masked]>
Date: Fri, 21 May 2021 09:20:57 -0400

On May 20, 2021 7:24 PM, Emily Shaffer wrote:
>
>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?

I'll get to it pretty quickly once it's rolled in.

>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.

Guards are fine. There's just a lot more work to do for me. We need to make sure that the rendering of ancestor processes are
generic enough not to be just pid_t through any interfaces where this is queried. In NonStop's case, char[25], should be sufficient
for the short term, but I would prefer something longer, say char[128] to be safe for the future in which to stick the ancestor. To
be completely unique, the ancestor is going to look like \node.$proc:sequence (where node is a 7 character name, proc is a 5
character name, and sequence is currently a long.

-Randall