Rollback bazelbuild#14510 because it causes remote test execution to fail

shs96c · copybara-github · commit f090433fcf0f · 2023-01-09T05:58:47.000-08:00
When the parent process is run as a different user to the test itself (as can happen in some remote build environments, such as buildbarn) using `kill -0 $PPID` fails, causing the test to be prematurely killed. This PR rolls back the (nicer, cleaner, not always working) fix and replaces it with the (less nice, less clean, more often working) check that was there before. Tested with a local build against buildbarn. Closes bazelbuild#17147. PiperOrigin-RevId: 500691976 Change-Id: Ife55db5c1ed88d40ba502257ce9b679b04eeb179
diff --git a/tools/test/test-setup.sh b/tools/test/test-setup.sh
@@ -331,9 +331,18 @@ fi
 childPid=$!
 
 # Cleanup helper
-# Assume that we don't have drastically reduced abilities to communicate signals
-# to our parent process. kill()ability means existence.
-( while kill -0 $PPID &> /dev/null; do  # magic 0 sigspec tests deliverability only
+# It would be nice to use `kill -0 $PPID` here, but when whatever called this
+# is running as a different user (as happens in remote execution) that will
+# return an error, causing us to prematurely reap a running test.
+( if ! (ps -p $$ &> /dev/null || [ "`pgrep -a -g $$ 2> /dev/null`" != "" ] ); then
+   # `ps` is known to be unrunnable in the darwin sandbox-exec environment due
+   # to being a set-uid root program. pgrep exists in most environments, but not
+   # universally. In the event that we find ourselves running in an environment
+   # where *neither* exists, we have no reliable way to check if our parent is
+   # still alive - so simply disable this cleanup routine entirely.
+   exit 0
+ fi
+ while ps -p $$ &> /dev/null || [ "`pgrep -a -g $$ 2> /dev/null`" != "" ]; do
     sleep 10
  done
  # Parent process not found - we've been abandoned! Clean up test processes.
