Visitar URL original
Preserve Maven toolchains across repeated setup-java runs (#1099) (#1… · actions/setup-java@71cfe8e · GitHub
Skip to content

Commit 71cfe8e

Browse files
brunoborgesCopilotCopilot
authored
Preserve Maven toolchains across repeated setup-java runs (#1099) (#1111)
* Preserve Maven toolchains across repeated setup-java runs (#1099) 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 * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent eb6157c commit 71cfe8e

4 files changed

Lines changed: 101 additions & 68 deletions

File tree

‎__tests__/toolchains.test.ts‎

Lines changed: 73 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ describe('toolchains tests', () => {
8686

8787
await toolchains.createToolchainsSettings({
8888
jdkInfo,
89-
settingsDirectory: altHome,
90-
overwriteSettings: true
89+
settingsDirectory: altHome
9190
});
9291

9392
expect(fs.existsSync(m2Dir)).toBe(false);
@@ -135,8 +134,7 @@ describe('toolchains tests', () => {
135134

136135
await toolchains.createToolchainsSettings({
137136
jdkInfo,
138-
settingsDirectory: m2Dir,
139-
overwriteSettings: true
137+
settingsDirectory: m2Dir
140138
});
141139

142140
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -217,8 +215,7 @@ describe('toolchains tests', () => {
217215

218216
await toolchains.createToolchainsSettings({
219217
jdkInfo,
220-
settingsDirectory: m2Dir,
221-
overwriteSettings: true
218+
settingsDirectory: m2Dir
222219
});
223220

224221
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -303,8 +300,7 @@ describe('toolchains tests', () => {
303300

304301
await toolchains.createToolchainsSettings({
305302
jdkInfo,
306-
settingsDirectory: m2Dir,
307-
overwriteSettings: true
303+
settingsDirectory: m2Dir
308304
});
309305

310306
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -381,8 +377,7 @@ describe('toolchains tests', () => {
381377

382378
await toolchains.createToolchainsSettings({
383379
jdkInfo,
384-
settingsDirectory: m2Dir,
385-
overwriteSettings: true
380+
settingsDirectory: m2Dir
386381
});
387382

388383
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -452,8 +447,7 @@ describe('toolchains tests', () => {
452447

453448
await toolchains.createToolchainsSettings({
454449
jdkInfo,
455-
settingsDirectory: m2Dir,
456-
overwriteSettings: true
450+
settingsDirectory: m2Dir
457451
});
458452

459453
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -545,8 +539,7 @@ describe('toolchains tests', () => {
545539

546540
await toolchains.createToolchainsSettings({
547541
jdkInfo,
548-
settingsDirectory: m2Dir,
549-
overwriteSettings: true
542+
settingsDirectory: m2Dir
550543
});
551544

552545
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -605,8 +598,7 @@ describe('toolchains tests', () => {
605598

606599
await toolchains.createToolchainsSettings({
607600
jdkInfo,
608-
settingsDirectory: m2Dir,
609-
overwriteSettings: true
601+
settingsDirectory: m2Dir
610602
});
611603

612604
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -664,8 +656,7 @@ describe('toolchains tests', () => {
664656

665657
await toolchains.createToolchainsSettings({
666658
jdkInfo,
667-
settingsDirectory: m2Dir,
668-
overwriteSettings: true
659+
settingsDirectory: m2Dir
669660
});
670661

671662
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -748,8 +739,7 @@ describe('toolchains tests', () => {
748739

749740
await toolchains.createToolchainsSettings({
750741
jdkInfo,
751-
settingsDirectory: m2Dir,
752-
overwriteSettings: true
742+
settingsDirectory: m2Dir
753743
});
754744

755745
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -828,8 +818,7 @@ describe('toolchains tests', () => {
828818

829819
await toolchains.createToolchainsSettings({
830820
jdkInfo,
831-
settingsDirectory: m2Dir,
832-
overwriteSettings: true
821+
settingsDirectory: m2Dir
833822
});
834823

835824
expect(fs.existsSync(m2Dir)).toBe(true);
@@ -854,7 +843,7 @@ describe('toolchains tests', () => {
854843
).toEqual(result);
855844
}, 100000);
856845

857-
it('does not overwrite existing toolchains.xml files', async () => {
846+
it('extends existing toolchains.xml files instead of overwriting them', async () => {
858847
const jdkInfo = {
859848
version: '17',
860849
vendor: 'Eclipse Temurin',
@@ -883,13 +872,20 @@ describe('toolchains tests', () => {
883872

884873
await toolchains.createToolchainsSettings({
885874
jdkInfo,
886-
settingsDirectory: m2Dir,
887-
overwriteSettings: false
875+
settingsDirectory: m2Dir
888876
});
889877

890878
expect(fs.existsSync(m2Dir)).toBe(true);
891879
expect(fs.existsSync(toolchainsFile)).toBe(true);
892-
expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual(originalFile);
880+
881+
const updated = fs.readFileSync(toolchainsFile, 'utf-8');
882+
// The pre-existing (Sun 1.6) toolchain must be preserved ...
883+
expect(updated).toContain('<id>sun_1.6</id>');
884+
expect(updated).toContain('<jdkHome>/opt/jdk/sun/1.6</jdkHome>');
885+
// ... and the newly installed JDK must be appended.
886+
expect(updated).toContain('<id>temurin_17</id>');
887+
expect(updated).toContain('<vendor>Eclipse Temurin</vendor>');
888+
expect(updated).toContain(`<jdkHome>${jdkInfo.jdkHome}</jdkHome>`);
893889
}, 100000);
894890

895891
it('generates valid toolchains.xml with minimal configuration', () => {
@@ -959,4 +955,55 @@ describe('toolchains tests', () => {
959955
)
960956
);
961957
}, 100000);
958+
959+
it('preserves toolchains from previous executions across multiple setup-java runs', async () => {
960+
// Regression test for https://github.com/actions/setup-java/issues/1099
961+
// Running setup-java several times in the same job (e.g. multiple steps / multiple
962+
// java-version entries) must accumulate every JDK in toolchains.xml rather
963+
// than replacing previously registered entries.
964+
(core.getInput as jest.Mock<any>).mockImplementation((name: string) => {
965+
if (name === 'settings-path') return m2Dir;
966+
return '';
967+
});
968+
969+
const runs = [
970+
{
971+
version: '8',
972+
distributionName: 'temurin',
973+
id: 'temurin_8',
974+
jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.1-12/x64'
975+
},
976+
{
977+
version: '11',
978+
distributionName: 'temurin',
979+
id: 'temurin_11',
980+
jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.1-12/x64'
981+
},
982+
{
983+
version: '17',
984+
distributionName: 'temurin',
985+
id: 'temurin_17',
986+
jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64'
987+
}
988+
];
989+
990+
for (const run of runs) {
991+
await toolchains.configureToolchains(
992+
run.version,
993+
run.distributionName,
994+
run.jdkHome,
995+
undefined
996+
);
997+
}
998+
999+
expect(fs.existsSync(toolchainsFile)).toBe(true);
1000+
const contents = fs.readFileSync(toolchainsFile, 'utf-8');
1001+
1002+
for (const run of runs) {
1003+
expect(contents).toContain(`<id>${run.id}</id>`);
1004+
expect(contents).toContain(`<jdkHome>${run.jdkHome}</jdkHome>`);
1005+
}
1006+
// Exactly one <toolchain> entry per run – no duplicates, none dropped.
1007+
expect((contents.match(/<toolchain>/g) || []).length).toBe(runs.length);
1008+
}, 100000);
9621009
});

‎dist/setup/index.js‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127351,32 +127351,29 @@ async function write(directory, settings, overwriteSettings) {
127351127351

127352127352

127353127353

127354-
127355127354
async function configureToolchains(version, distributionName, jdkHome, toolchainId) {
127356127355
const vendor = getInput(INPUT_MVN_TOOLCHAIN_VENDOR) || distributionName;
127357127356
const id = toolchainId || `${vendor}_${version}`;
127358127357
const settingsDirectory = getInput(INPUT_SETTINGS_PATH) ||
127359127358
external_path_.join(external_os_.homedir(), M2_DIR);
127360-
const overwriteSettings = util_getBooleanInput(INPUT_OVERWRITE_SETTINGS, true);
127361127359
await createToolchainsSettings({
127362127360
jdkInfo: {
127363127361
version,
127364127362
vendor,
127365127363
id,
127366127364
jdkHome
127367127365
},
127368-
settingsDirectory,
127369-
overwriteSettings
127366+
settingsDirectory
127370127367
});
127371127368
}
127372-
async function createToolchainsSettings({ jdkInfo, settingsDirectory, overwriteSettings }) {
127369+
async function createToolchainsSettings({ jdkInfo, settingsDirectory }) {
127373127370
info(`Creating ${MVN_TOOLCHAINS_FILE} for JDK version ${jdkInfo.version} from ${jdkInfo.vendor}`);
127374127371
// when an alternate m2 location is specified use only that location (no .m2 directory)
127375127372
// otherwise use the home/.m2/ path
127376127373
await mkdirP(settingsDirectory);
127377127374
const originalToolchains = await readExistingToolchainsFile(settingsDirectory);
127378127375
const updatedToolchains = generateToolchainDefinition(originalToolchains, jdkInfo.version, jdkInfo.vendor, jdkInfo.id, jdkInfo.jdkHome);
127379-
await writeToolchainsFileToDisk(settingsDirectory, updatedToolchains, overwriteSettings);
127376+
await writeToolchainsFileToDisk(settingsDirectory, updatedToolchains);
127380127377
}
127381127378
// only exported for testing purposes
127382127379
function generateToolchainDefinition(original, version, vendor, id, jdkHome) {
@@ -127458,18 +127455,20 @@ async function readExistingToolchainsFile(directory) {
127458127455
}
127459127456
return '';
127460127457
}
127461-
async function writeToolchainsFileToDisk(directory, settings, overwriteSettings) {
127458+
async function writeToolchainsFileToDisk(directory, settings) {
127462127459
const location = external_path_.join(directory, MVN_TOOLCHAINS_FILE);
127463127460
const settingsExists = external_fs_namespaceObject.existsSync(location);
127464-
if (settingsExists && overwriteSettings) {
127465-
info(`Overwriting existing file ${location}`);
127466-
}
127467-
else if (!settingsExists) {
127468-
info(`Writing to ${location}`);
127461+
// The toolchains file is produced by a non-destructive merge (existing JDK,
127462+
// custom, and non-jdk toolchains are preserved – see generateToolchainDefinition),
127463+
// so it is always safe to write it. Unlike settings.xml, it is therefore not
127464+
// gated behind the `overwrite-settings` input; that would prevent subsequent
127465+
// setup-java runs from registering additional JDKs and silently drop the
127466+
// toolchain entries created by earlier runs.
127467+
if (settingsExists) {
127468+
info(`Updating existing file ${location}`);
127469127469
}
127470127470
else {
127471-
info(`Skipping generation of ${location} because file already exists and overwriting is not enabled`);
127472-
return;
127471+
info(`Writing to ${location}`);
127473127472
}
127474127473
return external_fs_namespaceObject.writeFileSync(location, settings, {
127475127474
encoding: 'utf-8',

‎docs/advanced-usage.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,8 @@ The `setup-java` action generates a basic [Maven Toolchains declaration](https:/
801801
### Installing Multiple JDKs With Toolchains
802802
Subsequent calls to `setup-java` with distinct distribution and version parameters will continue to extend the toolchains declaration and make all specified Java versions available.
803803

804+
Toolchain entries are always merged non-destructively: existing JDK, custom, and user-managed toolchains are preserved, and only an entry with the exact same `type` and `provides.id` is replaced. This behavior is independent of the `overwrite-settings` input, which only controls regeneration of `settings.xml`. As a result, running `setup-java` several times in the same job (for example in multiple steps or with multiple `java-version` values) accumulates every JDK in `toolchains.xml` instead of dropping previously registered entries.
805+
804806
```yaml
805807
steps:
806808
- uses: actions/setup-java@v5

‎src/toolchains.ts‎

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import * as core from '@actions/core';
55
import * as io from '@actions/io';
66
import * as constants from './constants.js';
77

8-
import {getBooleanInput} from './util.js';
98
import {create as xmlCreate} from 'xmlbuilder2';
109

1110
interface JdkInfo {
@@ -27,10 +26,6 @@ export async function configureToolchains(
2726
const settingsDirectory =
2827
core.getInput(constants.INPUT_SETTINGS_PATH) ||
2928
path.join(os.homedir(), constants.M2_DIR);
30-
const overwriteSettings = getBooleanInput(
31-
constants.INPUT_OVERWRITE_SETTINGS,
32-
true
33-
);
3429

3530
await createToolchainsSettings({
3631
jdkInfo: {
@@ -39,19 +34,16 @@ export async function configureToolchains(
3934
id,
4035
jdkHome
4136
},
42-
settingsDirectory,
43-
overwriteSettings
37+
settingsDirectory
4438
});
4539
}
4640

4741
export async function createToolchainsSettings({
4842
jdkInfo,
49-
settingsDirectory,
50-
overwriteSettings
43+
settingsDirectory
5144
}: {
5245
jdkInfo: JdkInfo;
5346
settingsDirectory: string;
54-
overwriteSettings: boolean;
5547
}) {
5648
core.info(
5749
`Creating ${constants.MVN_TOOLCHAINS_FILE} for JDK version ${jdkInfo.version} from ${jdkInfo.vendor}`
@@ -68,11 +60,7 @@ export async function createToolchainsSettings({
6860
jdkInfo.id,
6961
jdkInfo.jdkHome
7062
);
71-
await writeToolchainsFileToDisk(
72-
settingsDirectory,
73-
updatedToolchains,
74-
overwriteSettings
75-
);
63+
await writeToolchainsFileToDisk(settingsDirectory, updatedToolchains);
7664
}
7765

7866
// only exported for testing purposes
@@ -172,22 +160,19 @@ async function readExistingToolchainsFile(directory: string) {
172160
return '';
173161
}
174162

175-
async function writeToolchainsFileToDisk(
176-
directory: string,
177-
settings: string,
178-
overwriteSettings: boolean
179-
) {
163+
async function writeToolchainsFileToDisk(directory: string, settings: string) {
180164
const location = path.join(directory, constants.MVN_TOOLCHAINS_FILE);
181165
const settingsExists = fs.existsSync(location);
182-
if (settingsExists && overwriteSettings) {
183-
core.info(`Overwriting existing file ${location}`);
184-
} else if (!settingsExists) {
185-
core.info(`Writing to ${location}`);
166+
// The toolchains file is produced by a non-destructive merge (existing JDK,
167+
// custom, and non-jdk toolchains are preserved – see generateToolchainDefinition),
168+
// so it is always safe to write it. Unlike settings.xml, it is therefore not
169+
// gated behind the `overwrite-settings` input; that would prevent subsequent
170+
// setup-java runs from registering additional JDKs and silently drop the
171+
// toolchain entries created by earlier runs.
172+
if (settingsExists) {
173+
core.info(`Updating existing file ${location}`);
186174
} else {
187-
core.info(
188-
`Skipping generation of ${location} because file already exists and overwriting is not enabled`
189-
);
190-
return;
175+
core.info(`Writing to ${location}`);
191176
}
192177

193178
return fs.writeFileSync(location, settings, {

0 commit comments

Comments
 (0)