From f049c79af2ed53029d95246ed6a4cc97037de83e Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Wed, 21 Jul 2021 22:04:26 +0200 Subject: [PATCH 1/6] Initial code to support BOM files --- src/targets/maven.ts | 92 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 87 insertions(+), 5 deletions(-) diff --git a/src/targets/maven.ts b/src/targets/maven.ts index fcc6e8098..8ee14e50d 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -5,12 +5,11 @@ import { } from '../artifact_providers/base'; import { BaseTarget } from './base'; import { homedir } from 'os'; -import { basename, join, parse } from 'path'; -import { promises as fsPromises } from 'fs'; +import { basename, extname, join, parse } from 'path'; +import { constants as fsConstants, promises as fsPromises } from 'fs'; import { checkExecutableIsPresent, extractZipArchive } from '../utils/system'; import { retrySpawnProcess } from '../utils/async'; import { withTempDir } from '../utils/files'; -import { checkEnvForPrerequisite } from '../utils/env'; import { ConfigurationError } from '../utils/errors'; import { stringToRegexp } from '../utils/filters'; @@ -21,6 +20,9 @@ const GRADLE_PROPERTIES_FILENAME = 'gradle.properties'; * https://docs.gradle.org/current/userguide/build_environment.html#sec:gradle_environment_variables */ const DEFAULT_GRADLE_USER_HOME = join(homedir(), '.gradle'); +const POM_DEFAULT_FILENAME = 'pom-default.xml'; +const POM_FILE_EXTNAME = '.xml'; // Must include the leading `.` +const BOM_FILE_KEY_REGEXP = stringToRegexp('/pom/'); export const targetSecrets = ['OSSRH_USERNAME', 'OSSRH_PASSWORD'] as const; type SecretsType = typeof targetSecrets[number]; @@ -258,12 +260,92 @@ export class MavenTarget extends BaseTarget { * @param distDir directory of the distribution. */ private async uploadDistribution(distDir: string): Promise { + const bomFile = await this.getBomFileFomDist(distDir); + if (bomFile) { + this.logger.debug('Found BOM: ', bomFile); + await this.uploadBomDistribution(bomFile); + } else { + this.logger.debug('Did not find a BOM.'); + await this.uploadPomDistribution(distDir); + } + } + + private async getBomFileFomDist( + distDir: string + ): Promise { + const pomFilepath = join(distDir, POM_DEFAULT_FILENAME); + if (await this.isBomFile(pomFilepath)) { + return pomFilepath; + } + + // There may be several files in the ZIP-ed artifact with the same name, + // where the BOM may be one of them (there may not be a BOM). Files may be + // renamed when extracting the ZIP, so the default name (`pom-default.xml`) + // may not match. It's assumed that any renaming keeps the same extension, + // so all files with the same extension are checked to identify the BOM. + const filesInDir = await fsPromises.readdir(distDir); + const potentialPoms = filesInDir + .filter(f => extname(f) === POM_FILE_EXTNAME) + .filter(f => f !== POM_DEFAULT_FILENAME) + .map(f => join(distDir, f)); + + for (const f of potentialPoms) { + if (await this.isBomFile(f)) { + return f; + } + } + + return undefined; + } + + /** + * Returns whether the given POM is a BOM. + * + * A BOM file is a POM file with the following key: + * `pom`, usually named as `pom-default.xml`. + * + * @param pomFilepath path to the POM. + * @returns true if the POM is a BOM. + */ + private async isBomFile(pomFilepath: string): Promise { + try { + await fsPromises.access(pomFilepath, fsConstants.R_OK); + const fileContents = await fsPromises.readFile(pomFilepath, { + encoding: 'utf8', + }); + const matchesRequiredKey = BOM_FILE_KEY_REGEXP.test(fileContents); + if (matchesRequiredKey) { + return true; + } + return false; + } catch (error) { + this.logger.warn( + `Error checking whether it's a BOM file: ${pomFilepath}\n`, + error + ); + return false; + } + } + + private async uploadBomDistribution(bomFile: string): Promise { + await retrySpawnProcess(this.mavenConfig.mavenCliPath, [ + 'gpg:sign-and-deploy-file', + `-Dfile=${bomFile}`, + `-DpomFile=${bomFile}`, + `-DrepositoryId=${this.mavenConfig.mavenRepoId}`, + `-Durl=${this.mavenConfig.mavenRepoUrl}`, + '--settings', + this.mavenConfig.mavenSettingsPath, + ]); + } + + private async uploadPomDistribution(distDir: string): Promise { const { targetFile, javadocFile, sourcesFile, pomFile, - } = this.getFilesForMavenCli(distDir); + } = this.getFilesForMavenPomDist(distDir); // Maven central is very flaky, so retrying with an exponential delay in // in case it fails. @@ -288,7 +370,7 @@ export class MavenTarget extends BaseTarget { * @param distDir directory of the distribution. * @returns record of required files. */ - private getFilesForMavenCli(distDir: string): Record { + private getFilesForMavenPomDist(distDir: string): Record { const moduleName = parse(distDir).base; return { targetFile: join(distDir, this.getTargetFilename(distDir)), From 70ea6f006ee0bbf0ddf6384cba6744d7295901e0 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 22 Jul 2021 12:42:21 +0200 Subject: [PATCH 2/6] Update tests --- src/targets/__tests__/maven.test.ts | 65 ++++++++++++++++++++++++++--- src/targets/maven.ts | 5 ++- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 497115ff4..76092318d 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -1,7 +1,12 @@ import { homedir } from 'os'; import { join } from 'path'; import { NoneArtifactProvider } from '../../artifact_providers/none'; -import { MavenTarget, targetOptions, targetSecrets } from '../maven'; +import { + MavenTarget, + POM_DEFAULT_FILENAME, + targetOptions, + targetSecrets, +} from '../maven'; import { retrySpawnProcess } from '../../utils/async'; import { withTempDir } from '../../utils/files'; @@ -11,6 +16,11 @@ jest.mock('fs', () => ({ ...jest.requireActual('fs'), promises: { writeFile: jest.fn(() => Promise.resolve()), + readFile: jest.fn((file: string) => file), + readdir: async () => Promise.resolve([]), // empty dir + access: jest.fn().mockImplementation(() => { + // do nothing + }), }, })); @@ -109,10 +119,8 @@ describe('publish', () => { async () => void callOrder.push('closeAndRelease') ); - const version = '1.0.0'; const revision = 'r3v1s10n'; - - await mvnTarget.publish(version, revision); + await mvnTarget.publish('1.0.0', revision); expect(gradlePropsMock).toHaveBeenCalledTimes(1); expect(uploadMock).toHaveBeenCalledTimes(1); expect(uploadMock).toHaveBeenLastCalledWith(revision); @@ -127,7 +135,7 @@ describe('publish', () => { ]); }); - test('upload', async () => { + test('upload POM', async () => { // simple mock to always use the same temporary directory, // instead of creating a new one (withTempDir as jest.MockedFunction).mockImplementation( @@ -143,9 +151,11 @@ describe('publish', () => { mvnTarget.artifactProvider.downloadArtifact = jest .fn() .mockResolvedValueOnce('artifact/download/path'); + mvnTarget.isBomFile = jest.fn().mockResolvedValueOnce(undefined); await mvnTarget.upload('r3v1s10n'); - expect(retrySpawnProcess).toBeCalledTimes(1); + + expect(retrySpawnProcess).toHaveBeenCalledTimes(1); const callArgs = (retrySpawnProcess as jest.MockedFunction< typeof retrySpawnProcess >).mock.calls[0]; @@ -172,6 +182,49 @@ describe('publish', () => { expect(cmdArgs[8]).toBe('--settings'); expect(cmdArgs[9]).toBe(DEFAULT_OPTION_VALUE); }); + + test('upload BOM', async () => { + // simple mock to always use the same temporary directory, + // instead of creating a new one + (withTempDir as jest.MockedFunction).mockImplementation( + async cb => { + return await cb(tmpDirName); + } + ); + + const mvnTarget = createMavenTarget(); + mvnTarget.getArtifactsForRevision = jest + .fn() + .mockResolvedValueOnce([{ filename: 'mockArtifact.zip' }]); + mvnTarget.artifactProvider.downloadArtifact = jest + .fn() + .mockResolvedValueOnce('artifact/download/path'); + mvnTarget.isBomFile = jest.fn().mockResolvedValueOnce('path/to/bomfile'); + + await mvnTarget.upload('r3v1s10n'); + + expect(retrySpawnProcess).toHaveBeenCalledTimes(1); + const callArgs = (retrySpawnProcess as jest.MockedFunction< + typeof retrySpawnProcess + >).mock.calls[0]; + + expect(callArgs).toHaveLength(2); + expect(callArgs[0]).toEqual(DEFAULT_OPTION_VALUE); + + const cmdArgs = callArgs[1] as string[]; + expect(cmdArgs).toHaveLength(7); + expect(cmdArgs[0]).toBe('gpg:sign-and-deploy-file'); + expect(cmdArgs[1]).toMatch( + new RegExp(`-Dfile=${tmpDirName}.+${POM_DEFAULT_FILENAME}`) + ); + expect(cmdArgs[2]).toMatch( + new RegExp(`-DpomFile=${tmpDirName}.*${POM_DEFAULT_FILENAME}`) + ); + expect(cmdArgs[3]).toBe(`-DrepositoryId=${DEFAULT_OPTION_VALUE}`); + expect(cmdArgs[4]).toBe(`-Durl=${DEFAULT_OPTION_VALUE}`); + expect(cmdArgs[5]).toBe('--settings'); + expect(cmdArgs[6]).toBe(DEFAULT_OPTION_VALUE); + }); }); describe('get gradle home directory', () => { diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 8ee14e50d..a2ba56d60 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -12,6 +12,7 @@ import { retrySpawnProcess } from '../utils/async'; import { withTempDir } from '../utils/files'; import { ConfigurationError } from '../utils/errors'; import { stringToRegexp } from '../utils/filters'; +import { checkEnvForPrerequisite } from '../utils/env'; const GRADLE_PROPERTIES_FILENAME = 'gradle.properties'; @@ -20,7 +21,7 @@ const GRADLE_PROPERTIES_FILENAME = 'gradle.properties'; * https://docs.gradle.org/current/userguide/build_environment.html#sec:gradle_environment_variables */ const DEFAULT_GRADLE_USER_HOME = join(homedir(), '.gradle'); -const POM_DEFAULT_FILENAME = 'pom-default.xml'; +export const POM_DEFAULT_FILENAME = 'pom-default.xml'; const POM_FILE_EXTNAME = '.xml'; // Must include the leading `.` const BOM_FILE_KEY_REGEXP = stringToRegexp('/pom/'); @@ -307,7 +308,7 @@ export class MavenTarget extends BaseTarget { * @param pomFilepath path to the POM. * @returns true if the POM is a BOM. */ - private async isBomFile(pomFilepath: string): Promise { + public async isBomFile(pomFilepath: string): Promise { try { await fsPromises.access(pomFilepath, fsConstants.R_OK); const fileContents = await fsPromises.readFile(pomFilepath, { From 2a431d47bf524069568a44f2704330efd3fa8339 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 22 Jul 2021 17:05:32 +0200 Subject: [PATCH 3/6] Address feedback --- src/targets/maven.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/targets/maven.ts b/src/targets/maven.ts index a2ba56d60..495bee09a 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -22,7 +22,7 @@ const GRADLE_PROPERTIES_FILENAME = 'gradle.properties'; */ const DEFAULT_GRADLE_USER_HOME = join(homedir(), '.gradle'); export const POM_DEFAULT_FILENAME = 'pom-default.xml'; -const POM_FILE_EXTNAME = '.xml'; // Must include the leading `.` +const POM_FILE_EXT = '.xml'; // Must include the leading `.` const BOM_FILE_KEY_REGEXP = stringToRegexp('/pom/'); export const targetSecrets = ['OSSRH_USERNAME', 'OSSRH_PASSWORD'] as const; @@ -271,6 +271,10 @@ export class MavenTarget extends BaseTarget { } } + /** + * Returns the path to the BOM file in the given distribution directory, and + * `undefined` if there isn't any. + */ private async getBomFileFomDist( distDir: string ): Promise { @@ -286,7 +290,7 @@ export class MavenTarget extends BaseTarget { // so all files with the same extension are checked to identify the BOM. const filesInDir = await fsPromises.readdir(distDir); const potentialPoms = filesInDir - .filter(f => extname(f) === POM_FILE_EXTNAME) + .filter(f => extname(f) === POM_FILE_EXT) .filter(f => f !== POM_DEFAULT_FILENAME) .map(f => join(distDir, f)); @@ -314,14 +318,11 @@ export class MavenTarget extends BaseTarget { const fileContents = await fsPromises.readFile(pomFilepath, { encoding: 'utf8', }); - const matchesRequiredKey = BOM_FILE_KEY_REGEXP.test(fileContents); - if (matchesRequiredKey) { - return true; - } - return false; + return BOM_FILE_KEY_REGEXP.test(fileContents); } catch (error) { this.logger.warn( - `Error checking whether it's a BOM file: ${pomFilepath}\n`, + `Could not determine if path corresponds to a BOM file: ${pomFilepath}\n` + + `Error:\n`, error ); return false; From 1389773342cb8f95c4c6286c579862049448b530 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 22 Jul 2021 17:06:37 +0200 Subject: [PATCH 4/6] Rename method --- src/targets/maven.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 495bee09a..79f093f3f 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -261,7 +261,7 @@ export class MavenTarget extends BaseTarget { * @param distDir directory of the distribution. */ private async uploadDistribution(distDir: string): Promise { - const bomFile = await this.getBomFileFomDist(distDir); + const bomFile = await this.getBomFileInDist(distDir); if (bomFile) { this.logger.debug('Found BOM: ', bomFile); await this.uploadBomDistribution(bomFile); @@ -275,9 +275,7 @@ export class MavenTarget extends BaseTarget { * Returns the path to the BOM file in the given distribution directory, and * `undefined` if there isn't any. */ - private async getBomFileFomDist( - distDir: string - ): Promise { + private async getBomFileInDist(distDir: string): Promise { const pomFilepath = join(distDir, POM_DEFAULT_FILENAME); if (await this.isBomFile(pomFilepath)) { return pomFilepath; From cdc23030abc0cec18ecca429820588e4140747df Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 22 Jul 2021 18:02:53 +0200 Subject: [PATCH 5/6] Address more feedback --- src/targets/__tests__/maven.test.ts | 2 +- src/targets/maven.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 76092318d..706aeae4e 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -151,7 +151,7 @@ describe('publish', () => { mvnTarget.artifactProvider.downloadArtifact = jest .fn() .mockResolvedValueOnce('artifact/download/path'); - mvnTarget.isBomFile = jest.fn().mockResolvedValueOnce(undefined); + mvnTarget.isBomFile = jest.fn().mockResolvedValueOnce(false); await mvnTarget.upload('r3v1s10n'); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 79f093f3f..4d8213084 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -286,6 +286,10 @@ export class MavenTarget extends BaseTarget { // renamed when extracting the ZIP, so the default name (`pom-default.xml`) // may not match. It's assumed that any renaming keeps the same extension, // so all files with the same extension are checked to identify the BOM. + // TODO: make sure all scenarios are considered and tested. + // Each file system may handle this case differently, and attended vs + // unattended mode also have different behaviours. It's not desired to get + // the BOM renamed in such a way that isn't handled by the target. const filesInDir = await fsPromises.readdir(distDir); const potentialPoms = filesInDir .filter(f => extname(f) === POM_FILE_EXT) @@ -320,7 +324,7 @@ export class MavenTarget extends BaseTarget { } catch (error) { this.logger.warn( `Could not determine if path corresponds to a BOM file: ${pomFilepath}\n` + - `Error:\n`, + 'Error:\n', error ); return false; From f10bd39826938689707796b329767ef452b26c37 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 22 Jul 2021 18:15:10 +0200 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dbe224b6..3b1617b22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - feat(maven): Add maven target to deploy to Maven Central (#258) - feat(symbol-collector): Add symbol-collector target (#266) +- ref(maven): Support BOM files in `maven` target (#270) ## 0.24.4