Skip to content

formula: skip Java splash screens in macOS tests - #24005

Merged
MikeMcQuaid merged 1 commit into
Homebrew:mainfrom
ashr123:java-launcher-splash-headless
Sep 17, 2026
Merged

MikeMcQuaid merged 1 commit into
Homebrew:mainfrom
ashr123:java-launcher-splash-headless

Conversation

@ashr123

@ashr123 ashr123 commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

AI disclosure: Claude Code (Claude Fable 5.1) diagnosed the failure, wrote the patch and the spec changes, and ran brew lgtm and the reproduction below; I reviewed the diff and will handle review comments myself.


Follow-up to #24000 / #24001.

What: adds Formula#test_sandbox_env, the environment of the sandboxed test phase on top of common_sandbox_env, and on macOS sets JDK_JAVA_OPTIONS=-Djava.awt.headless=true there, next to the -Djava.awt.headless=true that #24001 appended to _JAVA_OPTIONS for every phase.

Why: the java launcher decides whether to show a jar's SplashScreen-Image while parsing its own command line, before the JVM exists (java.c#L1356-L1364, #L1422-L1430). It honours only a -Djava.awt.headless=true it parsed itself, from its arguments or from JDK_JAVA_OPTIONS (JDK 9+); _JAVA_OPTIONS is read later by the JVM and cannot switch the splash off. On macOS the splash initialises AppKit (splashscreen_sys.m#L152-L155), which needs a WindowServer connection and a LaunchServices check-in, so under (deny mach-lookup) the process aborts before the application's main runs; a class-load trace shows no AWT class loaded at that point. brew test jpdfbookmarks (its jar manifest has SplashScreen-Image) is so far the only failure in Homebrew/homebrew-core#311429: it fails on all three macOS lanes in the dependents shard that runs it, with nothing printed after the JVM's Picked up _JAVA_OPTIONS: … line, and it fails identically locally with openjdk 26.0.2.1, so it is unrelated to the JDK version.

Why only the test phase: OpenJDK's configure probes the boot JDK with java -version 2>&1 and aborts with "Cannot continue" when that output contains "Picked up" (boot-jdk.m4#L79-L86). The openjdk, openjdk@11/17/21/25, graalvm and openj9 formulae clear only _JAVA_OPTIONS before configuring, so setting JDK_JAVA_OPTIONS in common_sandbox_env (my first revision of this PR) would have broken every macOS source build of a JDK. The test phase never configures a JDK, and it is where the splash failure happens.

Reproduction (macOS 26.5, Homebrew 7.0.3 with #24001, openjdk 26.0.2.1, jpdfbookmarks 2.5.2):

brew install jpdfbookmarks
brew test --verbose jpdfbookmarks   # before: JVM exit 134 after the "Picked up _JAVA_OPTIONS" line, "Error: jpdfbookmarks: failed"; with this patch: passes

Standalone, same command as the formula test, under the exact profile brew test generates (sandbox-exec -f profile.sb):

environment result
_JAVA_OPTIONS=-Djava.awt.headless=true (current) exit 134, no PDF
JDK_JAVA_OPTIONS=-Djava.awt.headless=true exit 0, PDF written
java -Djava.awt.headless=true -jar … exit 0, PDF written
current env plus mach-lookup allowed for com.apple.windowserver.active and com.apple.coreservices.launchservicesd exit 0 (either service alone still crashes: exit 134 and 133)

The last row is the alternative I did not take: it would widen the sandbox for every process to work around something the launcher already knows how to switch off, whereas this keeps (deny mach-lookup) untouched.

Side effects: the launcher prints NOTE: Picked up JDK_JAVA_OPTIONS: -Djava.awt.headless=true to stderr during tests, the same way the JVM already prints Picked up _JAVA_OPTIONS: …. Embedded JVMs that bypass the launcher are unaffected. The JDK 8 launcher has no JDK_JAVA_OPTIONS, so a splash jar tested on openjdk@8 would still abort; none does in homebrew-core today.

Tests: test/formula_spec.rb asserts common_sandbox_env has no launcher option; test/os/mac/formula_spec.rb asserts the exact JDK_JAVA_OPTIONS value in test_sandbox_env and that it keeps the common environment; test/os/linux/formula_spec.rb asserts the key is absent on Linux.

Copilot AI balanced review requested due to automatic review settings September 16, 2026 19:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation matches OpenJDK launcher behaviour and includes appropriate platform-specific tests.

Pull request overview

Ensures Java JAR splash screens do not access unavailable macOS window services during sandboxed formula phases.

Changes:

  • Adds launcher-level headless configuration on macOS.
  • Adds macOS and Linux coverage for platform-specific behaviour.
File summaries
File Description
Library/Homebrew/extend/os/mac/formula.rb Sets headless options for the JVM and Java launcher.
Library/Homebrew/test/os/mac/formula_spec.rb Verifies the macOS launcher option.
Library/Homebrew/test/os/linux/formula_spec.rb Verifies Linux remains unchanged.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@ashr123

ashr123 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@MikeMcQuaid can you review this PR?

@ashr123
ashr123 marked this pull request as draft September 16, 2026 20:03
- The `java` launcher decides from the options it parses itself, its
  arguments and `JDK_JAVA_OPTIONS`, whether to show a jar's
  `SplashScreen-Image`. `_JAVA_OPTIONS` is only read by the JVM, so the
  headless option from Homebrew#24001 cannot switch the splash off.
- On macOS the splash brings up AppKit, which needs the WindowServer and
  a LaunchServices check-in, so under the sandbox's `(deny mach-lookup)`
  the process aborts: `brew test jpdfbookmarks` fails with the JVM
  exiting 134 and no Java error printed.
- Add `Formula#test_sandbox_env` and set `JDK_JAVA_OPTIONS` there on
  macOS only. OpenJDK's `configure` refuses a boot JDK whose `-version`
  output reports picked-up options, and the JDK formulae clear only
  `_JAVA_OPTIONS` before building, so the build phase must stay as is.
- Cover the shared, macOS and Linux behaviour in the `Formula` specs.
@ashr123
ashr123 force-pushed the java-launcher-splash-headless branch from b9bf275 to 12ec3aa Compare September 16, 2026 20:10
@ashr123
ashr123 marked this pull request as ready for review September 16, 2026 20:10
@ashr123 ashr123 changed the title formula: skip Java splash screens in macOS sandboxed phases formula: skip Java splash screens in macOS tests Sep 16, 2026
@ashr123
ashr123 requested a balanced review from Copilot September 16, 2026 20:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The phase-specific implementation is focused, correctly composed and adequately tested across platforms.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@ashr123 ashr123 mentioned this pull request Sep 16, 2026
5 of 8 tasks

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Sep 17, 2026
Merged via the queue into Homebrew:main with commit 0df959b Sep 17, 2026
53 checks passed
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.

3 participants