diff --git a/__tests__/toolchains.test.ts b/__tests__/toolchains.test.ts index c751391..fe523aa 100644 --- a/__tests__/toolchains.test.ts +++ b/__tests__/toolchains.test.ts @@ -86,8 +86,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: altHome, - overwriteSettings: true + settingsDirectory: altHome }); expect(fs.existsSync(m2Dir)).toBe(false); @@ -135,8 +134,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -217,8 +215,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -303,8 +300,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -381,8 +377,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -452,8 +447,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -545,8 +539,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -605,8 +598,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -664,8 +656,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -748,8 +739,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -828,8 +818,7 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: true + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); @@ -854,7 +843,7 @@ describe('toolchains tests', () => { ).toEqual(result); }, 100000); - it('does not overwrite existing toolchains.xml files', async () => { + it('extends existing toolchains.xml files instead of overwriting them', async () => { const jdkInfo = { version: '17', vendor: 'Eclipse Temurin', @@ -883,13 +872,20 @@ describe('toolchains tests', () => { await toolchains.createToolchainsSettings({ jdkInfo, - settingsDirectory: m2Dir, - overwriteSettings: false + settingsDirectory: m2Dir }); expect(fs.existsSync(m2Dir)).toBe(true); expect(fs.existsSync(toolchainsFile)).toBe(true); - expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual(originalFile); + + const updated = fs.readFileSync(toolchainsFile, 'utf-8'); + // The pre-existing (Sun 1.6) toolchain must be preserved ... + expect(updated).toContain('sun_1.6'); + expect(updated).toContain('/opt/jdk/sun/1.6'); + // ... and the newly installed JDK must be appended. + expect(updated).toContain('temurin_17'); + expect(updated).toContain('Eclipse Temurin'); + expect(updated).toContain(`${jdkInfo.jdkHome}`); }, 100000); it('generates valid toolchains.xml with minimal configuration', () => { @@ -959,4 +955,55 @@ describe('toolchains tests', () => { ) ); }, 100000); + + it('preserves toolchains from previous executions across multiple setup-java runs', async () => { + // Regression test for https://github.com/actions/setup-java/issues/1099 + // Running setup-java several times in the same job (e.g. multiple steps / multiple + // java-version entries) must accumulate every JDK in toolchains.xml rather + // than replacing previously registered entries. + (core.getInput as jest.Mock).mockImplementation((name: string) => { + if (name === 'settings-path') return m2Dir; + return ''; + }); + + const runs = [ + { + version: '8', + distributionName: 'temurin', + id: 'temurin_8', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.1-12/x64' + }, + { + version: '11', + distributionName: 'temurin', + id: 'temurin_11', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.1-12/x64' + }, + { + version: '17', + distributionName: 'temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + } + ]; + + for (const run of runs) { + await toolchains.configureToolchains( + run.version, + run.distributionName, + run.jdkHome, + undefined + ); + } + + expect(fs.existsSync(toolchainsFile)).toBe(true); + const contents = fs.readFileSync(toolchainsFile, 'utf-8'); + + for (const run of runs) { + expect(contents).toContain(`${run.id}`); + expect(contents).toContain(`${run.jdkHome}`); + } + // Exactly one entry per run – no duplicates, none dropped. + expect((contents.match(//g) || []).length).toBe(runs.length); + }, 100000); }); diff --git a/dist/setup/index.js b/dist/setup/index.js index b3e2cb6..adce3fe 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -127351,13 +127351,11 @@ async function write(directory, settings, overwriteSettings) { - async function configureToolchains(version, distributionName, jdkHome, toolchainId) { const vendor = getInput(INPUT_MVN_TOOLCHAIN_VENDOR) || distributionName; const id = toolchainId || `${vendor}_${version}`; const settingsDirectory = getInput(INPUT_SETTINGS_PATH) || external_path_.join(external_os_.homedir(), M2_DIR); - const overwriteSettings = util_getBooleanInput(INPUT_OVERWRITE_SETTINGS, true); await createToolchainsSettings({ jdkInfo: { version, @@ -127365,18 +127363,17 @@ async function configureToolchains(version, distributionName, jdkHome, toolchain id, jdkHome }, - settingsDirectory, - overwriteSettings + settingsDirectory }); } -async function createToolchainsSettings({ jdkInfo, settingsDirectory, overwriteSettings }) { +async function createToolchainsSettings({ jdkInfo, settingsDirectory }) { info(`Creating ${MVN_TOOLCHAINS_FILE} for JDK version ${jdkInfo.version} from ${jdkInfo.vendor}`); // when an alternate m2 location is specified use only that location (no .m2 directory) // otherwise use the home/.m2/ path await mkdirP(settingsDirectory); const originalToolchains = await readExistingToolchainsFile(settingsDirectory); const updatedToolchains = generateToolchainDefinition(originalToolchains, jdkInfo.version, jdkInfo.vendor, jdkInfo.id, jdkInfo.jdkHome); - await writeToolchainsFileToDisk(settingsDirectory, updatedToolchains, overwriteSettings); + await writeToolchainsFileToDisk(settingsDirectory, updatedToolchains); } // only exported for testing purposes function generateToolchainDefinition(original, version, vendor, id, jdkHome) { @@ -127458,18 +127455,20 @@ async function readExistingToolchainsFile(directory) { } return ''; } -async function writeToolchainsFileToDisk(directory, settings, overwriteSettings) { +async function writeToolchainsFileToDisk(directory, settings) { const location = external_path_.join(directory, MVN_TOOLCHAINS_FILE); const settingsExists = external_fs_namespaceObject.existsSync(location); - if (settingsExists && overwriteSettings) { - info(`Overwriting existing file ${location}`); - } - else if (!settingsExists) { - info(`Writing to ${location}`); + // The toolchains file is produced by a non-destructive merge (existing JDK, + // custom, and non-jdk toolchains are preserved – see generateToolchainDefinition), + // so it is always safe to write it. Unlike settings.xml, it is therefore not + // gated behind the `overwrite-settings` input; that would prevent subsequent + // setup-java runs from registering additional JDKs and silently drop the + // toolchain entries created by earlier runs. + if (settingsExists) { + info(`Updating existing file ${location}`); } else { - info(`Skipping generation of ${location} because file already exists and overwriting is not enabled`); - return; + info(`Writing to ${location}`); } return external_fs_namespaceObject.writeFileSync(location, settings, { encoding: 'utf-8', diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index db300a7..733c0a2 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -801,6 +801,8 @@ The `setup-java` action generates a basic [Maven Toolchains declaration](https:/ ### Installing Multiple JDKs With Toolchains 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. +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. + ```yaml steps: - uses: actions/setup-java@v5 diff --git a/src/toolchains.ts b/src/toolchains.ts index 0e1d15d..2cdaab3 100644 --- a/src/toolchains.ts +++ b/src/toolchains.ts @@ -5,7 +5,6 @@ import * as core from '@actions/core'; import * as io from '@actions/io'; import * as constants from './constants.js'; -import {getBooleanInput} from './util.js'; import {create as xmlCreate} from 'xmlbuilder2'; interface JdkInfo { @@ -27,10 +26,6 @@ export async function configureToolchains( const settingsDirectory = core.getInput(constants.INPUT_SETTINGS_PATH) || path.join(os.homedir(), constants.M2_DIR); - const overwriteSettings = getBooleanInput( - constants.INPUT_OVERWRITE_SETTINGS, - true - ); await createToolchainsSettings({ jdkInfo: { @@ -39,19 +34,16 @@ export async function configureToolchains( id, jdkHome }, - settingsDirectory, - overwriteSettings + settingsDirectory }); } export async function createToolchainsSettings({ jdkInfo, - settingsDirectory, - overwriteSettings + settingsDirectory }: { jdkInfo: JdkInfo; settingsDirectory: string; - overwriteSettings: boolean; }) { core.info( `Creating ${constants.MVN_TOOLCHAINS_FILE} for JDK version ${jdkInfo.version} from ${jdkInfo.vendor}` @@ -68,11 +60,7 @@ export async function createToolchainsSettings({ jdkInfo.id, jdkInfo.jdkHome ); - await writeToolchainsFileToDisk( - settingsDirectory, - updatedToolchains, - overwriteSettings - ); + await writeToolchainsFileToDisk(settingsDirectory, updatedToolchains); } // only exported for testing purposes @@ -172,22 +160,19 @@ async function readExistingToolchainsFile(directory: string) { return ''; } -async function writeToolchainsFileToDisk( - directory: string, - settings: string, - overwriteSettings: boolean -) { +async function writeToolchainsFileToDisk(directory: string, settings: string) { const location = path.join(directory, constants.MVN_TOOLCHAINS_FILE); const settingsExists = fs.existsSync(location); - if (settingsExists && overwriteSettings) { - core.info(`Overwriting existing file ${location}`); - } else if (!settingsExists) { - core.info(`Writing to ${location}`); + // The toolchains file is produced by a non-destructive merge (existing JDK, + // custom, and non-jdk toolchains are preserved – see generateToolchainDefinition), + // so it is always safe to write it. Unlike settings.xml, it is therefore not + // gated behind the `overwrite-settings` input; that would prevent subsequent + // setup-java runs from registering additional JDKs and silently drop the + // toolchain entries created by earlier runs. + if (settingsExists) { + core.info(`Updating existing file ${location}`); } else { - core.info( - `Skipping generation of ${location} because file already exists and overwriting is not enabled` - ); - return; + core.info(`Writing to ${location}`); } return fs.writeFileSync(location, settings, {