Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/ci-guards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,9 @@ jobs:

- name: Validate nightly push throttle
if: ${{ matrix.group == 'release-notary' }}
run: python3 tests/test_ci_nightly_push_throttle.py
run: |
python3 tests/test_ci_nightly_push_throttle.py
python3 tests/test_ci_helper_prebuild_lifecycle.py

- name: Validate test compilation cache seeding
if: ${{ matrix.group == 'quality-sharding' }}
Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/nightly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,15 @@ jobs:
# step went from 21 minutes to 31-40 (#12704). The diagnostics
# wrapper does not need a serial build; PR 12704 recorded 95% free
# memory on the 50 GiB host during the failure it was written for.
#
# The app target's Rust helper phases run after the Swift compile but
# do not read its output. Build them now at low priority so those
# phases find the local-source helpers fresh instead of building them after
# the compile. A failed or late prebuild only leaves that work to the
# phases, so its status never fails this step.
python3 scripts/ci/run-with-helper-prebuild.py \
--derived-data build-universal --configuration Release --archs "$archs" \
--log "$RUNNER_TEMP/helper-prebuild.log" -- \
./scripts/ci/run-xcodebuild-with-diagnostics.sh -- \
env CMUX_SKIP_ZIG_BUILD=1 xcodebuild -scheme cmux -configuration Release -derivedDataPath build-universal \
-destination 'generic/platform=macOS' \
Expand Down
97 changes: 97 additions & 0 deletions scripts/ci/prebuild-app-helpers.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/env bash
# Build the Rust helpers that the cmux app target's late script phases bundle
# (diff sidecar and command palette Nucleo FFI) while xcodebuild compiles
# Swift.
#
# Those phases run after the cmux Swift compile although they do not read its
# output, so on CI their cargo builds (about 100 s) were the serial tail of the
# nightly build. This script overlaps the two local-source helpers and runs the same build scripts with the same Cargo
# target directories and the same toolchain-visible environment that Xcode gives
# the phases. When the phases then run, Cargo finds every unit fresh and the
# phases only copy, lipo, and sign.
#
# Correctness never depends on this script. If it fails, is slower than the
# Swift compile, or its environment differs from the phase's, the phase's own
# cargo invocation waits on Cargo's build-directory lock or rebuilds the dirty
# units exactly as it did before.
#
# usage: prebuild-app-helpers.sh --derived-data <path> --archs "<archs>"
# [--configuration <name>]
set -euo pipefail

ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
derived_data=""
archs=""
configuration="Release"

while (($#)); do
case "$1" in
--derived-data) derived_data="$2"; shift 2 ;;
--archs) archs="$2"; shift 2 ;;
--configuration) configuration="$2"; shift 2 ;;
*) echo "error: unknown argument: $1" >&2; exit 2 ;;
esac
done
if [[ -z "$derived_data" || -z "$archs" ]]; then
echo "usage: $0 --derived-data <path> --archs \"<archs>\" [--configuration <name>]" >&2
exit 2
fi
derived_data="$(mkdir -p "$derived_data" && cd "$derived_data" && pwd)"

# Xcode exports every build setting to script phases. rustc records
# MACOSX_DEPLOYMENT_TARGET in its dep-info and the cc crate reruns build scripts
# when SDKROOT changes, so both must match the phase's values or Cargo would
# rebuild. The project uses one deployment target for every configuration.
deployment_targets="$(
grep -o 'MACOSX_DEPLOYMENT_TARGET = [0-9.]*' "$ROOT/cmux.xcodeproj/project.pbxproj" \
| awk '{print $3}' | sort -u
)"
if [[ "$(printf '%s\n' "$deployment_targets" | wc -l | tr -d ' ')" != "1" ]]; then
echo "error: expected one MACOSX_DEPLOYMENT_TARGET in the project, found: $deployment_targets" >&2
exit 1
fi
export MACOSX_DEPLOYMENT_TARGET="$deployment_targets"
SDKROOT="$(xcrun --sdk macosx --show-sdk-path)"
export SDKROOT

# The diff sidecar keeps its Cargo target directory under the app target's
# TARGET_TEMP_DIR: $(PROJECT_TEMP_DIR)/$(CONFIGURATION)/$(TARGET_NAME).build.
target_temp_dir="$derived_data/Build/Intermediates.noindex/cmux.build/$configuration/cmux.build"
mkdir -p "$target_temp_dir"
scratch="$(mktemp -d "${TMPDIR:-/tmp}/cmux-helper-prebuild.XXXXXX")"
trap 'rm -rf "$scratch"' EXIT

run_helper() {
local name="$1"
shift
local started
started="$(date +%s)"
if "$@" >"$scratch/$name.log" 2>&1; then
echo "prebuilt $name in $(( $(date +%s) - started ))s"
else
local status=$?
echo "prebuild of $name failed with status $status after $(( $(date +%s) - started ))s; the Xcode phase will build it" >&2
tail -40 "$scratch/$name.log" >&2
return "$status"
fi
}

run_helper diff-sidecar env \
TARGET_TEMP_DIR="$target_temp_dir" \
CMUX_DIFF_SIDECAR_ARCHS="$archs" \
CMUX_DIFF_SIDECAR_MIN_MACOS="$MACOSX_DEPLOYMENT_TARGET" \
"$ROOT/scripts/build-diff-sidecar.sh" &
sidecar_pid=$!
run_helper nucleo-ffi env \
CMUX_NUCLEO_FFI_ARCHS="$archs" \
CMUX_NUCLEO_FFI_REQUIRE_CARGO=1 \
"$ROOT/scripts/build-command-palette-nucleo-ffi.sh" &
nucleo_pid=$!
# cmux-cua remains in the authoritative Xcode phase. Its source checkout
# mutates a shared Git cache; cancelling an optional prebuild during checkout
# could leave a Git index lock that poisons the subsequent required build.

status=0
wait "$sidecar_pid" || status=1
wait "$nucleo_pid" || status=1
exit "$status"
87 changes: 87 additions & 0 deletions scripts/ci/run-with-helper-prebuild.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#!/usr/bin/env python3
"""Run an optional helper alongside the build; only the build determines success."""
import argparse
import os
from pathlib import Path
import signal
import subprocess
import time


def stop_group(process):
if process is None:
return
# Kill the group even if its leader exited: grandchildren can still be alive.
try:
os.killpg(process.pid, signal.SIGTERM)
except ProcessLookupError:
process.wait()
return
time.sleep(0.2)
try:
os.killpg(process.pid, signal.SIGKILL)
except ProcessLookupError:
pass
process.wait()


def run(helper_command, build_command, log_path, helper_timeout=600):
helper = build = None
cancelled = 0

def cancel(signum, _frame):
nonlocal cancelled
cancelled = signum

previous = {sig: signal.signal(sig, cancel) for sig in (signal.SIGINT, signal.SIGTERM)}
try:
with open(log_path, "w") as log:
try:
helper = subprocess.Popen(helper_command, stdout=log, stderr=subprocess.STDOUT,
start_new_session=True)
except OSError as error:
print(f"::warning::Optional helper prebuild could not start: {error}", flush=True)
build = subprocess.Popen(build_command, start_new_session=True)
deadline = time.monotonic() + helper_timeout
while build.poll() is None and not cancelled:
if helper is not None and helper.poll() is not None:
if helper.returncode:
print("::warning::Optional helper prebuild failed; build phases remain authoritative", flush=True)
stop_group(helper)
helper = None
if helper is not None and time.monotonic() >= deadline:
print("::warning::Optional helper prebuild reached its deadline; build phases remain authoritative", flush=True)
stop_group(helper)
helper = None
time.sleep(0.05)
if cancelled:
return 128 + cancelled
return build.returncode if build.returncode >= 0 else 128 - build.returncode
finally:
# Neither an unsuccessful build nor cancellation waits for optional work.
stop_group(helper)
stop_group(build)
for sig, handler in previous.items():
signal.signal(sig, handler)
if Path(log_path).exists():
print(Path(log_path).read_text(errors="replace"), end="", flush=True)


def main():
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--derived-data", required=True)
parser.add_argument("--configuration", default="Release")
parser.add_argument("--archs", required=True)
parser.add_argument("--log", required=True)
parser.add_argument("command", nargs=argparse.REMAINDER)
args = parser.parse_args()
command = args.command[1:] if args.command[:1] == ["--"] else args.command
if not command:
parser.error("a build command is required")
helper = Path(__file__).with_name("prebuild-app-helpers.sh")
return run(["nice", "-n", "10", str(helper), "--derived-data", args.derived_data,
"--configuration", args.configuration, "--archs", args.archs], command, args.log)


if __name__ == "__main__":
raise SystemExit(main())
4 changes: 4 additions & 0 deletions tests/test-execution.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,10 @@ lane = "linux-guard"
path = "tests/test_cla_guard_metadata_routing.py"
lane = "linux-guard"

[[test]]
path = "tests/test_ci_helper_prebuild_lifecycle.py"
lane = "linux-guard"

[[test]]
path = "tests/test_ios_simulator_build_once.py"
lane = "linux-guard"
Expand Down
104 changes: 104 additions & 0 deletions tests/test_ci_helper_prebuild_lifecycle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
#!/usr/bin/env python3
"""The optional prebuild cannot delay or change the authoritative build result."""
import importlib.util
from pathlib import Path
import signal
import subprocess
import sys
import tempfile
import time
import unittest
from unittest.mock import Mock, patch

ROOT = Path(__file__).resolve().parents[1]
SCRIPT = ROOT / "scripts/ci/run-with-helper-prebuild.py"


class LifecycleTests(unittest.TestCase):
def run_case(self, helper, build, timeout=5):
with tempfile.TemporaryDirectory() as tmp:
log = Path(tmp) / "helper.log"
code = (
"import importlib.util; "
f"s=importlib.util.spec_from_file_location('runner',{str(SCRIPT)!r}); "
"m=importlib.util.module_from_spec(s); s.loader.exec_module(m); "
f"raise SystemExit(m.run({[sys.executable, '-c', helper]!r}, "
f"{[sys.executable, '-c', build]!r}, {str(log)!r}, {timeout}))"
)
result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, timeout=8)
return result

def test_optional_work_does_not_mutate_the_shared_cua_source_cache(self):
prebuild = (ROOT / "scripts/ci/prebuild-app-helpers.sh").read_text()
self.assertNotIn('run_helper cmux-cua', prebuild)
self.assertNotIn('"$ROOT/scripts/build-cmux-cua.sh"', prebuild)

def test_failed_build_does_not_wait_for_hung_helper(self):
result = self.run_case("import signal; signal.pause()", "raise SystemExit(17)")
self.assertEqual(result.returncode, 17, result.stderr)

def simulated(self, helper_status, ticks):
spec = importlib.util.spec_from_file_location("runner", SCRIPT)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
helper = Mock(returncode=helper_status)
helper.poll.return_value = helper_status
build = Mock(returncode=0)
build.poll.side_effect = [None, 0]
with tempfile.TemporaryDirectory() as tmp:
with patch.object(module.subprocess, "Popen", side_effect=[helper, build]), \
patch.object(module.time, "monotonic", side_effect=ticks), \
patch.object(module.time, "sleep"), \
patch.object(module, "stop_group") as stop:
result = module.run(["helper"], ["build"], str(Path(tmp)/"log"), 10)
return result, stop.call_args_list

def test_failed_helper_does_not_fail_successful_build(self):
result, stopped = self.simulated(23, [0])
self.assertEqual(result, 0)
self.assertEqual(len(stopped), 3)

def test_completed_helper_is_not_reported_as_timed_out(self):
# Only the initial clock read is available; a completed helper must not
# consume the deadline check or wait for optional work.
result, _ = self.simulated(0, [0])
self.assertEqual(result, 0)

def test_helper_deadline_does_not_stop_build(self):
result, stopped = self.simulated(None, [0, 11])
self.assertEqual(result, 0)
self.assertEqual(len(stopped), 3)

def test_cancellation_kills_helper_descendants(self):
with tempfile.TemporaryDirectory() as tmp:
marker = Path(tmp) / "escaped"
child = f"import signal,pathlib; signal.signal(signal.SIGTERM, lambda *_: (pathlib.Path({str(marker)!r}).touch(), exit(0))); print('child ready',flush=True); signal.pause()"
helper = f"import subprocess,sys,signal; subprocess.Popen([sys.executable,'-c',{child!r}]); signal.pause()"
log = Path(tmp) / "helper.log"
code = (
"import importlib.util; "
f"s=importlib.util.spec_from_file_location('runner',{str(SCRIPT)!r}); "
"m=importlib.util.module_from_spec(s); s.loader.exec_module(m); "
f"raise SystemExit(m.run({[sys.executable,'-c',helper]!r}, "
f"{[sys.executable,'-c','import signal; signal.pause()']!r}, {str(log)!r}, 10))"
)
proc = subprocess.Popen([sys.executable, "-c", code], stdout=subprocess.DEVNULL)
try:
deadline = time.monotonic() + 3
while time.monotonic() < deadline:
if log.exists() and "child ready" in log.read_text():
break
time.sleep(.02)
else:
self.fail("helper did not start")
proc.send_signal(signal.SIGTERM)
self.assertEqual(proc.wait(timeout=3), 143)
self.assertTrue(marker.exists(), "helper grandchild did not receive cancellation")
finally:
if proc.poll() is None:
proc.kill()
proc.wait()


if __name__ == "__main__":
unittest.main()
Loading