Re: [PATCH v2] fsmonitor OSX: fix hangs for submodules

Koji Nakamaru [off-list ref] writes:

quoted

quoted

+test_expect_success "submodule implicitly starts daemon by pull" '
+ test_atexit "stop_watchdog" &&
+ test_when_finished "stop_git && rm -rf cloned super sub" &&

If stop_git ever returns with non-zero status, "rm -rf" will be
skipped, which I am not sure is a good idea.

The whole test_when_finished would fail in such a case, so you would
notice the problem right away, which is a plus, though.

t/README discusses that test_when_finished and test_atexit differ about
the "--immediate" option. As git and its subprocesses are the test
target, I moved stop_git to the current place. This might be however
confusing when someone later reads this test. Should we simply put
stop_git and stop_watchdong in test_atexit?

That is not what I meant.

I was merely questioning the &&-chaining that stops "rm -fr" from
running if stop_git ever fails (and your earlier iteration you had
multiple "rm -fr" ;-chained, not &&-chained---not using && is often
more appropriate in a when_finished handler).

quoted

quoted

+ set -m &&

I have to wonder how portable (and necessary) this is.

POSIX says it shall be supported if the implementation supports the
User Portability Utilities option.  It also says that it was added
to apply only to the UPE because it applies primarily to interactive
use, not shell script applications.  And our test scripts are of
course not interactive.

How about the following modification? It still utilizes $git_pgid to
filter processes, but avoids "set -m".

Nah, your original reads much better, and the code is grabbing and
using the process group information anyway (and my question about
"-m" was more about "should we be relying on process group features
in this test to kill them all?").

I am OK with the idea that we can assume, at least among the
platforms that support fsmonitor, that sending a signal to a process
group would cause the signal delivered to the member processes just
as we expect.

Thanks.