@ethomson can you pull the plug on the build ? I think it's exhibiting the thing you had warned me about 😭.

@ethomson can you pull the plug on the build ? I think it's exhibiting the thing you had warned me about 😭.

I cancelled it. If you send me your microsoft account name I can add you to the project.

Thanks !

I'm done with this. The first commit fixes the TMI output problem leaks causes with its envvars, the second is to get the convenient leak-frames, because when -atExit triggers, the process under inspection is already gone from memory, so you can't inspect its stack. I do realize it's kinda hacky though, but I've tried really hard to get a failure to register, but AFAICT nohup just causes the return from leaks to be ignored, and also swallows its reports.

Here's the link to the failed test build I was using, since I'm about to remove the explicit leak I introduced for testing purposes.

This is definitely an improvement over where we were at. I wonder if we can tweak this a bit so that the test runner itself doesn't need to know about leaks but can instead run some cleanup process before it exits that is specified via environment variable or something.

The tricky part is that leaks itself needs to know the pid. 🤔

Sounds tricky to make it more generic, especially since AFAIK this is the only use-case we need it for.

I think it's fine as it is, now that the macOS build report leaks correctly. This has the bonus that I can run that code locally now, instead of my old "break-on-return, grab $pid, run leaks" dance.

An alternative would be to design some kind of "pattern evaluation system", so you could say --clar-eval-on-exit="exec leaks $PID", which sounds fun but heavy-handed.

An alternative would be to design some kind of "pattern evaluation system", so you could say --clar-eval-on-exit="exec leaks $PID", which sounds fun but heavy-handed.

Yes, you're right - I was thinking of an at exit type of execution handler, but I didn't want to build any sort of pattern evaluation. Since system(3) invokes things via /bin/sh -c then we can just use shell variables (eg $PPID.)

I was thinking about:

diff --git a/tests/main.c b/tests/main.c
index c16a9e7b9..c5a01566f 100644
--- a/tests/main.c
+++ b/tests/main.c
@@ -7,8 +7,8 @@ int __cdecl main(int argc, char *argv[])
 int main(int argc, char *argv[])
 #endif
 {
-   int res;
-   char *leak_env;
+   int res, atexit_res = 0;
+   char *atexit;
 
    clar_test_init(argc, argv);
 
@@ -29,18 +29,8 @@ int main(int argc, char *argv[])
    cl_global_trace_disable();
    git_libgit2_shutdown();
 
-#ifdef __APPLE__
-   leak_env = getenv("LEAK_CHECK");
-   if (leak_env && strcmp(leak_env, "leaks") == 0) {
-       char cmd[40];
-       int leaked;
-       sprintf((char *)&cmd, "leaks -quiet %d", getpid());
-       leaked = system(cmd);
-       return res || leaked;
-   }
-#else
-   (void)leak_env;
-#endif
+   if ((atexit = getenv("CLAR_ATEXIT")) != NULL)
+       atexit_res = system(atexit);
 
-   return res;
+   return res || atexit_res;
 }

Then you should be able to: CLAR_ATEXIT='leaks -quiet $PPID' ./libgit2_clar -sapply

That variable name looks a little weird (visually). Maybe CLAR_AT_EXIT? I dunno. Anyway, my hope is that we build something that we can upstream back to clar. That's something that we haven't been doing a ton of lately but I'd like to get us back in the habit.

This for sure sounds like a less-hacky idea. I'll try to integrate it and report back.

👍 on upstreaming to clar, too. I had originally started to write the XML support there, but I needed test cases, and I got bitten hard while trying a subtree-merge, so I just submitted the code here instead. It would be ❤️ to have a set of scripts to automatically push-pull commits to clar, but I don't git hard enough to be sure it's doable.

This has been rewritten as a envvar-passed shell command, so it's ready to 🚢.

THIS IS SUCH AN IMPROVEMENT. Thanks @tiennou!

Seriously, the old macOS build output was near impossible to understand. This is amazing.