Preserve Maven toolchains across repeated setup-java runs (#1099)#1111
Conversation
Toolchain generation was gated behind the `overwrite-settings` input, which is documented to control only regeneration of `settings.xml`. Because `generateToolchainDefinition` already performs a non-destructive merge (existing JDK, custom, and user-managed toolchains are preserved, and only an entry with the same `type` + `provides.id` is replaced), skipping the write when `overwrite-settings: false` caused later setup-java executions to drop toolchain entries registered by earlier runs. Decouple toolchains generation from `overwrite-settings`: the toolchains file is now always written, so consecutive runs accumulate every JDK. `settings.xml` behavior (auth.ts) is unchanged. - src/toolchains.ts: drop overwriteSettings from configureToolchains / createToolchainsSettings / writeToolchainsFileToDisk; always write. - __tests__/toolchains.test.ts: update call sites, rewrite the "does not overwrite" test to assert non-destructive extension, and add a regression test for consecutive configureToolchains executions. - docs/advanced-usage.md: clarify merge is non-destructive and independent of overwrite-settings. - dist/setup/index.js: rebuilt. Fixes #1099 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0bcec457-b86b-4902-b6e6-6dfd6b2570f7
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where running actions/setup-java multiple times within the same job could cause Maven ~/.m2/toolchains.xml to lose previously-registered JDK toolchain entries. It makes toolchains persistence independent of the overwrite-settings input (which is intended to control settings.xml only), relying on the existing non-destructive merge behavior when generating toolchains.
Changes:
- Always write
toolchains.xmlafter merging, regardless ofoverwrite-settings. - Update and expand toolchains tests, including a multi-run regression test ensuring all JDK entries are preserved without duplicates.
- Clarify the non-destructive merge behavior in advanced usage docs; rebuild
dist.
Show a summary per file
| File | Description |
|---|---|
src/toolchains.ts |
Removes overwrite-settings gating so toolchains are always persisted after merge. |
__tests__/toolchains.test.ts |
Updates call sites and adds regression coverage for repeated runs preserving all toolchains. |
docs/advanced-usage.md |
Documents that toolchains merging is non-destructive and independent of overwrite-settings. |
dist/setup/index.js |
Rebuilt distribution output reflecting the source changes. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/4 changed files
- Comments generated: 2
- Review effort level: Low
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@lmvysakh can you please review this? |
|
@brunoborges This PR will raise a conflict to my #1109 one I guess. |
|
Hy @brunoborges , Reviewed and Approved PR. |
Summary
Fixes #1099. Running
setup-javamultiple times in the same job (matrix builds or multiplejava-versionvalues) could drop toolchain entries registered by earlier runs, leaving only one JDK in~/.m2/toolchains.xml.Root cause
Toolchains generation was gated behind the
overwrite-settingsinput. Whenoverwrite-settings: false,writeToolchainsFileToDiskskipped writing entirely if the file already existed — so a secondsetup-javarun never persisted its JDK, silently discarding the merged result.This gating was inappropriate because:
overwrite-settingsis documented (inaction.yml) to control regeneration ofsettings.xmlonly.generateToolchainDefinitionalready performs a non-destructive merge: existing JDK, custom, and user-managed toolchains are preserved, and only an entry with the exact sametype+provides.idis replaced.So writing the toolchains file is always safe and should not be blocked.
Changes
src/toolchains.ts: RemoveoverwriteSettingsfromconfigureToolchains,createToolchainsSettings, andwriteToolchainsFileToDisk. The toolchains file is now always written (non-destructively).settings.xmlbehavior inauth.tsis unchanged.__tests__/toolchains.test.ts: Update call sites, rewrite the "does not overwrite" test to assert non-destructive extension, and add a regression test that runsconfigureToolchainsthree times and asserts every JDK is preserved with no duplicates.docs/advanced-usage.md: Clarify that the merge is non-destructive and independent ofoverwrite-settings.dist/setup/index.js: Rebuilt.Testing
npm run build,npm run format-check,npm run lintall pass.npm test: 909 passing, including the new regression test.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com