Choose best Gradle version to use for cache cleanup (#526)

The cache-cleanup operation works by executing Gradle on a dummy project
and a custom init-script. The init-script requires at least Gradle 8.11
to work.

Ideally, the version of Gradle used for cleanup should be no older than
the newest one that wrote entries to Gradle User Home. If an older
Gradle version is used for cache-cleanup, it will not remove entries
written specifically for newer versions.

With this change, we now attempt to ensure that cache-cleanup is run
with the best Gradle version available. We inspect the Gradle version on
PATH to see if it is new enough, otherwise we will provision a Gradle
version equal to the newest one that ran in the Job.

The logic is:
- Determine the newest version of Gradle that was executed during the
Job. This is the 'minimum version' for cache cleanup.
- Inspect the Gradle version on PATH (if any) to see if it is equal to
or newer than the 'minimum version'.
- If the version Gradle on PATH is new enough, use that version for
cache-cleanup.
- If not, attempt to provision Gradle with the 'minimum version'.

Fixes #436

.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

build

@@ -36,6 +36,10 @@ case "$1" in
         npm clean-install
         npm run build
         ;;
+    test)
+        shift
+        npm test -- $@
+        ;;
     *)
         npm run build
         ;;

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 {

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<void> {
+    async forceCleanup(buildResults: BuildResults): Promise<void> {
+        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<string> {
+        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<void> {
+    async forceCleanupFilesOlderThan(cleanTimestamp: string, executable: string): Promise<void> {
         // 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)

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<void> {
+async function performCacheCleanup(gradleUserHome: string, buildResults: BuildResults): Promise<void> {
     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)}`)
     }

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<GradleExecutable | undefined> {
@@ -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]
+    }
+}

sources/src/execution/provision.ts

@@ -88,7 +88,7 @@ async function gradleReleaseNightly(): Promise<GradleVersionInfo> {
 async function gradleRelease(version: string): Promise<GradleVersionInfo> {
     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
 }

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)

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})
 

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 = `