From 36b7a1cb2f604bded95522a7b8229d0d20da9b4e Mon Sep 17 00:00:00 2001 From: Nikolas Grottendieck Date: Wed, 20 Sep 2023 00:21:55 +0200 Subject: [PATCH 1/2] fix: Maven Toolchains grows unexpectedly On self-hosted runners toolchains.xml may survive multiple runs and unexpectedly grow as a result of the toolchains setup simply appending the JDK definition even if one with the same `type` and `provides.id` already exists. Restructuring the parsing step and filtering the potentially existing list of toolchain definitions prevents this and also fixes toolchain.xml files that already contain duplicates. Fixes #530 --- __tests__/toolchains.test.ts | 451 ++++++++++++++++++++++++++++++++++- dist/setup/index.js | 79 +++--- src/toolchains.ts | 105 +++++--- 3 files changed, 561 insertions(+), 74 deletions(-) diff --git a/__tests__/toolchains.test.ts b/__tests__/toolchains.test.ts index 483077dc1..4aa1c1874 100644 --- a/__tests__/toolchains.test.ts +++ b/__tests__/toolchains.test.ts @@ -143,18 +143,467 @@ describe('toolchains tests', () => { `; const result = ` - + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + + + jdk + + 1.6 + Sun + sun_1.6 + + + /opt/jdk/sun/1.6 + + +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + + it('does not discard custom elements in existing toolchain definitions', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ` + + jdk + + 1.6 + Sun + sun_1.6 + foo + + + /opt/jdk/sun/1.6 + /usr/local/bin/bash + + + `; + const result = ` + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + jdk 1.6 Sun sun_1.6 + foo /opt/jdk/sun/1.6 + /usr/local/bin/bash +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + + it('does not discard existing, custom toolchain definitions', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ` + + foo + + baz + + + /usr/local/bin/foo + + + `; + const result = ` + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + + + foo + + baz + + + /usr/local/bin/foo + + +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + + it('does not duplicate existing toolchain definitions', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ` + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + + `; + const result = ` + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + + it('does not duplicate existing toolchain definitions if multiple exist', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ` + + jdk + + 1.6 + Sun + sun_1.6 + + + /opt/jdk/sun/1.6 + + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + + `; + const result = ` + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + + + jdk + + 1.6 + Sun + sun_1.6 + + + /opt/jdk/sun/1.6 + + +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + + it('handles an empty list of existing toolchains correctly', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ` + `; + const result = ` + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + + it('handles an empty existing toolchains.xml correctly', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ``; + const result = ` + jdk diff --git a/dist/setup/index.js b/dist/setup/index.js index 434039045..d619a3d65 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -80911,46 +80911,53 @@ function createToolchainsSettings({ jdkInfo, settingsDirectory, overwriteSetting exports.createToolchainsSettings = createToolchainsSettings; // only exported for testing purposes function generateToolchainDefinition(original, version, vendor, id, jdkHome) { - let xmlObj; + let jsToolchains = [ + { + type: 'jdk', + provides: { + version: `${version}`, + vendor: `${vendor}`, + id: `${id}` + }, + configuration: { + jdkHome: `${jdkHome}` + } + } + ]; if (original === null || original === void 0 ? void 0 : original.length) { - xmlObj = (0, xmlbuilder2_1.create)(original) + // convert existing toolchains into TS native objects for better handling + // xmlbuilder2 will convert the document into a `{toolchains: { toolchain: [] | {} }}` structure + // instead of the desired `toolchains: [{}]` one or simply `[{}]` + const jsObj = (0, xmlbuilder2_1.create)(original) .root() - .ele({ - toolchain: { - type: 'jdk', - provides: { - version: `${version}`, - vendor: `${vendor}`, - id: `${id}` - }, - configuration: { - jdkHome: `${jdkHome}` - } + .toObject(); + if (jsObj.toolchains && jsObj.toolchains.toolchain) { + // in case only a single child exists xmlbuilder2 will not create an array and using verbose = true equally doesn't work here + // See https://oozcitak.github.io/xmlbuilder2/serialization.html#js-object-and-map-serializers for details + if (Array.isArray(jsObj.toolchains.toolchain)) { + jsToolchains.push(...jsObj.toolchains.toolchain); } - }); - } - else - xmlObj = (0, xmlbuilder2_1.create)({ - toolchains: { - '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', - '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', - '@xsi:schemaLocation': 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd', - toolchain: [ - { - type: 'jdk', - provides: { - version: `${version}`, - vendor: `${vendor}`, - id: `${id}` - }, - configuration: { - jdkHome: `${jdkHome}` - } - } - ] + else { + jsToolchains.push(jsObj.toolchains.toolchain); } - }); - return xmlObj.end({ + } + // remove potential duplicates based on type & id (which should be a unique combination); + // self.findIndex will only return the first occurrence, ensuring duplicates are skipped + jsToolchains = jsToolchains.filter((value, index, self) => + // ensure non-jdk toolchains are kept in the results, we must not touch them because they belong to the user + value.type !== 'jdk' || + index === + self.findIndex(t => t.type === value.type && t.provides.id === value.provides.id)); + } + // TODO: technically bad because we shouldn't re-create the toolchains root node (with possibly different schema values) if it already exists, however, just overriding the toolchain array with xmlbuilder2 is … uh non-trivial + return (0, xmlbuilder2_1.create)({ + toolchains: { + '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', + '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', + '@xsi:schemaLocation': 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd', + toolchain: jsToolchains + } + }).end({ format: 'xml', wellFormed: false, headless: false, diff --git a/src/toolchains.ts b/src/toolchains.ts index 77cae83f7..e91a68be2 100644 --- a/src/toolchains.ts +++ b/src/toolchains.ts @@ -83,47 +83,59 @@ export function generateToolchainDefinition( id: string, jdkHome: string ) { - let xmlObj; + let jsToolchains: Toolchain[] = [ + { + type: 'jdk', + provides: { + version: `${version}`, + vendor: `${vendor}`, + id: `${id}` + }, + configuration: { + jdkHome: `${jdkHome}` + } + } + ]; if (original?.length) { - xmlObj = xmlCreate(original) + // convert existing toolchains into TS native objects for better handling + // xmlbuilder2 will convert the document into a `{toolchains: { toolchain: [] | {} }}` structure + // instead of the desired `toolchains: [{}]` one or simply `[{}]` + const jsObj = xmlCreate(original) .root() - .ele({ - toolchain: { - type: 'jdk', - provides: { - version: `${version}`, - vendor: `${vendor}`, - id: `${id}` - }, - configuration: { - jdkHome: `${jdkHome}` - } - } - }); - } else - xmlObj = xmlCreate({ - toolchains: { - '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', - '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', - '@xsi:schemaLocation': - 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd', - toolchain: [ - { - type: 'jdk', - provides: { - version: `${version}`, - vendor: `${vendor}`, - id: `${id}` - }, - configuration: { - jdkHome: `${jdkHome}` - } - } - ] + .toObject() as unknown as ExtractedToolchains; + if (jsObj.toolchains && jsObj.toolchains.toolchain) { + // in case only a single child exists xmlbuilder2 will not create an array and using verbose = true equally doesn't work here + // See https://oozcitak.github.io/xmlbuilder2/serialization.html#js-object-and-map-serializers for details + if (Array.isArray(jsObj.toolchains.toolchain)) { + jsToolchains.push(...jsObj.toolchains.toolchain); + } else { + jsToolchains.push(jsObj.toolchains.toolchain); } - }); + } + + // remove potential duplicates based on type & id (which should be a unique combination); + // self.findIndex will only return the first occurrence, ensuring duplicates are skipped + jsToolchains = jsToolchains.filter( + (value, index, self) => + // ensure non-jdk toolchains are kept in the results, we must not touch them because they belong to the user + value.type !== 'jdk' || + index === + self.findIndex( + t => t.type === value.type && t.provides.id === value.provides.id + ) + ); + } - return xmlObj.end({ + // TODO: technically bad because we shouldn't re-create the toolchains root node (with possibly different schema values) if it already exists, however, just overriding the toolchain array with xmlbuilder2 is … uh non-trivial + return xmlCreate({ + toolchains: { + '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', + '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', + '@xsi:schemaLocation': + 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd', + toolchain: jsToolchains + } + }).end({ format: 'xml', wellFormed: false, headless: false, @@ -166,3 +178,22 @@ async function writeToolchainsFileToDisk( flag: 'w' }); } + +interface ExtractedToolchains { + toolchains: { + toolchain: Toolchain[] | Toolchain; + }; +} + +// Toolchain type definition according to Maven Toolchains XSD 1.1.0 +interface Toolchain { + type: string; + provides: + | { + version: string; + vendor: string; + id: string; + } + | any; + configuration: any; +} From 2ef9b3852e909e5d092c690be25d690a849b8ab9 Mon Sep 17 00:00:00 2001 From: Bruno Borges Date: Mon, 6 Jul 2026 15:16:16 -0400 Subject: [PATCH 2/2] fix: guard toolchain dedup and preserve existing root attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address reviewer feedback on the Maven toolchains dedup logic: - Treat jdk toolchains without a string `provides.id` as non-deduplicatable and use optional access when comparing ids, so partially-formed or nonstandard toolchains.xml files no longer crash setup. - Preserve the existing `` root attributes (xmlns, schemaLocation, …) when present, falling back to the 1.1.0 defaults only for attributes the existing file is missing. This avoids silently rewriting user-managed metadata or changing the effective namespace. Adds tests covering custom root attributes and id-less jdk toolchains, and rebuilds dist/. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- __tests__/toolchains.test.ts | 164 +++++++++++++++++++++++++++++++++++ dist/setup/index.js | 53 ++++++----- src/toolchains.ts | 47 +++++++--- 3 files changed, 230 insertions(+), 34 deletions(-) diff --git a/__tests__/toolchains.test.ts b/__tests__/toolchains.test.ts index 4aa1c1874..38cac18ca 100644 --- a/__tests__/toolchains.test.ts +++ b/__tests__/toolchains.test.ts @@ -650,6 +650,170 @@ describe('toolchains tests', () => { ).toEqual(result); }, 100000); + it('preserves custom root attributes on existing toolchains.xml', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ` + + jdk + + 1.6 + Sun + sun_1.6 + + + /opt/jdk/sun/1.6 + + + `; + const result = ` + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + + + jdk + + 1.6 + Sun + sun_1.6 + + + /opt/jdk/sun/1.6 + + +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + + it('keeps partially-formed jdk toolchains without an id instead of crashing', async () => { + const jdkInfo = { + version: '17', + vendor: 'Eclipse Temurin', + id: 'temurin_17', + jdkHome: '/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64' + }; + + const originalFile = ` + + jdk + + 1.6 + Sun + + + /opt/jdk/sun/1.6 + + + `; + const result = ` + + + jdk + + 17 + Eclipse Temurin + temurin_17 + + + /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.1-12/x64 + + + + jdk + + 1.6 + Sun + + + /opt/jdk/sun/1.6 + + +`; + + fs.mkdirSync(m2Dir, {recursive: true}); + fs.writeFileSync(toolchainsFile, originalFile); + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + + await toolchains.createToolchainsSettings({ + jdkInfo, + settingsDirectory: m2Dir, + overwriteSettings: true + }); + + expect(fs.existsSync(m2Dir)).toBe(true); + expect(fs.existsSync(toolchainsFile)).toBe(true); + expect(fs.readFileSync(toolchainsFile, 'utf-8')).toEqual( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ); + expect( + toolchains.generateToolchainDefinition( + originalFile, + jdkInfo.version, + jdkInfo.vendor, + jdkInfo.id, + jdkInfo.jdkHome + ) + ).toEqual(result); + }, 100000); + it('does not overwrite existing toolchains.xml files', async () => { const jdkInfo = { version: '17', diff --git a/dist/setup/index.js b/dist/setup/index.js index 6d8a1661c..863627d5c 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -81237,6 +81237,12 @@ function generateToolchainDefinition(original, version, vendor, id, jdkHome) { } } ]; + // default root attributes, used when the existing file does not declare its own + let rootAttributes = { + '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', + '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', + '@xsi:schemaLocation': 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd' + }; if (original === null || original === void 0 ? void 0 : original.length) { // convert existing toolchains into TS native objects for better handling // xmlbuilder2 will convert the document into a `{toolchains: { toolchain: [] | {} }}` structure @@ -81244,32 +81250,39 @@ function generateToolchainDefinition(original, version, vendor, id, jdkHome) { const jsObj = (0, xmlbuilder2_1.create)(original) .root() .toObject(); - if (jsObj.toolchains && jsObj.toolchains.toolchain) { - // in case only a single child exists xmlbuilder2 will not create an array and using verbose = true equally doesn't work here - // See https://oozcitak.github.io/xmlbuilder2/serialization.html#js-object-and-map-serializers for details - if (Array.isArray(jsObj.toolchains.toolchain)) { - jsToolchains.push(...jsObj.toolchains.toolchain); - } - else { - jsToolchains.push(jsObj.toolchains.toolchain); + if (jsObj.toolchains) { + // preserve the existing root attributes (xmlns, schemaLocation, …) so we don't + // silently rewrite user-managed metadata or change the effective XML namespace; + // xmlbuilder2 exposes attributes as `@`-prefixed keys on the element object + const existingAttributes = Object.fromEntries(Object.entries(jsObj.toolchains).filter(([key]) => key.startsWith('@'))); + // fall back to the defaults only for attributes the existing file is missing + rootAttributes = Object.assign(Object.assign({}, rootAttributes), existingAttributes); + if (jsObj.toolchains.toolchain) { + // in case only a single child exists xmlbuilder2 will not create an array and using verbose = true equally doesn't work here + // See https://oozcitak.github.io/xmlbuilder2/serialization.html#js-object-and-map-serializers for details + if (Array.isArray(jsObj.toolchains.toolchain)) { + jsToolchains.push(...jsObj.toolchains.toolchain); + } + else { + jsToolchains.push(jsObj.toolchains.toolchain); + } } } // remove potential duplicates based on type & id (which should be a unique combination); // self.findIndex will only return the first occurrence, ensuring duplicates are skipped - jsToolchains = jsToolchains.filter((value, index, self) => - // ensure non-jdk toolchains are kept in the results, we must not touch them because they belong to the user - value.type !== 'jdk' || - index === - self.findIndex(t => t.type === value.type && t.provides.id === value.provides.id)); + jsToolchains = jsToolchains.filter((value, index, self) => { + var _a; + // ensure non-jdk toolchains are kept in the results, we must not touch them because they belong to the user + return value.type !== 'jdk' || + // keep toolchains that lack a usable string id (e.g. partially-formed user files); + // we cannot safely deduplicate them and must not crash while reading them + typeof ((_a = value.provides) === null || _a === void 0 ? void 0 : _a.id) !== 'string' || + index === + self.findIndex(t => { var _a, _b; return t.type === value.type && ((_a = t.provides) === null || _a === void 0 ? void 0 : _a.id) === ((_b = value.provides) === null || _b === void 0 ? void 0 : _b.id); }); + }); } - // TODO: technically bad because we shouldn't re-create the toolchains root node (with possibly different schema values) if it already exists, however, just overriding the toolchain array with xmlbuilder2 is … uh non-trivial return (0, xmlbuilder2_1.create)({ - toolchains: { - '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', - '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', - '@xsi:schemaLocation': 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd', - toolchain: jsToolchains - } + toolchains: Object.assign(Object.assign({}, rootAttributes), { toolchain: jsToolchains }) }).end({ format: 'xml', wellFormed: false, diff --git a/src/toolchains.ts b/src/toolchains.ts index e91a68be2..a94a56302 100644 --- a/src/toolchains.ts +++ b/src/toolchains.ts @@ -96,6 +96,13 @@ export function generateToolchainDefinition( } } ]; + // default root attributes, used when the existing file does not declare its own + let rootAttributes: Record = { + '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', + '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', + '@xsi:schemaLocation': + 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd' + }; if (original?.length) { // convert existing toolchains into TS native objects for better handling // xmlbuilder2 will convert the document into a `{toolchains: { toolchain: [] | {} }}` structure @@ -103,13 +110,24 @@ export function generateToolchainDefinition( const jsObj = xmlCreate(original) .root() .toObject() as unknown as ExtractedToolchains; - if (jsObj.toolchains && jsObj.toolchains.toolchain) { - // in case only a single child exists xmlbuilder2 will not create an array and using verbose = true equally doesn't work here - // See https://oozcitak.github.io/xmlbuilder2/serialization.html#js-object-and-map-serializers for details - if (Array.isArray(jsObj.toolchains.toolchain)) { - jsToolchains.push(...jsObj.toolchains.toolchain); - } else { - jsToolchains.push(jsObj.toolchains.toolchain); + if (jsObj.toolchains) { + // preserve the existing root attributes (xmlns, schemaLocation, …) so we don't + // silently rewrite user-managed metadata or change the effective XML namespace; + // xmlbuilder2 exposes attributes as `@`-prefixed keys on the element object + const existingAttributes = Object.fromEntries( + Object.entries(jsObj.toolchains).filter(([key]) => key.startsWith('@')) + ) as Record; + // fall back to the defaults only for attributes the existing file is missing + rootAttributes = {...rootAttributes, ...existingAttributes}; + + if (jsObj.toolchains.toolchain) { + // in case only a single child exists xmlbuilder2 will not create an array and using verbose = true equally doesn't work here + // See https://oozcitak.github.io/xmlbuilder2/serialization.html#js-object-and-map-serializers for details + if (Array.isArray(jsObj.toolchains.toolchain)) { + jsToolchains.push(...jsObj.toolchains.toolchain); + } else { + jsToolchains.push(jsObj.toolchains.toolchain); + } } } @@ -119,20 +137,19 @@ export function generateToolchainDefinition( (value, index, self) => // ensure non-jdk toolchains are kept in the results, we must not touch them because they belong to the user value.type !== 'jdk' || + // keep toolchains that lack a usable string id (e.g. partially-formed user files); + // we cannot safely deduplicate them and must not crash while reading them + typeof value.provides?.id !== 'string' || index === self.findIndex( - t => t.type === value.type && t.provides.id === value.provides.id + t => t.type === value.type && t.provides?.id === value.provides?.id ) ); } - // TODO: technically bad because we shouldn't re-create the toolchains root node (with possibly different schema values) if it already exists, however, just overriding the toolchain array with xmlbuilder2 is … uh non-trivial return xmlCreate({ toolchains: { - '@xmlns': 'http://maven.apache.org/TOOLCHAINS/1.1.0', - '@xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance', - '@xsi:schemaLocation': - 'http://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd', + ...rootAttributes, toolchain: jsToolchains } }).end({ @@ -181,7 +198,9 @@ async function writeToolchainsFileToDisk( interface ExtractedToolchains { toolchains: { - toolchain: Toolchain[] | Toolchain; + // root attributes such as xmlns / schemaLocation are exposed as `@`-prefixed keys + [attribute: `@${string}`]: string; + toolchain?: Toolchain[] | Toolchain; }; }