diff --git a/__tests__/toolchains.test.ts b/__tests__/toolchains.test.ts index 4aa1c187..38cac18c 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 6d8a1661..863627d5 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 e91a68be..a94a5630 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; }; }