From d713cac0bc3d91526dde9e8ef66406d240428d5f Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Tue, 6 Aug 2024 14:18:37 +0200 Subject: [PATCH 1/5] fix: add missing await to `importGPGKey` function call ## Context We have recently set up a multiple target setup in one of our repos: https://github.com/getsentry/sentry-kotlin-multiplatform/blob/main/.craft.yml and noticed an error during publish that indicates an error within the `importGPGKey` function: https://github.com/getsentry/publish/actions/runs/10250810494/job/28357374501#step:11:166 ## Fix The `importGPGKey` function returns a promise but is not being awaited Side note: shouldn't this be caught by some static code analyzer or linter? --- src/targets/maven.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/targets/maven.ts b/src/targets/maven.ts index ad10707a..0ac5978e 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -93,7 +93,7 @@ export class MavenTarget extends BaseTarget { this.mavenConfig = this.getMavenConfig(); if (process.env.GPG_PRIVATE_KEY) { - importGPGKey(process.env.GPG_PRIVATE_KEY); + await importGPGKey(process.env.GPG_PRIVATE_KEY); } } From cf47fd2d629ba1f8dd31d8211e788942a5ca638f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 6 Aug 2024 14:44:49 +0200 Subject: [PATCH 2/5] move to publish --- src/targets/__tests__/maven.test.ts | 9 ++++++--- src/targets/maven.ts | 25 ++++++++++++++++--------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 540e9dd7..7f4eaaeb 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -260,10 +260,11 @@ describe('Maven target configuration', () => { expect(typeof mvnTarget.config.kmp.appleDistDirRegex).toBe('string'); }); - test('import GPG private key if one is present in the environment', () => { + test('import GPG private key if one is present in the environment', async () => { setTargetSecretsInEnv(); process.env.GPG_PRIVATE_KEY = DEFAULT_OPTION_VALUE; - createMavenTarget(getFullTargetConfig()); + const mvnTarget = createMavenTarget(getFullTargetConfig()); + await mvnTarget.publish('1.0.0', 'r3v1s10n'); expect(importGPGKey).toHaveBeenCalledWith(DEFAULT_OPTION_VALUE); }); }); @@ -371,7 +372,9 @@ describe('upload', () => { .fn() .mockResolvedValueOnce('artifact/download/path'); mvnTarget.isBomFile = jest.fn().mockResolvedValueOnce(false); - mvnTarget.getPomFileInDist = jest.fn().mockResolvedValueOnce('pom-default.xml'); + mvnTarget.getPomFileInDist = jest + .fn() + .mockResolvedValueOnce('pom-default.xml'); await mvnTarget.upload('r3v1s10n'); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 0ac5978e..8809b478 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -91,10 +91,6 @@ export class MavenTarget extends BaseTarget { ) { super(config, artifactProvider); this.mavenConfig = this.getMavenConfig(); - - if (process.env.GPG_PRIVATE_KEY) { - await importGPGKey(process.env.GPG_PRIVATE_KEY); - } } /** @@ -236,6 +232,9 @@ export class MavenTarget extends BaseTarget { * @param revision Git commit SHA to be published. */ public async publish(_version: string, revison: string): Promise { + if (process.env.GPG_PRIVATE_KEY) { + await importGPGKey(process.env.GPG_PRIVATE_KEY); + } await this.upload(revison); await this.closeAndReleaseRepository(); } @@ -297,7 +296,9 @@ export class MavenTarget extends BaseTarget { this.logger.debug('Found POM: ', pomFile); await this.uploadPomDistribution(distDir); } else { - this.logger.warn(`No BOM/POM file found in: ${distDir}, skipping directory`); + this.logger.warn( + `No BOM/POM file found in: ${distDir}, skipping directory` + ); } } @@ -346,7 +347,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; @@ -520,9 +521,15 @@ export class MavenTarget extends BaseTarget { * this function renames module.json to dist.module, * making it fit for mvn publishing. */ - public async fixModuleFileName(distDir: string, moduleFile: string): Promise { + public async fixModuleFileName( + distDir: string, + moduleFile: string + ): Promise { const fallbackFile = join(distDir, 'module.json'); - if (!await this.fileExists(moduleFile) && await this.fileExists(fallbackFile)) { + if ( + !(await this.fileExists(moduleFile)) && + (await this.fileExists(fallbackFile)) + ) { await fsPromises.rename(fallbackFile, moduleFile); } } @@ -572,7 +579,7 @@ export class MavenTarget extends BaseTarget { javadocFile: join(distDir, `${moduleName}-javadoc.jar`), sourcesFile: join(distDir, `${moduleName}-sources.jar`), pomFile: join(distDir, 'pom-default.xml'), - moduleFile: join(distDir, `${moduleName}.module`) + moduleFile: join(distDir, `${moduleName}.module`), }; } From 8572d009a1d0bd9139f286976f8eb8586accb49f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 6 Aug 2024 14:49:31 +0200 Subject: [PATCH 3/5] fix test --- src/targets/__tests__/maven.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 7f4eaaeb..c3bfc4fb 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -263,7 +263,12 @@ describe('Maven target configuration', () => { test('import GPG private key if one is present in the environment', async () => { setTargetSecretsInEnv(); process.env.GPG_PRIVATE_KEY = DEFAULT_OPTION_VALUE; + const callOrder: string[] = []; const mvnTarget = createMavenTarget(getFullTargetConfig()); + mvnTarget.upload = jest.fn(async () => void callOrder.push('upload')); + mvnTarget.closeAndReleaseRepository = jest.fn( + async () => void callOrder.push('closeAndReleaseRepository') + ); await mvnTarget.publish('1.0.0', 'r3v1s10n'); expect(importGPGKey).toHaveBeenCalledWith(DEFAULT_OPTION_VALUE); }); From 5c7f6f919c9a2cb4fa9cc31b64a9fe1ea211ed6d Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 7 Aug 2024 15:14:11 +0200 Subject: [PATCH 4/5] revert some formatting --- src/targets/__tests__/maven.test.ts | 5 +---- src/targets/maven.ts | 18 +++++------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index c3bfc4fb..61fd345a 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -377,10 +377,7 @@ describe('upload', () => { .fn() .mockResolvedValueOnce('artifact/download/path'); mvnTarget.isBomFile = jest.fn().mockResolvedValueOnce(false); - mvnTarget.getPomFileInDist = jest - .fn() - .mockResolvedValueOnce('pom-default.xml'); - + mvnTarget.getPomFileInDist = jest.fn().mockResolvedValueOnce('pom-default.xml'); await mvnTarget.upload('r3v1s10n'); expect(retrySpawnProcess).toHaveBeenCalledTimes(1); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 8809b478..e0978fcc 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -296,9 +296,7 @@ export class MavenTarget extends BaseTarget { this.logger.debug('Found POM: ', pomFile); await this.uploadPomDistribution(distDir); } else { - this.logger.warn( - `No BOM/POM file found in: ${distDir}, skipping directory` - ); + this.logger.warn(`No BOM/POM file found in: ${distDir}, skipping directory`); } } @@ -347,7 +345,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; @@ -521,15 +519,9 @@ export class MavenTarget extends BaseTarget { * this function renames module.json to dist.module, * making it fit for mvn publishing. */ - public async fixModuleFileName( - distDir: string, - moduleFile: string - ): Promise { + public async fixModuleFileName(distDir: string, moduleFile: string): Promise { const fallbackFile = join(distDir, 'module.json'); - if ( - !(await this.fileExists(moduleFile)) && - (await this.fileExists(fallbackFile)) - ) { + if (!await this.fileExists(moduleFile) && await this.fileExists(fallbackFile)) { await fsPromises.rename(fallbackFile, moduleFile); } } @@ -579,7 +571,7 @@ export class MavenTarget extends BaseTarget { javadocFile: join(distDir, `${moduleName}-javadoc.jar`), sourcesFile: join(distDir, `${moduleName}-sources.jar`), pomFile: join(distDir, 'pom-default.xml'), - moduleFile: join(distDir, `${moduleName}.module`), + moduleFile: join(distDir, `${moduleName}.module`) }; } From 88351e6e7a538e50f906b84a24528069cb033475 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 7 Aug 2024 15:14:50 +0200 Subject: [PATCH 5/5] add missing newline --- src/targets/__tests__/maven.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 61fd345a..28e03b98 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -378,6 +378,7 @@ describe('upload', () => { .mockResolvedValueOnce('artifact/download/path'); mvnTarget.isBomFile = jest.fn().mockResolvedValueOnce(false); mvnTarget.getPomFileInDist = jest.fn().mockResolvedValueOnce('pom-default.xml'); + await mvnTarget.upload('r3v1s10n'); expect(retrySpawnProcess).toHaveBeenCalledTimes(1);