From 74f9fa840c246034893b566503faa34cf5364479 Mon Sep 17 00:00:00 2001 From: daz Date: Thu, 23 Jan 2025 12:07:37 -0700 Subject: [PATCH 1/5] Make it easier to run NPM tests locally --- build | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build b/build index c99303a..657f586 100755 --- a/build +++ b/build @@ -36,6 +36,10 @@ case "$1" in npm clean-install npm run build ;; + test) + shift + npm test -- $@ + ;; *) npm run build ;; From 24aa9155d9eb9b6b82eb0544eb26a135d493aca5 Mon Sep 17 00:00:00 2001 From: daz Date: Thu, 23 Jan 2025 12:08:44 -0700 Subject: [PATCH 2/5] Fix minor typo in log message --- sources/src/execution/provision.ts | 2 +- sources/test/jest/cache-debug.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sources/src/execution/provision.ts b/sources/src/execution/provision.ts index 8bc5711..a3404fb 100644 --- a/sources/src/execution/provision.ts +++ b/sources/src/execution/provision.ts @@ -88,7 +88,7 @@ async function gradleReleaseNightly(): Promise { async function gradleRelease(version: string): Promise { const versionInfo = await findGradleVersionDeclaration(version) if (!versionInfo) { - throw new Error(`Gradle version ${version} does not exists`) + throw new Error(`Gradle version ${version} does not exist`) } return versionInfo } diff --git a/sources/test/jest/cache-debug.test.ts b/sources/test/jest/cache-debug.test.ts index ee880b0..4b8a771 100644 --- a/sources/test/jest/cache-debug.test.ts +++ b/sources/test/jest/cache-debug.test.ts @@ -10,7 +10,7 @@ fs.rmSync(testTmp, {recursive: true, force: true}) describe("--info and --stacktrace", () => { describe("will be created", () => { - it("when gradle.properties does not exists", async () => { + it("when gradle.properties does not exist", async () => { const emptyGradleHome = `${testTmp}/empty-gradle-home` fs.mkdirSync(emptyGradleHome, {recursive: true}) From ec4681f7f5e169fab045aa8f67bf2b5dd0e7bc07 Mon Sep 17 00:00:00 2001 From: daz Date: Thu, 23 Jan 2025 12:09:21 -0700 Subject: [PATCH 3/5] Test provision of rc and milestone versions --- .github/workflows/integ-test-provision-gradle-versions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integ-test-provision-gradle-versions.yml b/.github/workflows/integ-test-provision-gradle-versions.yml index add8416..493b78d 100644 --- a/.github/workflows/integ-test-provision-gradle-versions.yml +++ b/.github/workflows/integ-test-provision-gradle-versions.yml @@ -78,7 +78,7 @@ jobs: strategy: fail-fast: false matrix: - gradle: ["8.12", 8.9, 8.1, 7.6.4, 6.9.4, 5.6.4, 4.10.3, 3.5.1] + gradle: ["8.13-milestone-2", "8.12", "8.12-rc-1", 8.9, 8.1, 7.6.4, 6.9.4, 5.6.4, 4.10.3, 3.5.1] os: ${{fromJSON(inputs.runner-os)}} include: - java-version: 11 From edf9e3c8c79b44bb63a4b39a73170065e73aa555 Mon Sep 17 00:00:00 2001 From: daz Date: Thu, 23 Jan 2025 14:07:12 -0700 Subject: [PATCH 4/5] Improve comparison of Gradle versions --- sources/src/execution/gradle.ts | 68 +++++++++++--- sources/test/jest/gradle-version.test.ts | 115 ++++++++++++++++------- 2 files changed, 139 insertions(+), 44 deletions(-) diff --git a/sources/src/execution/gradle.ts b/sources/src/execution/gradle.ts index f8237c6..86f72db 100644 --- a/sources/src/execution/gradle.ts +++ b/sources/src/execution/gradle.ts @@ -35,18 +35,45 @@ async function executeGradleBuild(executable: string | undefined, root: string, } export function versionIsAtLeast(actualVersion: string, requiredVersion: string): boolean { - const splitVersion = actualVersion.split('-') - const coreVersion = splitVersion[0] - const prerelease = splitVersion.length > 1 - - const actualSemver = semver.coerce(coreVersion)! - const comparisonSemver = semver.coerce(requiredVersion)! - - if (prerelease) { - return semver.gt(actualSemver, comparisonSemver) - } else { - return semver.gte(actualSemver, comparisonSemver) + if (actualVersion === requiredVersion) { + return true } + + const actual = new GradleVersion(actualVersion) + const required = new GradleVersion(requiredVersion) + + const actualSemver = semver.coerce(actual.versionPart)! + const comparisonSemver = semver.coerce(required.versionPart)! + + if (semver.gt(actualSemver, comparisonSemver)) { + return true // Actual version is greater than comparison. So it's at least as new. + } + if (semver.lt(actualSemver, comparisonSemver)) { + return false // Actual version is less than comparison. So it's not as new. + } + + // Actual and required version numbers are equal, so compare the other parts + + if (actual.snapshotPart || required.snapshotPart) { + if (actual.snapshotPart && !required.snapshotPart && !required.stagePart) { + return false // Actual has a snapshot, but required is a plain version. Required is newer. + } + if (required.snapshotPart && !actual.snapshotPart && !actual.stagePart) { + return true // Required has a snapshot, but actual is a plain version. Actual is newer. + } + + return false // Cannot compare case where both versions have a snapshot or stage + } + + if (actual.stagePart) { + if (required.stagePart) { + return actual.stagePart >= required.stagePart // Compare stages for newer + } + + return false // Actual has a stage, but required does not. So required is always newer. + } + + return true // Actual has no stage part or snapshot part, so it cannot be older than required. } export async function findGradleVersionOnPath(): Promise { @@ -72,3 +99,22 @@ class GradleExecutable { readonly executable: string ) {} } + +class GradleVersion { + static PATTERN = /((\d+)(\.\d+)+)(-([a-z]+)-(\w+))?(-(SNAPSHOT|\d{14}([-+]\d{4})?))?/ + + versionPart: string + stagePart: string + snapshotPart: string + + constructor(readonly version: string) { + const matcher = GradleVersion.PATTERN.exec(version) + if (!matcher) { + throw new Error(`'${version}' is not a valid Gradle version string (examples: '1.0', '1.0-rc-1')`) + } + + this.versionPart = matcher[1] + this.stagePart = matcher[4] + this.snapshotPart = matcher[7] + } +} diff --git a/sources/test/jest/gradle-version.test.ts b/sources/test/jest/gradle-version.test.ts index 9bcf520..16f664c 100644 --- a/sources/test/jest/gradle-version.test.ts +++ b/sources/test/jest/gradle-version.test.ts @@ -3,43 +3,92 @@ import {describe, expect, it} from '@jest/globals' import {versionIsAtLeast, parseGradleVersionFromOutput} from '../../src/execution/gradle' describe('gradle', () => { - describe('can compare version with', () => { - it('same version', async () => { - expect(versionIsAtLeast('6.7.1', '6.7.1')).toBe(true) - expect(versionIsAtLeast('7.0', '7.0')).toBe(true) - expect(versionIsAtLeast('7.0', '7.0.0')).toBe(true) - }) - it('newer version', async () => { - expect(versionIsAtLeast('6.7.1', '6.7.2')).toBe(false) - expect(versionIsAtLeast('7.0', '8.0')).toBe(false) - expect(versionIsAtLeast('7.0', '7.0.1')).toBe(false) - }) - it('older version', async () => { - expect(versionIsAtLeast('6.7.2', '6.7.1')).toBe(true) - expect(versionIsAtLeast('8.0', '7.0')).toBe(true) - expect(versionIsAtLeast('7.0.1', '7.0')).toBe(true) - }) - it('rc version', async () => { - expect(versionIsAtLeast('8.0.2-rc-1', '8.0.1')).toBe(true) - expect(versionIsAtLeast('8.0.2-rc-1', '8.0.2')).toBe(false) - expect(versionIsAtLeast('8.1-rc-1', '8.0')).toBe(true) - expect(versionIsAtLeast('8.0-rc-1', '8.0')).toBe(false) - }) - it('snapshot version', async () => { - expect(versionIsAtLeast('8.11-20240829002031+0000', '8.10')).toBe(true) - expect(versionIsAtLeast('8.11-20240829002031+0000', '8.10.1')).toBe(true) - expect(versionIsAtLeast('8.11-20240829002031+0000', '8.11')).toBe(false) + describe('can compare versions that are', () => { + function versionsAreOrdered(versions: string[]): void { + for (let i = 0; i < versions.length; i++) { + // Compare with all other versions + for (let j = 0; j < versions.length; j++) { + if (i >= j) { + it(`${versions[i]} is at least ${versions[j]}`, () => { + expect(versionIsAtLeast(versions[i], versions[j])).toBe(true) + }) + } else { + it(`${versions[i]} is NOT at least ${versions[j]}`, () => { + expect(versionIsAtLeast(versions[i], versions[j])).toBe(false) + }) + } + } + } + } - expect(versionIsAtLeast('8.10.2-20240828012138+0000', '8.10')).toBe(true) - expect(versionIsAtLeast('8.10.2-20240828012138+0000', '8.10.1')).toBe(true) - expect(versionIsAtLeast('8.10.2-20240828012138+0000', '8.10.2')).toBe(false) - expect(versionIsAtLeast('8.10.2-20240828012138+0000', '8.11')).toBe(false) + function versionsAreNotOrdered(versions: string[]): void { + for (let i = 0; i < versions.length; i++) { + // Compare with all other versions + for (let j = 0; j < versions.length; j++) { + if (i !== j) { + it(`${versions[i]} is NOT at least ${versions[j]}`, () => { + expect(versionIsAtLeast(versions[i], versions[j])).toBe(false) + }) + } + } + } + } - expect(versionIsAtLeast('9.1-branch-provider_api_migration_public_api_changes-20240826121451+0000', '9.0')).toBe(true) - expect(versionIsAtLeast('9.1-branch-provider_api_migration_public_api_changes-20240826121451+0000', '9.0.1')).toBe(true) - expect(versionIsAtLeast('9.1-branch-provider_api_migration_public_api_changes-20240826121451+0000', '9.1')).toBe(false) + function versionsAreEqual(versions: string[]): void { + for (let i = 0; i < versions.length; i++) { + // Compare with all other versions + for (let j = 0; j < versions.length; j++) { + it(`${versions[i]} is at least ${versions[j]}`, () => { + expect(versionIsAtLeast(versions[i], versions[j])).toBe(true) + }) + } + } + } + + describe('simple versions', () => { + versionsAreOrdered(['6.0', '6.7', '6.7.1', '6.7.2', '7.0', '7.0.1', '7.1', '8.0', '8.12.1']) + + versionsAreEqual(['7.0', '7.0.0']) + versionsAreEqual(['7.1', '7.1.0']) + }) + + describe('rc versions', () => { + versionsAreOrdered([ + '8.10', '8.11-rc-1', '8.11-rc-2', '8.11', '8.11.1-rc-1', '8.11.1' + ]) + }) + + describe('milestone versions', () => { + versionsAreOrdered([ + '8.12.1', '8.12.2-milestone-1', '8.12.2', '8.13-milestone-1', '8.13-milestone-2', '8.13' + ]) + versionsAreOrdered([ + '8.12.1', '8.12.2-milestone-1', '8.12.2-milestone-2', '8.12.2-rc-1', '8.12.2' + ]) + }) + + describe('preview versions', () => { + versionsAreOrdered([ + '8.12.1', '8.12.2-preview-1', '8.12.2', '8.13-preview-1', '8.13-preview-2', '8.13' + ]) + versionsAreOrdered([ + '8.12.1', '8.12.2-milestone-1', '8.12.2-preview-1', '8.12.2-rc-1', '8.12.2' + ]) + }) + + describe('snapshot versions', () => { + versionsAreOrdered([ + '8.10.1', '8.10.2-20240828012138+0000', '8.10.2', '8.11-20240829002031+0000', '8.11' + ]) + versionsAreOrdered([ + '9.0', '9.1-branch-provider_api_migration_public_api_changes-20240826121451+0000', '9.1' + ]) + versionsAreNotOrdered([ + '8.10.2-20240828012138+0000', '8.10.2-20240828010000+1000', '8.10.2-milestone-1' + ]) }) }) + describe('can parse version from output', () => { it('major version', async () => { const output = ` From c6e631b4a7d38c5a3204ba460eec6706f5ea8aaf Mon Sep 17 00:00:00 2001 From: daz Date: Thu, 23 Jan 2025 17:37:15 -0700 Subject: [PATCH 5/5] Attempt to provision best gradle version for cache-cleanup --- sources/src/build-results.ts | 13 +++++++++ sources/src/caching/cache-cleaner.ts | 35 ++++++++++++++++++++----- sources/src/caching/caches.ts | 6 ++--- sources/test/jest/cache-cleanup.test.ts | 4 +-- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/sources/src/build-results.ts b/sources/src/build-results.ts index 82934cf..9969867 100644 --- a/sources/src/build-results.ts +++ b/sources/src/build-results.ts @@ -1,5 +1,6 @@ import * as fs from 'fs' import * as path from 'path' +import {versionIsAtLeast} from './execution/gradle' export interface BuildResult { get rootProjectName(): string @@ -32,6 +33,18 @@ export class BuildResults { const allHomes = this.results.map(buildResult => buildResult.gradleHomeDir) return Array.from(new Set(allHomes)) } + + highestGradleVersion(): string | null { + if (this.results.length === 0) { + return null + } + return this.results + .map(result => result.gradleVersion) + .reduce((maxVersion: string, currentVersion: string) => { + if (!maxVersion) return currentVersion + return versionIsAtLeast(currentVersion, maxVersion) ? currentVersion : maxVersion + }) + } } export function loadBuildResults(): BuildResults { diff --git a/sources/src/caching/cache-cleaner.ts b/sources/src/caching/cache-cleaner.ts index e050c3b..91ed908 100644 --- a/sources/src/caching/cache-cleaner.ts +++ b/sources/src/caching/cache-cleaner.ts @@ -4,6 +4,8 @@ import * as exec from '@actions/exec' import fs from 'fs' import path from 'path' import * as provisioner from '../execution/provision' +import {BuildResults} from '../build-results' +import {versionIsAtLeast} from '../execution/gradle' export class CacheCleaner { private readonly gradleUserHome: string @@ -21,13 +23,37 @@ export class CacheCleaner { return timestamp } - async forceCleanup(): Promise { + async forceCleanup(buildResults: BuildResults): Promise { + const executable = await this.gradleExecutableForCleanup(buildResults) const cleanTimestamp = core.getState('clean-timestamp') - await this.forceCleanupFilesOlderThan(cleanTimestamp) + await this.forceCleanupFilesOlderThan(cleanTimestamp, executable) + } + + /** + * Attempt to use the newest Gradle version that was used to run a build, at least 8.11. + * + * This will avoid the need to provision a Gradle version for the cleanup when not necessary. + */ + private async gradleExecutableForCleanup(buildResults: BuildResults): Promise { + const preferredVersion = buildResults.highestGradleVersion() + if (preferredVersion && versionIsAtLeast(preferredVersion, '8.11')) { + try { + return await provisioner.provisionGradleAtLeast(preferredVersion) + } catch (e) { + // Ignore the case where the preferred version cannot be located in https://services.gradle.org/versions/all. + // This can happen for snapshot Gradle versions. + core.info( + `Failed to provision Gradle ${preferredVersion} for cache cleanup. Falling back to default version.` + ) + } + } + + // Fallback to the minimum version required for cache-cleanup + return await provisioner.provisionGradleAtLeast('8.11') } // Visible for testing - async forceCleanupFilesOlderThan(cleanTimestamp: string): Promise { + async forceCleanupFilesOlderThan(cleanTimestamp: string, executable: string): Promise { // Run a dummy Gradle build to trigger cache cleanup const cleanupProjectDir = path.resolve(this.tmpDir, 'dummy-cleanup-project') fs.mkdirSync(cleanupProjectDir, {recursive: true}) @@ -55,9 +81,6 @@ export class CacheCleaner { ) fs.writeFileSync(path.resolve(cleanupProjectDir, 'build.gradle'), 'task("noop") {}') - // TODO: This is ineffective: we should be using the newest version of Gradle that ran a build, or a newer version if it's available on PATH. - const executable = await provisioner.provisionGradleAtLeast('8.12') - await core.group('Executing Gradle to clean up caches', async () => { core.info(`Cleaning up caches last used before ${cleanTimestamp}`) await this.executeCleanupBuild(executable, cleanupProjectDir) diff --git a/sources/src/caching/caches.ts b/sources/src/caching/caches.ts index d3f4356..0ffd4b0 100644 --- a/sources/src/caching/caches.ts +++ b/sources/src/caching/caches.ts @@ -102,7 +102,7 @@ export async function save( cacheListener.setCacheCleanupDisabled(CLEANUP_DISABLED_DUE_TO_CONFIG_CACHE_HIT) } else if (cacheConfig.shouldPerformCacheCleanup(buildResults.anyFailed())) { cacheListener.setCacheCleanupEnabled() - await performCacheCleanup(gradleUserHome) + await performCacheCleanup(gradleUserHome, buildResults) } else { core.info('Not performing cache-cleanup due to build failure') cacheListener.setCacheCleanupDisabled(CLEANUP_DISABLED_DUE_TO_FAILURE) @@ -114,10 +114,10 @@ export async function save( }) } -async function performCacheCleanup(gradleUserHome: string): Promise { +async function performCacheCleanup(gradleUserHome: string, buildResults: BuildResults): Promise { const cacheCleaner = new CacheCleaner(gradleUserHome, process.env['RUNNER_TEMP']!) try { - await cacheCleaner.forceCleanup() + await cacheCleaner.forceCleanup(buildResults) } catch (e) { core.warning(`Cache cleanup failed. Will continue. ${String(e)}`) } diff --git a/sources/test/jest/cache-cleanup.test.ts b/sources/test/jest/cache-cleanup.test.ts index 4bf9f98..5debc5c 100644 --- a/sources/test/jest/cache-cleanup.test.ts +++ b/sources/test/jest/cache-cleanup.test.ts @@ -28,7 +28,7 @@ test('will cleanup unused dependency jars and build-cache entries', async () => expect(fs.existsSync(commonsMath311)).toBe(true) expect(fs.readdirSync(buildCacheDir).length).toBe(4) // gc.properties, build-cache-1.lock, and 2 task entries - await cacheCleaner.forceCleanupFilesOlderThan(timestamp) + await cacheCleaner.forceCleanupFilesOlderThan(timestamp, 'gradle') expect(fs.existsSync(commonsMath31)).toBe(false) expect(fs.existsSync(commonsMath311)).toBe(true) @@ -68,7 +68,7 @@ test('will cleanup unused gradle versions', async () => { // The wrapper won't be removed if it was recently downloaded. Age it. setUtimes(wrapper802, new Date(Date.now() - 48 * 60 * 60 * 1000)) - await cacheCleaner.forceCleanupFilesOlderThan(timestamp) + await cacheCleaner.forceCleanupFilesOlderThan(timestamp, 'gradle') expect(fs.existsSync(gradle802)).toBe(false) expect(fs.existsSync(transforms3)).toBe(false)