From 1f3a57058480920431880ee9825c81e3140b9a7b Mon Sep 17 00:00:00 2001 From: Sam Pinkus Date: Wed, 8 Jan 2025 16:01:51 +1000 Subject: [PATCH 1/3] Readability improvement: rename PipenvCache,PoetryCache "patterns" field to "cacheDependencyPath" to be consistent with base class and cache-factory. --- src/cache-distributions/pipenv-cache.ts | 6 +++--- src/cache-distributions/poetry-cache.ts | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/cache-distributions/pipenv-cache.ts b/src/cache-distributions/pipenv-cache.ts index 8ddcbed8..79674c20 100644 --- a/src/cache-distributions/pipenv-cache.ts +++ b/src/cache-distributions/pipenv-cache.ts @@ -8,9 +8,9 @@ import CacheDistributor from './cache-distributor'; class PipenvCache extends CacheDistributor { constructor( private pythonVersion: string, - protected patterns: string = '**/Pipfile.lock' + protected cacheDependencyPath: string = '**/Pipfile.lock' ) { - super('pipenv', patterns); + super('pipenv', cacheDependencyPath); } protected async getCacheGlobalDirectories() { @@ -31,7 +31,7 @@ class PipenvCache extends CacheDistributor { } protected async computeKeys() { - const hash = await glob.hashFiles(this.patterns); + const hash = await glob.hashFiles(this.cacheDependencyPath); const primaryKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${process.arch}-python-${this.pythonVersion}-${this.packageManager}-${hash}`; const restoreKey = undefined; return { diff --git a/src/cache-distributions/poetry-cache.ts b/src/cache-distributions/poetry-cache.ts index cd3a189b..90739dbc 100644 --- a/src/cache-distributions/poetry-cache.ts +++ b/src/cache-distributions/poetry-cache.ts @@ -10,16 +10,16 @@ import {logWarning} from '../utils'; class PoetryCache extends CacheDistributor { constructor( private pythonVersion: string, - protected patterns: string = '**/poetry.lock', + protected cacheDependencyPath: string = '**/poetry.lock', protected poetryProjects: Set = new Set() ) { - super('poetry', patterns); + super('poetry', cacheDependencyPath); } protected async getCacheGlobalDirectories() { // Same virtualenvs path may appear for different projects, hence we use a Set const paths = new Set(); - const globber = await glob.create(this.patterns); + const globber = await glob.create(this.cacheDependencyPath); for await (const file of globber.globGenerator()) { const basedir = path.dirname(file); @@ -45,7 +45,7 @@ class PoetryCache extends CacheDistributor { } protected async computeKeys() { - const hash = await glob.hashFiles(this.patterns); + const hash = await glob.hashFiles(this.cacheDependencyPath); // "v2" is here to invalidate old caches of this cache distributor, which were created broken: const primaryKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${process.arch}-python-${this.pythonVersion}-${this.packageManager}-v2-${hash}`; const restoreKey = undefined; From 0024ce0d1431e6ee4caaa02949514d8adf36b25e Mon Sep 17 00:00:00 2001 From: Sam Pinkus Date: Wed, 8 Jan 2025 16:09:52 +1000 Subject: [PATCH 2/3] Readability improvement: Move CacheDistributor constructor params to readonly abstract fields of concrete cache implementations. Remove empty CacheDistributor constructor. --- src/cache-distributions/cache-distributor.ts | 6 ++---- src/cache-distributions/pip-cache.ts | 5 +++-- src/cache-distributions/pipenv-cache.ts | 6 ++++-- src/cache-distributions/poetry-cache.ts | 7 +++++-- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/cache-distributions/cache-distributor.ts b/src/cache-distributions/cache-distributor.ts index 211946fb..55a5a3d6 100644 --- a/src/cache-distributions/cache-distributor.ts +++ b/src/cache-distributions/cache-distributor.ts @@ -10,10 +10,8 @@ export enum State { abstract class CacheDistributor { protected CACHE_KEY_PREFIX = 'setup-python'; - constructor( - protected packageManager: string, - protected cacheDependencyPath: string - ) {} + protected abstract readonly packageManager: string; + protected abstract readonly cacheDependencyPath: string; protected abstract getCacheGlobalDirectories(): Promise; protected abstract computeKeys(): Promise<{ diff --git a/src/cache-distributions/pip-cache.ts b/src/cache-distributions/pip-cache.ts index d64ae931..c1c9d022 100644 --- a/src/cache-distributions/pip-cache.ts +++ b/src/cache-distributions/pip-cache.ts @@ -12,12 +12,13 @@ import {CACHE_DEPENDENCY_BACKUP_PATH} from './constants'; class PipCache extends CacheDistributor { private cacheDependencyBackupPath: string = CACHE_DEPENDENCY_BACKUP_PATH; + protected readonly packageManager = 'pip'; constructor( private pythonVersion: string, - cacheDependencyPath = '**/requirements.txt' + protected readonly cacheDependencyPath = '**/requirements.txt' ) { - super('pip', cacheDependencyPath); + super(); } protected async getCacheGlobalDirectories() { diff --git a/src/cache-distributions/pipenv-cache.ts b/src/cache-distributions/pipenv-cache.ts index 79674c20..978f63f7 100644 --- a/src/cache-distributions/pipenv-cache.ts +++ b/src/cache-distributions/pipenv-cache.ts @@ -6,11 +6,13 @@ import * as core from '@actions/core'; import CacheDistributor from './cache-distributor'; class PipenvCache extends CacheDistributor { + protected readonly packageManager = 'pipenv'; + constructor( private pythonVersion: string, - protected cacheDependencyPath: string = '**/Pipfile.lock' + protected readonly cacheDependencyPath: string = '**/Pipfile.lock' ) { - super('pipenv', cacheDependencyPath); + super(); } protected async getCacheGlobalDirectories() { diff --git a/src/cache-distributions/poetry-cache.ts b/src/cache-distributions/poetry-cache.ts index 90739dbc..1baffafd 100644 --- a/src/cache-distributions/poetry-cache.ts +++ b/src/cache-distributions/poetry-cache.ts @@ -8,12 +8,15 @@ import CacheDistributor from './cache-distributor'; import {logWarning} from '../utils'; class PoetryCache extends CacheDistributor { + protected readonly packageManager = 'poetry'; + + constructor( private pythonVersion: string, - protected cacheDependencyPath: string = '**/poetry.lock', + protected readonly cacheDependencyPath: string = '**/poetry.lock', protected poetryProjects: Set = new Set() ) { - super('poetry', cacheDependencyPath); + super(); } protected async getCacheGlobalDirectories() { From d99639b3c5aa255304dc050e96e317c42f939cf9 Mon Sep 17 00:00:00 2001 From: Sam Pinkus Date: Wed, 8 Jan 2025 16:37:25 +1000 Subject: [PATCH 3/3] Code improvemnt: Rm CacheDistributor.cacheDependencyPath field and pass back from computeKeys instead, simplifying CacheDistributor.restoreCache error check. Rm now uneeded constant.ts file. --- src/cache-distributions/cache-distributor.ts | 12 +++--------- src/cache-distributions/constants.ts | 1 - src/cache-distributions/pip-cache.ts | 15 +++++++++------ src/cache-distributions/pipenv-cache.ts | 3 ++- src/cache-distributions/poetry-cache.ts | 3 ++- 5 files changed, 16 insertions(+), 18 deletions(-) delete mode 100644 src/cache-distributions/constants.ts diff --git a/src/cache-distributions/cache-distributor.ts b/src/cache-distributions/cache-distributor.ts index 55a5a3d6..efc9eee4 100644 --- a/src/cache-distributions/cache-distributor.ts +++ b/src/cache-distributions/cache-distributor.ts @@ -1,6 +1,5 @@ import * as cache from '@actions/cache'; import * as core from '@actions/core'; -import {CACHE_DEPENDENCY_BACKUP_PATH} from './constants'; export enum State { STATE_CACHE_PRIMARY_KEY = 'cache-primary-key', @@ -11,24 +10,19 @@ export enum State { abstract class CacheDistributor { protected CACHE_KEY_PREFIX = 'setup-python'; protected abstract readonly packageManager: string; - protected abstract readonly cacheDependencyPath: string; protected abstract getCacheGlobalDirectories(): Promise; protected abstract computeKeys(): Promise<{ primaryKey: string; restoreKey: string[] | undefined; + cacheDependencyPath: string; }>; protected async handleLoadedCache() {} public async restoreCache() { - const {primaryKey, restoreKey} = await this.computeKeys(); + const {primaryKey, restoreKey, cacheDependencyPath} = await this.computeKeys(); if (primaryKey.endsWith('-')) { - const file = - this.packageManager === 'pip' - ? `${this.cacheDependencyPath - .split('\n') - .join(',')} or ${CACHE_DEPENDENCY_BACKUP_PATH}` - : this.cacheDependencyPath.split('\n').join(','); + const file = cacheDependencyPath.split('\n').join(','); throw new Error( `No file in ${process.cwd()} matched to [${file}], make sure you have checked out the target repository` ); diff --git a/src/cache-distributions/constants.ts b/src/cache-distributions/constants.ts deleted file mode 100644 index 08dc7450..00000000 --- a/src/cache-distributions/constants.ts +++ /dev/null @@ -1 +0,0 @@ -export const CACHE_DEPENDENCY_BACKUP_PATH = '**/pyproject.toml'; diff --git a/src/cache-distributions/pip-cache.ts b/src/cache-distributions/pip-cache.ts index c1c9d022..16450c67 100644 --- a/src/cache-distributions/pip-cache.ts +++ b/src/cache-distributions/pip-cache.ts @@ -8,10 +8,9 @@ import os from 'os'; import CacheDistributor from './cache-distributor'; import {getLinuxInfo, IS_LINUX, IS_WINDOWS} from '../utils'; -import {CACHE_DEPENDENCY_BACKUP_PATH} from './constants'; class PipCache extends CacheDistributor { - private cacheDependencyBackupPath: string = CACHE_DEPENDENCY_BACKUP_PATH; + private cacheDependencyBackupPath = '**/pyproject.toml'; protected readonly packageManager = 'pip'; constructor( @@ -60,9 +59,12 @@ class PipCache extends CacheDistributor { } protected async computeKeys() { - const hash = - (await glob.hashFiles(this.cacheDependencyPath)) || - (await glob.hashFiles(this.cacheDependencyBackupPath)); + let cacheDependencyPath = this.cacheDependencyPath; + let hash = await glob.hashFiles(this.cacheDependencyPath); + if(!hash) { + hash = await glob.hashFiles(this.cacheDependencyBackupPath); + cacheDependencyPath = this.cacheDependencyBackupPath; + } let primaryKey = ''; let restoreKey = ''; @@ -77,7 +79,8 @@ class PipCache extends CacheDistributor { return { primaryKey, - restoreKey: [restoreKey] + restoreKey: [restoreKey], + cacheDependencyPath, }; } } diff --git a/src/cache-distributions/pipenv-cache.ts b/src/cache-distributions/pipenv-cache.ts index 978f63f7..66627caf 100644 --- a/src/cache-distributions/pipenv-cache.ts +++ b/src/cache-distributions/pipenv-cache.ts @@ -38,7 +38,8 @@ class PipenvCache extends CacheDistributor { const restoreKey = undefined; return { primaryKey, - restoreKey + restoreKey, + cacheDependencyPath: this.cacheDependencyPath, }; } } diff --git a/src/cache-distributions/poetry-cache.ts b/src/cache-distributions/poetry-cache.ts index 1baffafd..b8b83a25 100644 --- a/src/cache-distributions/poetry-cache.ts +++ b/src/cache-distributions/poetry-cache.ts @@ -54,7 +54,8 @@ class PoetryCache extends CacheDistributor { const restoreKey = undefined; return { primaryKey, - restoreKey + restoreKey, + cacheDependencyPath: this.cacheDependencyPath, }; }