Skip to content

fix: unbreak main, a compile error and an empty xcodebuild argument - #2435

Merged
datlechin merged 3 commits into
mainfrom
fix/unbreak-main-ci
Aug 26, 2026
Merged

fix: unbreak main, a compile error and an empty xcodebuild argument#2435
datlechin merged 3 commits into
mainfrom
fix/unbreak-main-ci

Conversation

@datlechin

@datlechin datlechin commented Aug 26, 2026

Copy link
Copy Markdown
Member

main is red on two independent counts. This fixes both. They are separate commits and can be split if you would rather.

1. The build break, which is mine

fix(tabs): restore the quinaryLabel spelling that Xcode 26.4 compiles

#2434 shipped Color(nsColor: .quinaryLabelColor) and CI stopped at:

error: 'quinaryLabelColor' has been renamed to 'quinaryLabel'
    internal static var trackEdge: Color { Color(nsColor: .quinaryLabelColor) }

The two spellings swap places between toolchains. The Xcode 27 beta I develop against marks quinaryLabel deprecated and points at quinaryLabelColor; Xcode 26.4.1, which CI builds with, rejects quinaryLabelColor outright as renamed to quinaryLabel. I took the beta's hint on a line I was already moving, and only CI has the toolchain that disagrees. Reverted to the spelling that was there before, with a comment so the next person does not take the same hint.

It is one line and one root cause; the three reported failures are the file, its compile batch and the project.

2. The quarantine break, which predates it

ci: stop an empty test quarantine passing an empty argument to xcodebuild

The "Run unit tests" step has failed on every main run since 3cc00c0a1, with no test executing:

xcodebuild: error: Unknown build action ''.

scripts/ci/quarantine_args.py ends with print("\n".join(args)). With no quarantine entries that is print(""), which is a blank line. The workflow reads that stdout one line per argument, so the blank line becomes a literal empty argument and xcodebuild reads an empty argument as a build action.

3cc00c0a1 emptied .github/macos-test-quarantine.txt. The file's own header says "The list is empty. Keep it that way", so the empty list is the goal rather than an edge case, and the run history matches exactly: 036945d51 cut the list to 6 entries and passed; every run from 3cc00c0a1 onward failed.

Measured on the runner's own shell, /bin/bash 3.2.57:

producer emits resulting argc
nothing at all 0, harmless
one blank line 1, and the argument is the empty string

An empty array was never the problem; one empty string was.

Two changes, because the producer should not emit it and the consumer should not accept it:

  • quarantine_args.py prints nothing when there is nothing to skip.
  • the workflow's read loop skips empty lines, so no producer can inject an empty argument again.

The guard the script exists for is untouched: an entry that would skip nothing still fails the job with its hint, verified.

Test

scripts/ci/test_quarantine_args.py, run by a new Repo Hygiene step next to the existing test_update_registry.py. Six cases: an empty list and a whitespace-only file print nothing at all, no emitted line is ever blank, a whole-suite entry still works, and both "would skip nothing" refusals still fail the job.

It is a real regression test. Against the unfixed script it fails with AssertionError: '\n'.

Repo Hygiene is the right home: the macOS suite cannot catch this, because the script runs before any test does and when it gets this wrong no test runs at all.

3. The guard that had never once failed a job

ci: fail the test step when the quarantine or shard lister refuses to run

Found while verifying the fix above, and it is the worse bug of the two.

The comment over the unit test step says the script "refuses to run if any entry would skip nothing". It does refuse: it prints its diagnostic and exits 1. But it is invoked inside a process substitution, done < <(scripts/ci/quarantine-args.sh ...), and bash discards a process substitution's exit status. Measured:

bad-quarantine.txt:1: 'NoSuchSuite/nope()' would skip nothing: nothing by that name is enumerated
loop finished, argc=0
step exit=0

So an inert quarantine entry printed a warning nobody reads, the array came back empty, and the step ran the whole suite unskipped and reported success. The refusal that the script exists for, and that the comment promises, has never failed a job.

Both loops now redirect to a file on their own line, so bash -e sees the failure. Measured end to end on the new block:

quarantine argc reaching xcodebuild step exit
empty 0 0
one real entry 1, correct value 0
an entry that skips nothing n/a 1

Two smaller things in the same lines:

  • The UI shard loop had no empty-line guard, and its [ "${#ONLY_ARGS[@]}" -gt 0 ] neighbour is blind to this shape: a blank line makes the count 1, so it reads as protection and is not. Measured on bash 3.2.57. That path cannot fire the bug today, because list-tests.sh prints the runnable set rather than the quarantined one, so emptying its quarantine maximises output rather than emptying it. The guard is there so it stays that way.
  • "${SKIP_ARGS[@]}" is now ${SKIP_ARGS[@]+"${SKIP_ARGS[@]}"}. The array is genuinely empty for the first time, and on bash 3.2 under set -u the plain form is an "unbound variable" error. This step runs bash -e today, but sibling steps in the same file already use set -euo pipefail.

Verification

  • verify.sh build PASS
  • verify.sh test EditorTabStripChromeTests EditorTabStripSurfacesTests PASS, 21 of 21
  • actionlint clean on both changed workflows, which is what Repo Hygiene runs and is the only thing that checks an inline run: block
  • Simulated the whole new unit test block against an empty, a valid and an inert quarantine file, and confirmed the exit status in each
  • python3 scripts/ci/test_quarantine_args.py passes here and fails against the version on main
  • Reproduced the CI failure locally against the real script before fixing it: empty list, stdout is exactly \n, and the read loop builds one empty argument

I could not build against Xcode 26.4.1: only the beta is installed here, which is the whole reason the first bug reached CI. CI is the check for that one.

No CHANGELOG entry: neither change alters anything a user sees. quinaryLabel and quinaryLabelColor are the same colour.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit 3896f69 into main Aug 26, 2026
4 of 7 checks passed
@datlechin
datlechin deleted the fix/unbreak-main-ci branch August 26, 2026 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant