Merge branch 'mc/tr2-process-ancestry-cleanup' into next

Add process ancestry data to trace2 on macOS to match what we
already do on Linux and Windows.  Also adjust the way Windows
implementation reports this information to match the other two.

* mc/tr2-process-ancestry-cleanup:
  t0213: add trace2 cmd_ancestry tests
  test-tool: extend trace2 helper with 400ancestry
  trace2: emit cmd_ancestry data for Windows
  trace2: refactor Windows process ancestry trace2 event
  build: include procinfo.c impl for macOS
  trace2: add macOS process ancestry tracing

Author: gitster

compat/darwin/procinfo.c

@@ -0,0 +1,97 @@
+#include "git-compat-util.h"
+#include "strbuf.h"
+#include "strvec.h"
+#include "trace2.h"
+#include <sys/sysctl.h>
+
+/*
+ * An arbitrarily chosen value to limit the depth of the ancestor chain.
+ */
+#define NR_PIDS_LIMIT 10
+
+/*
+ * Get the process name and parent PID for a given PID using sysctl().
+ * Returns 0 on success, -1 on failure.
+ */
+static int get_proc_info(pid_t pid, struct strbuf *name, pid_t *ppid)
+{
+	int mib[4];
+	struct kinfo_proc proc;
+	size_t size = sizeof(proc);
+
+	mib[0] = CTL_KERN;
+	mib[1] = KERN_PROC;
+	mib[2] = KERN_PROC_PID;
+	mib[3] = pid;
+
+	if (sysctl(mib, 4, &proc, &size, NULL, 0) < 0)
+		return -1;
+
+	if (size == 0)
+		return -1;
+
+	strbuf_addstr(name, proc.kp_proc.p_comm);
+	*ppid = proc.kp_eproc.e_ppid;
+
+	return 0;
+}
+
+/*
+ * Recursively push process names onto the ancestry array.
+ * We guard against cycles by limiting the depth to NR_PIDS_LIMIT.
+ */
+static void push_ancestry_name(struct strvec *names, pid_t pid, int depth)
+{
+	struct strbuf name = STRBUF_INIT;
+	pid_t ppid;
+
+	if (depth >= NR_PIDS_LIMIT)
+		return;
+
+	if (pid <= 0)
+		return;
+
+	if (get_proc_info(pid, &name, &ppid) < 0)
+		goto cleanup;
+
+	strvec_push(names, name.buf);
+
+	/*
+	 * Recurse to the parent process. Stop if ppid not valid
+	 * or if we've reached ourselves (cycle).
+	 */
+	if (ppid && ppid != pid)
+		push_ancestry_name(names, ppid, depth + 1);
+
+cleanup:
+	strbuf_release(&name);
+}
+
+void trace2_collect_process_info(enum trace2_process_info_reason reason)
+{
+	struct strvec names = STRVEC_INIT;
+
+	if (!trace2_is_enabled())
+		return;
+
+	switch (reason) {
+	case TRACE2_PROCESS_INFO_STARTUP:
+		push_ancestry_name(&names, getppid(), 0);
+		if (names.nr)
+			trace2_cmd_ancestry(names.v);
+
+		strvec_clear(&names);
+		break;
+
+	case TRACE2_PROCESS_INFO_EXIT:
+		/*
+		 * The Windows version of this calls its
+		 * get_peak_memory_info() here. We may want to insert
+		 * similar process-end statistics here in the future.
+		 */
+		break;
+
+	default:
+		BUG("trace2_collect_process_info: unknown reason '%d'", reason);
+	}
+}

compat/win32/trace2_win32_process_info.c

@@ -3,6 +3,7 @@
 #include "../../git-compat-util.h"
 #include "../../json-writer.h"
 #include "../../repository.h"
+#include "../../strvec.h"
 #include "../../trace2.h"
 #include "lazyload.h"
 #include <psapi.h>
@@ -32,12 +33,7 @@ static int find_pid(DWORD pid, HANDLE hSnapshot, PROCESSENTRY32 *pe32)
 }
 
 /*
- * Accumulate JSON array of our parent processes:
- *     [
- *         exe-name-parent,
- *         exe-name-grand-parent,
- *         ...
- *     ]
+ * Accumulate array of our parent process names.
  *
  * Note: we only report the filename of the process executable; the
  *       only way to get its full pathname is to use OpenProcess()
@@ -73,7 +69,7 @@ static int find_pid(DWORD pid, HANDLE hSnapshot, PROCESSENTRY32 *pe32)
  * simple and avoid the alloc/realloc overhead.  It is OK if we
  * truncate the search and return a partial answer.
  */
-static void get_processes(struct json_writer *jw, HANDLE hSnapshot)
+static void get_processes(struct strvec *names, HANDLE hSnapshot)
 {
 	PROCESSENTRY32 pe32;
 	DWORD pid;
@@ -82,19 +78,19 @@ static void get_processes(struct json_writer *jw, HANDLE hSnapshot)
 
 	pid = GetCurrentProcessId();
 	while (find_pid(pid, hSnapshot, &pe32)) {
-		/* Only report parents. Omit self from the JSON output. */
+		/* Only report parents. Omit self from the output. */
 		if (nr_pids)
-			jw_array_string(jw, pe32.szExeFile);
+			strvec_push(names, pe32.szExeFile);
 
 		/* Check for cycle in snapshot. (Yes, it happened.) */
 		for (k = 0; k < nr_pids; k++)
 			if (pid == pid_list[k]) {
-				jw_array_string(jw, "(cycle)");
+				strvec_push(names, "(cycle)");
 				return;
 			}
 
 		if (nr_pids == NR_PIDS_LIMIT) {
-			jw_array_string(jw, "(truncated)");
+			strvec_push(names, "(truncated)");
 			return;
 		}
 
@@ -105,24 +101,14 @@ static void get_processes(struct json_writer *jw, HANDLE hSnapshot)
 }
 
 /*
- * Emit JSON data for the current and parent processes.  Individual
- * trace2 targets can decide how to actually print it.
+ * Collect the list of parent process names.
  */
-static void get_ancestry(void)
+static void get_ancestry(struct strvec *names)
 {
 	HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
 
 	if (hSnapshot != INVALID_HANDLE_VALUE) {
-		struct json_writer jw = JSON_WRITER_INIT;
-
-		jw_array_begin(&jw, 0);
-		get_processes(&jw, hSnapshot);
-		jw_end(&jw);
-
-		trace2_data_json("process", the_repository, "windows/ancestry",
-				 &jw);
-
-		jw_release(&jw);
+		get_processes(names, hSnapshot);
 		CloseHandle(hSnapshot);
 	}
 }
@@ -176,13 +162,35 @@ static void get_peak_memory_info(void)
 
 void trace2_collect_process_info(enum trace2_process_info_reason reason)
 {
+	struct strvec names = STRVEC_INIT;
+
 	if (!trace2_is_enabled())
 		return;
 
 	switch (reason) {
 	case TRACE2_PROCESS_INFO_STARTUP:
 		get_is_being_debugged();
-		get_ancestry();
+		get_ancestry(&names);
+		if (names.nr) {
+			/*
+			  Emit the ancestry data as a data_json event to
+			  maintain compatibility for consumers of the older
+			  "windows/ancestry" event.
+			 */
+			struct json_writer jw = JSON_WRITER_INIT;
+			jw_array_begin(&jw, 0);
+			for (size_t i = 0; i < names.nr; i++)
+				jw_array_string(&jw, names.v[i]);
+			jw_end(&jw);
+			trace2_data_json("process", the_repository,
+					 "windows/ancestry", &jw);
+			jw_release(&jw);
+
+			/* Emit the ancestry data with the new event. */
+			trace2_cmd_ancestry(names.v);
+		}
+
+		strvec_clear(&names);
 		return;
 
 	case TRACE2_PROCESS_INFO_EXIT:

config.mak.uname

@@ -149,6 +149,8 @@ ifeq ($(uname_S),Darwin)
 	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
 	CSPRNG_METHOD = arc4random
 	USE_ENHANCED_BASIC_REGULAR_EXPRESSIONS = YesPlease
+	HAVE_PLATFORM_PROCINFO = YesPlease
+	COMPAT_OBJS += compat/darwin/procinfo.o
 
         ifeq ($(uname_M),arm64)
 		HOMEBREW_PREFIX = /opt/homebrew

contrib/buildsystems/CMakeLists.txt

@@ -274,6 +274,8 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux")
 	add_compile_definitions(PROCFS_EXECUTABLE_PATH="/proc/self/exe" HAVE_DEV_TTY )
 	list(APPEND compat_SOURCES unix-socket.c unix-stream-server.c compat/linux/procinfo.c)
+elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
+	list(APPEND compat_SOURCES compat/darwin/procinfo.c)
 endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "Windows")

meson.build

@@ -1294,6 +1294,8 @@ if host_machine.system() == 'linux'
   libgit_sources += 'compat/linux/procinfo.c'
 elif host_machine.system() == 'windows'
   libgit_sources += 'compat/win32/trace2_win32_process_info.c'
+elif host_machine.system() == 'darwin'
+  libgit_sources += 'compat/darwin/procinfo.c'
 else
   libgit_sources += 'compat/stub/procinfo.c'
 endif

t/helper/test-trace2.c

@@ -466,6 +466,63 @@ static int ut_303redact_def_param(int argc, const char **argv)
 	return 0;
 }
 
+/*
+ * Run a child process with specific trace2 environment settings so that
+ * we can capture its trace2 output (including cmd_ancestry) in isolation.
+ *
+ * test-tool trace2 400ancestry <target> <output_file> [<child_command_line>]
+ *
+ * <target> is one of: normal, perf, event
+ *
+ * For example:
+ *     test-tool trace2 400ancestry normal out.normal test-tool trace2 001return 0
+ *
+ * The child process inherits a controlled trace2 environment where only
+ * the specified target is directed to <output_file>. The parent's trace2
+ * environment variables are cleared in the child so that only the child's
+ * events are captured.
+ *
+ * This is used by t0213-trace2-ancestry.sh to test cmd_ancestry events.
+ * The child process will see "test-tool" as its immediate parent in the
+ * process ancestry, giving us a predictable value to verify.
+ */
+static int ut_400ancestry(int argc, const char **argv)
+{
+	struct child_process cmd = CHILD_PROCESS_INIT;
+	const char *target;
+	const char *outfile;
+	int result;
+
+	if (argc < 3)
+		die("expect <target> <output_file> <child_command_line>");
+
+	target = argv[0];
+	outfile = argv[1];
+	argv += 2;
+	argc -= 2;
+
+	/* Clear all trace2 environment variables in the child. */
+	strvec_push(&cmd.env, "GIT_TRACE2=");
+	strvec_push(&cmd.env, "GIT_TRACE2_PERF=");
+	strvec_push(&cmd.env, "GIT_TRACE2_EVENT=");
+	strvec_push(&cmd.env, "GIT_TRACE2_BRIEF=1");
+
+	/* Set only the requested target. */
+	if (!strcmp(target, "normal"))
+		strvec_pushf(&cmd.env, "GIT_TRACE2=%s", outfile);
+	else if (!strcmp(target, "perf"))
+		strvec_pushf(&cmd.env, "GIT_TRACE2_PERF=%s", outfile);
+	else if (!strcmp(target, "event"))
+		strvec_pushf(&cmd.env, "GIT_TRACE2_EVENT=%s", outfile);
+	else
+		die("invalid target '%s', expected: normal, perf, event",
+		    target);
+
+	strvec_pushv(&cmd.args, argv);
+	result = run_command(&cmd);
+	exit(result);
+}
+
 /*
  * Usage:
  *     test-tool trace2 <ut_name_1> <ut_usage_1>
@@ -497,6 +554,8 @@ static struct unit_test ut_table[] = {
 	{ ut_301redact_child_start, "301redact_child_start", "<argv...>" },
 	{ ut_302redact_exec,        "302redact_exec",        "<exe> <argv...>" },
 	{ ut_303redact_def_param,   "303redact_def_param",   "<key> <value>" },
+
+	{ ut_400ancestry,           "400ancestry",           "<target> <output_file> [<child_command_line>]" },
 };
 /* clang-format on */
 

t/meson.build

@@ -134,6 +134,7 @@ integration_tests = [
   't0210-trace2-normal.sh',
   't0211-trace2-perf.sh',
   't0212-trace2-event.sh',
+  't0213-trace2-ancestry.sh',
   't0300-credentials.sh',
   't0301-credential-cache.sh',
   't0302-credential-store.sh',

t/t0210-trace2-normal.sh

@@ -74,8 +74,9 @@ scrub_normal () {
 	#      This line is only emitted when RUNTIME_PREFIX is defined,
 	#      so just omit it for testing purposes.
 	#
-	#   4. 'cmd_ancestry' is not implemented everywhere, so for portability's
-	#      sake, skip it when parsing normal.
+	#   4. 'cmd_ancestry' output depends on how the test is run and
+	#      is not relevant to the features we are testing here.
+	#      Ancestry tests are covered in t0213-trace2-ancestry.sh instead.
 	sed \
 		-e 's/elapsed:[0-9]*\.[0-9][0-9]*\([eE][-+]\{0,1\}[0-9][0-9]*\)\{0,1\}/elapsed:_TIME_/g' \
 		-e "s/^start '[^']*' \(.*\)/start _EXE_ \1/" \