Improve cache logging

This should fix the warnings issued when saving artifact bundles.

.github/workflows/integTest-action-inputs.yml

@@ -0,0 +1,27 @@
+name: Test different action inputs
+
+on:
+  pull_request:
+  push:
+  workflow_dispatch:
+
+env:
+  CACHE_KEY_PREFIX: ${{github.workflow}}#${{github.run_number}}-
+
+jobs:
+  action-inputs:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout sources
+      uses: actions/checkout@v2
+    - name: Invoke with multi-line arguments
+      uses: ./
+      with:
+        build-root-directory: __tests__/samples/groovy-dsl
+        arguments: |
+            --configuration-cache
+            --build-cache
+            -DsystemProperty=FOO
+            -PgradleProperty=BAR
+            test
+            jar

.github/workflows/integTest-caching.yml

@@ -82,3 +82,19 @@ jobs:
         build-root-directory: __tests__/samples/groovy-dsl
         arguments: test --configuration-cache
         cache-read-only: true
+
+  # Check that the build can run when no bundles are restored
+  no-bundles-restored:
+    needs: seed-build
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout sources
+      uses: actions/checkout@v2
+    - name: Execute Gradle build with no cache artifact bundles restored
+      uses: ./
+      with:
+        build-root-directory: __tests__/samples/groovy-dsl
+        arguments: test
+        cache-artifact-bundles: '[]'
+        cache-read-only: true
+

.github/workflows/integTest-execution.yml

@@ -33,7 +33,7 @@ jobs:
       with:
         gradle-version: release-candidate
         build-root-directory: __tests__/samples/no-wrapper
-        arguments: help -DgradleVersionCheck=7.2
+        arguments: help
     - name: Test use defined Gradle executable
       uses: ./
       with:

README.md

@@ -47,6 +47,7 @@ Each invocation will start its run with the filesystem state remaining from the
 ### Command-line arguments
 
 The `arguments` input can used to pass arbitrary arguments to the `gradle` command line.
+Arguments can be supplied in a single line, or as a multi-line input.
 
 Here are some valid examples:
 ```yaml
@@ -54,8 +55,11 @@ arguments: build
 arguments: check --scan
 arguments: some arbitrary tasks
 arguments: build -PgradleProperty=foo
-arguments: build -DsystemProperty=bar
+arguments: |
-....
+    build
+    --scan
+    -PgradleProperty=foo
+    -DsystemProperty=bar
 ```
 
 See `gradle --help` for more information.
@@ -182,6 +186,17 @@ You can enable read-only caching for any of the caches as follows:
 cache-read-only: true
 ```
 
+### Cache debugging
+
+It is possible to enable additional debug logging for cache operations. You do via the `CACHE_DEBUG_ENABLED` environment variable:
+
+```yaml
+env:
+  CACHE_DEBUG_ENABLED: true
+```
+
+Note that this setting will also prevent certain cache operations from running in parallel, further assisting with debugging.
+
 ## Build scans
 
 If your build publishes a [build scan](https://gradle.com/build-scans/) the `gradle-build-action` action will:

action.yml

@@ -14,7 +14,7 @@ inputs:
     description: Path to the root directory of the build
     required: false
   arguments:
-    description: Gradle command line arguments, see gradle --help
+    description: Gradle command line arguments (supports multi-line input)
     required: false
   cache-disabled:
     description: When 'true', all caching is disabled. No entries will be written to or read from the cache.
@@ -30,6 +30,16 @@ inputs:
     description: Used to uniquely identify the current job invocation. Defaults to the matrix values for this job; this should not be overridden by users.
     required: false
     default: ${{ toJSON(matrix) }}
+  cache-artifact-bundles:
+    description: Names and patterns of artifact bundles to cache separately. For internal use only.
+    required: false
+    default: |
+        [
+          ["generated-gradle-jars", "caches/*/generated-gradle-jars/*.jar"],
+          ["wrapper-zips", "wrapper/dists/*/*/*.zip"],
+          ["dependency-jars", "caches/modules-*/files-*/**/*.jar"],
+          ["instrumented-jars", "caches/jars-*/*/"]
+        ]
 
 outputs:
   build-scan-url:

src/cache-gradle-user-home.ts

@@ -5,18 +5,16 @@ import * as core from '@actions/core'
 import * as glob from '@actions/glob'
 import * as exec from '@actions/exec'
 
-import {AbstractCache, hashFileNames} from './cache-utils'
+import {
+    AbstractCache,
+    getCacheKeyPrefix,
+    hashFileNames,
+    tryDelete
+} from './cache-utils'
 
 // Which paths under Gradle User Home should be cached
 const CACHE_PATH = ['caches', 'notifications']
 
-const COMMON_ARTIFACT_CACHES = new Map([
-    ['generated-gradle-jars', 'caches/*/generated-gradle-jars/*.jar'],
-    ['wrapper-zips', 'wrapper/dists/*/*/*.zip'],
-    ['dependency-jars', 'caches/modules-*/files-*/**/*.jar'],
-    ['instrumented-jars', 'caches/jars-*/*/*.jar']
-])
-
 export class GradleUserHomeCache extends AbstractCache {
     private gradleUserHome: string
 
@@ -26,15 +24,15 @@ export class GradleUserHomeCache extends AbstractCache {
     }
 
     async afterRestore(): Promise<void> {
-        await this.reportCacheEntrySize('as restored from cache')
+        await this.reportGradleUserHomeSize('as restored from cache')
-        await this.restoreCommonArtifacts()
+        await this.restoreArtifactBundles()
-        await this.reportCacheEntrySize('after restoring common artifacts')
+        await this.reportGradleUserHomeSize('after restoring common artifacts')
     }
 
-    private async restoreCommonArtifacts(): Promise<void> {
+    private async restoreArtifactBundles(): Promise<void> {
         const processes: Promise<void>[] = []
-        for (const [bundle, pattern] of this.getCommonArtifactPaths()) {
+        for (const [bundle, pattern] of this.getArtifactBundles()) {
-            const p = this.restoreCommonArtifactBundle(bundle, pattern)
+            const p = this.restoreArtifactBundle(bundle, pattern)
             // Run sequentially when debugging enabled
             if (this.cacheDebuggingEnabled) {
                 await p
@@ -45,13 +43,13 @@ export class GradleUserHomeCache extends AbstractCache {
         await Promise.all(processes)
     }
 
-    private async restoreCommonArtifactBundle(
+    private async restoreArtifactBundle(
         bundle: string,
         artifactPath: string
     ): Promise<void> {
-        const cacheMetaFile = this.getCacheMetaFile(bundle)
+        const bundleMetaFile = this.getBundleMetaFile(bundle)
-        if (fs.existsSync(cacheMetaFile)) {
+        if (fs.existsSync(bundleMetaFile)) {
-            const cacheKey = fs.readFileSync(cacheMetaFile, 'utf-8').trim()
+            const cacheKey = fs.readFileSync(bundleMetaFile, 'utf-8').trim()
             const restoreKey = await this.restoreCache([artifactPath], cacheKey)
             if (restoreKey) {
                 core.info(
@@ -59,17 +57,17 @@ export class GradleUserHomeCache extends AbstractCache {
                 )
             } else {
                 this.debug(
-                    `Failed to restore ${bundle} with key ${cacheKey} to ${artifactPath}`
+                    `Did not restore ${bundle} with key ${cacheKey} to ${artifactPath}`
                 )
             }
         } else {
             this.debug(
-                `No metafile found to restore ${bundle}: ${cacheMetaFile}`
+                `No metafile found to restore ${bundle}: ${bundleMetaFile}`
             )
         }
     }
 
-    private getCacheMetaFile(name: string): string {
+    private getBundleMetaFile(name: string): string {
         return path.resolve(
             this.gradleUserHome,
             'caches',
@@ -77,47 +75,18 @@ export class GradleUserHomeCache extends AbstractCache {
         )
     }
 
-    private async reportCacheEntrySize(label: string): Promise<void> {
-        if (!this.cacheDebuggingEnabled) {
-            return
-        }
-        if (!fs.existsSync(this.gradleUserHome)) {
-            return
-        }
-        const result = await exec.getExecOutput(
-            'du',
-            ['-h', '-c', '-t', '5M'],
-            {
-                cwd: this.gradleUserHome,
-                silent: true,
-                ignoreReturnCode: true
-            }
-        )
-
-        core.info(`Gradle User Home cache entry (directories >5M): ${label}`)
-
-        core.info(
-            result.stdout
-                .trimEnd()
-                .replace(/\t/g, '    ')
-                .split('\n')
-                .map(it => {
-                    return `  ${it}`
-                })
-                .join('\n')
-        )
-
-        core.info('-----------------------')
-    }
-
     async beforeSave(): Promise<void> {
-        await this.saveCommonArtifacts()
+        await this.reportGradleUserHomeSize('before saving common artifacts')
+        await this.saveArtifactBundles()
+        await this.reportGradleUserHomeSize(
+            'after saving common artifacts (./wrapper dir is not cached)'
+        )
     }
 
-    private async saveCommonArtifacts(): Promise<void> {
+    private async saveArtifactBundles(): Promise<void> {
         const processes: Promise<void>[] = []
-        for (const [bundle, pattern] of this.getCommonArtifactPaths()) {
+        for (const [bundle, pattern] of this.getArtifactBundles()) {
-            const p = this.saveCommonArtifactBundle(bundle, pattern)
+            const p = this.saveArtifactBundle(bundle, pattern)
             // Run sequentially when debugging enabled
             if (this.cacheDebuggingEnabled) {
                 await p
@@ -128,28 +97,31 @@ export class GradleUserHomeCache extends AbstractCache {
         await Promise.all(processes)
     }
 
-    private async saveCommonArtifactBundle(
+    private async saveArtifactBundle(
         bundle: string,
         artifactPath: string
     ): Promise<void> {
-        const cacheMetaFile = this.getCacheMetaFile(bundle)
+        const bundleMetaFile = this.getBundleMetaFile(bundle)
 
-        const globber = await glob.create(artifactPath)
+        const globber = await glob.create(artifactPath, {
-        const commonArtifactFiles = await globber.glob()
+            implicitDescendants: false,
+            followSymbolicLinks: false
+        })
+        const bundleFiles = await globber.glob()
 
         // Handle no matching files
-        if (commonArtifactFiles.length === 0) {
+        if (bundleFiles.length === 0) {
             this.debug(`No files found to cache for ${bundle}`)
-            if (fs.existsSync(cacheMetaFile)) {
+            if (fs.existsSync(bundleMetaFile)) {
-                fs.unlinkSync(cacheMetaFile)
+                tryDelete(bundleMetaFile)
             }
             return
         }
 
-        const previouslyRestoredKey = fs.existsSync(cacheMetaFile)
+        const previouslyRestoredKey = fs.existsSync(bundleMetaFile)
-            ? fs.readFileSync(cacheMetaFile, 'utf-8').trim()
+            ? fs.readFileSync(bundleMetaFile, 'utf-8').trim()
             : ''
-        const cacheKey = this.createCacheKey(bundle, commonArtifactFiles)
+        const cacheKey = this.createCacheKey(bundle, bundleFiles)
 
         if (previouslyRestoredKey === cacheKey) {
             this.debug(
@@ -159,22 +131,26 @@ export class GradleUserHomeCache extends AbstractCache {
             core.info(`Caching ${bundle} with cache key: ${cacheKey}`)
             await this.saveCache([artifactPath], cacheKey)
 
-            this.debug(`Writing cache metafile: ${cacheMetaFile}`)
+            this.debug(`Writing cache metafile: ${bundleMetaFile}`)
-            fs.writeFileSync(cacheMetaFile, cacheKey)
+            fs.writeFileSync(bundleMetaFile, cacheKey)
         }
 
-        for (const file of commonArtifactFiles) {
+        for (const file of bundleFiles) {
-            fs.unlinkSync(file)
+            tryDelete(file)
         }
     }
 
     protected createCacheKey(bundle: string, files: string[]): string {
-        const cacheKeyPrefix = process.env['CACHE_KEY_PREFIX'] || ''
+        const cacheKeyPrefix = getCacheKeyPrefix()
         const relativeFiles = files.map(x =>
             path.relative(this.gradleUserHome, x)
         )
         const key = hashFileNames(relativeFiles)
 
+        this.debug(
+            `Generating cache key for ${bundle} from files: ${relativeFiles}`
+        )
+
         return `${cacheKeyPrefix}${bundle}-${key}`
     }
 
@@ -197,12 +173,50 @@ export class GradleUserHomeCache extends AbstractCache {
         return CACHE_PATH.map(x => path.resolve(this.gradleUserHome, x))
     }
 
-    private getCommonArtifactPaths(): Map<string, string> {
+    private getArtifactBundles(): Map<string, string> {
+        const artifactBundleDefinition = core.getInput('cache-artifact-bundles')
+        this.debug(
+            `Using artifact bundle definition: ${artifactBundleDefinition}`
+        )
+        const artifactBundles = JSON.parse(artifactBundleDefinition)
         return new Map(
-            Array.from(COMMON_ARTIFACT_CACHES, ([key, value]) => [
+            Array.from(artifactBundles, ([key, value]) => [
                 key,
                 path.resolve(this.gradleUserHome, value)
             ])
         )
     }
+
+    private async reportGradleUserHomeSize(label: string): Promise<void> {
+        if (!this.cacheDebuggingEnabled) {
+            return
+        }
+        if (!fs.existsSync(this.gradleUserHome)) {
+            return
+        }
+        const result = await exec.getExecOutput(
+            'du',
+            ['-h', '-c', '-t', '5M'],
+            {
+                cwd: this.gradleUserHome,
+                silent: true,
+                ignoreReturnCode: true
+            }
+        )
+
+        core.info(`Gradle User Home (directories >5M): ${label}`)
+
+        core.info(
+            result.stdout
+                .trimEnd()
+                .replace(/\t/g, '    ')
+                .split('\n')
+                .map(it => {
+                    return `  ${it}`
+                })
+                .join('\n')
+        )
+
+        core.info('-----------------------')
+    }
 }

src/cache-utils.ts

@@ -3,6 +3,7 @@ import * as cache from '@actions/cache'
 import * as github from '@actions/github'
 import * as crypto from 'crypto'
 import * as path from 'path'
+import * as fs from 'fs'
 
 export function isCacheDisabled(): boolean {
     return core.getBooleanInput('cache-disabled')
@@ -16,9 +17,13 @@ export function isCacheDebuggingEnabled(): boolean {
     return process.env['CACHE_DEBUG_ENABLED'] ? true : false
 }
 
+export function getCacheKeyPrefix(): string {
+    // Prefix can be used to force change all cache keys (defaults to cache protocol version)
+    return process.env['CACHE_KEY_PREFIX'] || 'v2-'
+}
+
 function generateCacheKey(cacheName: string): CacheKey {
-    // Prefix can be used to force change all cache keys
+    const cacheKeyPrefix = getCacheKeyPrefix()
-    const cacheKeyPrefix = process.env['CACHE_KEY_PREFIX'] || ''
 
     // At the most general level, share caches for all executions on the same OS
     const runnerOs = process.env['RUNNER_OS'] || ''
@@ -60,6 +65,34 @@ export function hashFileNames(fileNames: string[]): string {
     )
 }
 
+/**
+ * Attempt to delete a file or directory, waiting to allow locks to be released
+ */
+export async function tryDelete(file: string): Promise<void> {
+    const stat = fs.lstatSync(file)
+    for (let count = 0; count < 3; count++) {
+        try {
+            if (stat.isDirectory()) {
+                fs.rmdirSync(file, {recursive: true})
+            } else {
+                fs.unlinkSync(file)
+            }
+            return
+        } catch (error) {
+            if (count === 2) {
+                throw error
+            } else {
+                core.warning(String(error))
+                await delay(1000)
+            }
+        }
+    }
+}
+
+async function delay(ms: number): Promise<void> {
+    return new Promise(resolve => setTimeout(resolve, ms))
+}
+
 class CacheKey {
     key: string
     restoreKeys: string[]
@@ -98,6 +131,12 @@ export abstract class AbstractCache {
 
         core.saveState(this.cacheKeyStateKey, cacheKey.key)
 
+        this.debug(
+            `Requesting ${this.cacheDescription} with
+                key:${cacheKey.key}
+                restoreKeys:[${cacheKey.restoreKeys}]`
+        )
+
         const cacheResult = await this.restoreCache(
             this.getCachePath(),
             cacheKey.key,
@@ -117,7 +156,13 @@ export abstract class AbstractCache {
             `Restored ${this.cacheDescription} from cache key: ${cacheResult}`
         )
 
-        await this.afterRestore()
+        try {
+            await this.afterRestore()
+        } catch (error) {
+            core.warning(
+                `Restore ${this.cacheDescription} failed in 'afterRestore': ${error}`
+            )
+        }
 
         return
     }
@@ -169,7 +214,14 @@ export abstract class AbstractCache {
             return
         }
 
-        await this.beforeSave()
+        try {
+            await this.beforeSave()
+        } catch (error) {
+            core.warning(
+                `Save ${this.cacheDescription} failed in 'beforeSave': ${error}`
+            )
+            return
+        }
 
         core.info(
             `Caching ${this.cacheDescription} with cache key: ${cacheKey}`

src/caches.ts

@@ -7,7 +7,9 @@ const BUILD_ROOT_DIR = 'BUILD_ROOT_DIR'
 
 export async function restore(buildRootDirectory: string): Promise<void> {
     if (isCacheDisabled()) {
-        core.debug('Cache read disabled')
+        core.info(
+            'Cache is disabled: will not restore state from previous builds.'
+        )
         return
     }
 
@@ -22,7 +24,9 @@ export async function restore(buildRootDirectory: string): Promise<void> {
 
 export async function save(): Promise<void> {
     if (isCacheReadOnly()) {
-        core.debug('Cache is read-only: not saving cache entry')
+        core.info(
+            'Cache is read-only: will not save state for use in subsequent builds.'
+        )
         return
     }