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

On May 20, 2021 7:24 PM, Emily Shaffer wrote:

On Thu, May 20, 2021 at 05:36:25PM -0400, Randall S. Becker wrote:

quoted

On May 20, 2021 5:06 PM, Emily Shaffer wrote:

quoted

To: git@vger.kernel.org
Cc: Emily Shaffer <redacted>; Ævar Arnfjörð Bjarmason
[off-list ref]; Junio C Hamano [off-list ref]; Jeff
Hostetler [off-list ref]; Bagas Sanjaya
[off-list ref]
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.

quoted

quoted

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.

quoted

quoted

Learning which tools and wrappers invoke Git, and how, would give us insight to decide where to improve Git's usability and

performance.

quoted

quoted

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.

quoted

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