Use sh -o pipefail for all shell commands in Buildbot

[gsnedders]

44f9121

Use sh -o pipefail for all shell commands in Buildbot
https://bugs.webkit.org/show_bug.cgi?id=276081
rdar://problem/130908663

Reviewed by Elliott Williams.

We've had multiple bugs caused by pipelines exiting cleanly despite
commands within them failing, for example the above linked bug 276081
(where run-webkit-tests exited uncleanly but filter-test-logs exited
cleanly, thus we were able to totally break run-webkit-tests without
noticing), and also bug 271808 (where build-webkit exited uncleanly
but filter-build-webkit exited cleanly, thus we didn't notice the bot
being in a bogus configuration and no build occurring).

To avoid this, this makes us always pass -o pipefail to the shell we
are creating. This is new in POSIX 2024, but commonly `sh` is provided
by `bash` or `zsh` which have long supported the `pipefail` option,
even when invoked as `sh` (and in POSIX-compliant mode). This is
potentially risky if we happen to have any bots configured to use
older versions of, e.g., dash, but I do not believe we do.

* Tools/CISupport/build-webkit-org/steps.py:
(ShellMixin.shell_command):
(CompileWebKit):
(CompileWebKit.start):
(RunJavaScriptCoreTests):
(RunJavaScriptCoreTests.run):
(RunWebDriverTests):
(RunWebDriverTests.run):
* Tools/CISupport/build-webkit-org/steps_unittest.py:
(TestCompileWebKit.test_success_architecture):
* Tools/CISupport/ews-build/steps.py:
(ShellMixin.shell_command):
* Tools/CISupport/ews-build/steps_unittest.py:
(test_success):
(test_warnings):
(test_flaky_failures_in_first_run):
(test_first_run_failed_unexpectedly):
(test_too_many_flaky_failures_in_first_and_second_run):
(TestRunWebKitTestsInStressMode.test_success_wk1):
(TestRunWebKitTestsInStressMode.test_failure):
(TestRunWebKitTestsInStressMode.test_success):
(TestRunWebKitTestsInStressMode.test_success_additional_arguments):
(TestRunWebKitTestsInStressGuardmallocMode.test_success):
(TestRunWebKitTestsInStressGuardmallocMode.test_failure):
(TestRunWebKitTestsWithoutChange.test_success):
(TestRunWebKitTestsWithoutChange.test_run_subtest_tests_success):
(TestRunWebKitTestsWithoutChange.test_run_subtest_tests_removes_skipped_that_fails):
(TestRunWebKitTestsWithoutChange.test_run_subtest_tests_fail):
(TestRunWebKitTestsWithoutChange.test_run_subtest_tests_limit_exceeded):
(TestRunWebKitTestsWithoutChange.test_failure):
(TestRunWebKit1Tests.test_success):
(TestRunWebKit1Tests.test_failure):
(TestRunWebKitTestsRedTree.test_success):
(TestRunWebKitTestsRedTree.test_set_properties_when_executed_scope_this_class):
(TestRunWebKitTestsRepeatFailuresRedTree.test_success):
(TestRunWebKitTestsRepeatFailuresRedTree.test_set_properties_when_executed_scope_this_class):
(TestRunWebKitTestsRepeatFailuresWithoutChangeRedTree.test_success):
(TestRunWebKitTestsRepeatFailuresWithoutChangeRedTree.test_step_with_change_did_timeout):
(TestRunWebKitTestsRepeatFailuresWithoutChangeRedTree.test_set_properties_when_executed_scope_this_class):
(TestUpdateWorkingDirectory.test_success):
(TestUpdateWorkingDirectory.test_success_branch):
(TestUpdateWorkingDirectory.test_success_remote):
(TestUpdateWorkingDirectory.test_success_remote_branch):
(TestCheckOutPullRequest.test_success):
(TestCheckOutPullRequest.test_success_apple):
(TestCheckOutPullRequest.test_success_integration_remote):
(TestCheckOutPullRequest.test_success_wincairo):
(TestCheckOutPullRequest.test_failure):
(TestGetTestExpectationsBaseline.test_success):
(TestGetTestExpectationsBaseline.test_additional_args):
(TestGetUpdatedTestExpectations.test_success):
(TestGetUpdatedTestExpectations.test_additional_args):

Canonical link: https://commits.webkit.org/281869@main

73b52d6

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	🧪 wpe-wk2	🧪 wincairo-tests
🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	🧪 api-wpe
	🧪 ios-wk2-wpt	🧪 mac-wk1	✅ 🛠 wpe-cairo
	🧪 api-ios	🧪 mac-wk2	🛠 gtk
	✅ 🛠 vision	🧪 mac-AS-debug-wk2	🧪 gtk-wk2
❌ 🧪 services	✅ 🛠 vision-sim	🧪 mac-wk2-stress	🧪 api-gtk
	🧪 vision-wk2
✅ 🛠 🧪 unsafe-merge	🛠 tv
	🛠 tv-sim
	🛠 watch
	🛠 watch-sim

[webkit-early-warning-system]

EWS run on previous version of this PR (hash db149d3)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
✅ 🧪 services	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

[gsnedders]

This also fixes #29792, by virtue of fixing this everywhere.

[dpogue]

Super minor, but FYI there's a typo in the commit message:

this makes us always pass -o passfail to the shell

should read -o pipefail (rather than passfail)

[emw-apple]

It's a very reasonable change in isolation, though I wonder if we have any places where we currently depend on not having pipefail, e.g. a command which can safely fail or is expected to exit nonzero. If you haven't already, be sure to check every command we run which has a | character.

[gsnedders]

It's a very reasonable change in isolation, though I wonder if we have any places where we currently depend on not having pipefail, e.g. a command which can safely fail or is expected to exit nonzero. If you haven't already, be sure to check every probably a good idea to look at every command we run which has a | character.

I did; this is a large part of why it's an optional argument so we could do otherwise — it just turns out I don't believe we ever want the default semantic.

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 50c2f19)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
✅ 🧪 services	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

[gsnedders]

Super minor, but FYI there's a typo in the commit message:

this makes us always pass -o passfail to the shell

should read -o pipefail (rather than passfail)

(only change in the new push is fixing the commit message, thanks @dpogue for catching that!)

[webkit-early-warning-system]

EWS run on current version of this PR (hash 73b52d6)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	🧪 wpe-wk2	🧪 wincairo-tests
🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	🧪 api-wpe
	🧪 ios-wk2-wpt	🧪 mac-wk1	✅ 🛠 wpe-cairo
	🧪 api-ios	🧪 mac-wk2	🛠 gtk
	✅ 🛠 vision	🧪 mac-AS-debug-wk2	🧪 gtk-wk2
❌ 🧪 services	✅ 🛠 vision-sim	🧪 mac-wk2-stress	🧪 api-gtk
	🧪 vision-wk2
✅ 🛠 🧪 unsafe-merge	🛠 tv
	🛠 tv-sim
	🛠 watch
	🛠 watch-sim

[gsnedders]

see https://bugs.webkit.org/show_bug.cgi?id=276081#c4 for why I'd delayed landing this, but adding this to the merge-queue now

[webkit-commit-queue]

Committed 281869@main (44f9121): https://commits.webkit.org/281869@main

Reviewed commits have been landed. Closing PR #30424 and removing active labels.