From c98dcdec10cb8a608340cdf4e5cf041ca4edca6c Mon Sep 17 00:00:00 2001 From: Dmitry Shibanov Date: Wed, 3 Nov 2021 13:10:35 +0300 Subject: [PATCH] resolving comments --- action.yml | 2 +- dist/cache-save/index.js | 26 ++++++++-------- dist/setup/index.js | 32 ++++++++++---------- src/cache-distributions/cache-distributor.ts | 13 +++++--- src/cache-distributions/cache-factory.ts | 6 ++-- src/cache-distributions/pip-cache.ts | 10 +++--- src/cache-distributions/pipenv-cache.ts | 6 ++-- src/cache-save.ts | 18 ++++++----- 8 files changed, 61 insertions(+), 52 deletions(-) diff --git a/action.yml b/action.yml index 861b5bda..bd742381 100644 --- a/action.yml +++ b/action.yml @@ -15,7 +15,7 @@ inputs: description: Used to pull python distributions from actions/python-versions. Since there's a default, this is typically not supplied by the user. default: ${{ github.token }} cache-dependency-path: - description: 'Used to specify a package manager for caching in the default directory. Supported values: pip, pipenv' + description: 'Used to specify the path to a dependency files. Supports wildcards or a list of file names for caching multiple dependencies.' outputs: python-version: description: "The installed python version. Useful when given a version range as input." diff --git a/dist/cache-save/index.js b/dist/cache-save/index.js index e951ef8d..0de249fd 100644 --- a/dist/cache-save/index.js +++ b/dist/cache-save/index.js @@ -37180,22 +37180,22 @@ var State; State["CACHE_PATHS"] = "cache-paths"; })(State = exports.State || (exports.State = {})); class CacheDistributor { - constructor(toolName, patterns) { + constructor(toolName, cacheDependencyPath) { this.toolName = toolName; - this.patterns = patterns; + this.cacheDependencyPath = cacheDependencyPath; this.CACHE_KEY_PREFIX = 'setup-python'; } restoreCache() { return __awaiter(this, void 0, void 0, function* () { const { primaryKey, restoreKey } = yield this.computeKeys(); - const cachePath = yield this.getCacheGlobalDirectories(); - core.saveState(State.CACHE_PATHS, cachePath); - core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); if (primaryKey.endsWith('-')) { - throw new Error(`No file in ${process.cwd()} matched to [${this.patterns + throw new Error(`No file in ${process.cwd()} matched to [${this.cacheDependencyPath .split('\n') .join(',')}], make sure you have checked out the target repository`); } + const cachePath = yield this.getCacheGlobalDirectories(); + core.saveState(State.CACHE_PATHS, cachePath); + core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey); if (matchedKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); @@ -45796,7 +45796,7 @@ function run() { try { const cache = core.getInput('cache'); if (cache) { - yield saveCache(); + yield saveCache(cache); } } catch (error) { @@ -45805,12 +45805,12 @@ function run() { } }); } -function saveCache() { +function saveCache(packageManager) { return __awaiter(this, void 0, void 0, function* () { - const cacheDirPaths = JSON.parse(core.getState(cache_distributor_1.State.CACHE_PATHS)); - core.debug(`paths for caching are ${cacheDirPaths.join(', ')}`); - if (!isCacheDirectoryExists(cacheDirPaths)) { - throw new Error('Cache directories do not exist'); + const cachePaths = JSON.parse(core.getState(cache_distributor_1.State.CACHE_PATHS)); + core.debug(`paths for caching are ${cachePaths.join(', ')}`); + if (!isCacheDirectoryExists(cachePaths)) { + throw new Error(`Cache folder path is retrieved for ${packageManager} but doesn't exist on disk: ${cachePaths.join(', ')}`); } const primaryKey = core.getState(cache_distributor_1.State.STATE_CACHE_PRIMARY_KEY); const matchedKey = core.getState(cache_distributor_1.State.CACHE_MATCHED_KEY); @@ -45824,7 +45824,7 @@ function saveCache() { return; } try { - yield cache.saveCache(cacheDirPaths, primaryKey); + yield cache.saveCache(cachePaths, primaryKey); core.info(`Cache saved with the key: ${primaryKey}`); } catch (error) { diff --git a/dist/setup/index.js b/dist/setup/index.js index 14ad6f3f..cddfc666 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -7156,9 +7156,9 @@ class PipenvCache extends cache_distributor_1.default { } getCacheGlobalDirectories() { return __awaiter(this, void 0, void 0, function* () { - const cachePath = path.join(os.homedir(), this.getVirtualenvsPath()); - core.debug(`Pipenv virtualenvs path is ${cachePath}`); - return [cachePath]; + const resolvedPath = path.join(os.homedir(), this.getVirtualenvsPath()); + core.debug(`global cache directory path is ${resolvedPath}`); + return [resolvedPath]; }); } computeKeys() { @@ -34460,8 +34460,8 @@ const path = __importStar(__webpack_require__(622)); const os_1 = __importDefault(__webpack_require__(87)); const cache_distributor_1 = __importDefault(__webpack_require__(435)); class PipCache extends cache_distributor_1.default { - constructor(patterns = '**/requirements.txt') { - super('pip', patterns); + constructor(cacheDependencyPath = '**/requirements.txt') { + super('pip', cacheDependencyPath); } getCacheGlobalDirectories() { return __awaiter(this, void 0, void 0, function* () { @@ -34479,9 +34479,9 @@ class PipCache extends cache_distributor_1.default { } computeKeys() { return __awaiter(this, void 0, void 0, function* () { - const hash = yield glob.hashFiles(this.patterns); + const hash = yield glob.hashFiles(this.cacheDependencyPath); const primaryKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${this.toolName}-${hash}`; - const restoreKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${this.toolName}-`; + const restoreKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${this.toolName}`; return { primaryKey, restoreKey: [restoreKey] @@ -35424,22 +35424,22 @@ var State; State["CACHE_PATHS"] = "cache-paths"; })(State = exports.State || (exports.State = {})); class CacheDistributor { - constructor(toolName, patterns) { + constructor(toolName, cacheDependencyPath) { this.toolName = toolName; - this.patterns = patterns; + this.cacheDependencyPath = cacheDependencyPath; this.CACHE_KEY_PREFIX = 'setup-python'; } restoreCache() { return __awaiter(this, void 0, void 0, function* () { const { primaryKey, restoreKey } = yield this.computeKeys(); - const cachePath = yield this.getCacheGlobalDirectories(); - core.saveState(State.CACHE_PATHS, cachePath); - core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); if (primaryKey.endsWith('-')) { - throw new Error(`No file in ${process.cwd()} matched to [${this.patterns + throw new Error(`No file in ${process.cwd()} matched to [${this.cacheDependencyPath .split('\n') .join(',')}], make sure you have checked out the target repository`); } + const cachePath = yield this.getCacheGlobalDirectories(); + core.saveState(State.CACHE_PATHS, cachePath); + core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey); if (matchedKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); @@ -43887,13 +43887,13 @@ var PackageManagers; PackageManagers["Pip"] = "pip"; PackageManagers["Pipenv"] = "pipenv"; })(PackageManagers = exports.PackageManagers || (exports.PackageManagers = {})); -function getCacheDistributor(packageManager, pythonVersion, patterns) { +function getCacheDistributor(packageManager, pythonVersion, cacheDependencyPath) { return __awaiter(this, void 0, void 0, function* () { switch (packageManager) { case PackageManagers.Pip: - return new pip_cache_1.default(patterns); + return new pip_cache_1.default(cacheDependencyPath); case PackageManagers.Pipenv: - return new pipenv_cache_1.default(pythonVersion, patterns); + return new pipenv_cache_1.default(pythonVersion, cacheDependencyPath); default: throw new Error(`Caching for '${packageManager}' is not supported`); } diff --git a/src/cache-distributions/cache-distributor.ts b/src/cache-distributions/cache-distributor.ts index e049c354..7a8a6b8f 100644 --- a/src/cache-distributions/cache-distributor.ts +++ b/src/cache-distributions/cache-distributor.ts @@ -9,7 +9,7 @@ export enum State { abstract class CacheDistributor { protected CACHE_KEY_PREFIX = 'setup-python'; - constructor(protected toolName: string, protected patterns: string) {} + constructor(protected toolName: string, protected cacheDependencyPath: string) {} protected abstract getCacheGlobalDirectories(): Promise; protected abstract computeKeys(): Promise<{ @@ -19,22 +19,25 @@ abstract class CacheDistributor { public async restoreCache() { const {primaryKey, restoreKey} = await this.computeKeys(); - const cachePath = await this.getCacheGlobalDirectories(); - core.saveState(State.CACHE_PATHS, cachePath); - core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); if (primaryKey.endsWith('-')) { throw new Error( - `No file in ${process.cwd()} matched to [${this.patterns + `No file in ${process.cwd()} matched to [${this.cacheDependencyPath .split('\n') .join(',')}], make sure you have checked out the target repository` ); } + const cachePath = await this.getCacheGlobalDirectories(); + + core.saveState(State.CACHE_PATHS, cachePath); + core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey); + const matchedKey = await cache.restoreCache( cachePath, primaryKey, restoreKey ); + if (matchedKey) { core.saveState(State.CACHE_MATCHED_KEY, matchedKey); core.info(`Cache restored from key: ${matchedKey}`); diff --git a/src/cache-distributions/cache-factory.ts b/src/cache-distributions/cache-factory.ts index 2af9829e..cd6a55be 100644 --- a/src/cache-distributions/cache-factory.ts +++ b/src/cache-distributions/cache-factory.ts @@ -9,13 +9,13 @@ export enum PackageManagers { export async function getCacheDistributor( packageManager: string, pythonVersion: string, - patterns: string | undefined + cacheDependencyPath: string | undefined ) { switch (packageManager) { case PackageManagers.Pip: - return new PipCache(patterns); + return new PipCache(cacheDependencyPath); case PackageManagers.Pipenv: - return new PipenvCache(pythonVersion, patterns); + return new PipenvCache(pythonVersion, cacheDependencyPath); default: throw new Error(`Caching for '${packageManager}' is not supported`); } diff --git a/src/cache-distributions/pip-cache.ts b/src/cache-distributions/pip-cache.ts index b83ab7ef..f83ea2a1 100644 --- a/src/cache-distributions/pip-cache.ts +++ b/src/cache-distributions/pip-cache.ts @@ -8,14 +8,15 @@ import os from 'os'; import CacheDistributor from './cache-distributor'; class PipCache extends CacheDistributor { - constructor(patterns: string = '**/requirements.txt') { - super('pip', patterns); + constructor(cacheDependencyPath: string = '**/requirements.txt') { + super('pip', cacheDependencyPath); } protected async getCacheGlobalDirectories() { const {stdout, stderr, exitCode} = await exec.getExecOutput( 'pip cache dir' ); + if (stderr) { throw new Error( `Could not get cache folder path for pip package manager` @@ -34,9 +35,10 @@ class PipCache 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']}-${this.toolName}-${hash}`; - const restoreKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${this.toolName}-`; + const restoreKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${this.toolName}`; + return { primaryKey, restoreKey: [restoreKey] diff --git a/src/cache-distributions/pipenv-cache.ts b/src/cache-distributions/pipenv-cache.ts index bf1720b3..6fe8974c 100644 --- a/src/cache-distributions/pipenv-cache.ts +++ b/src/cache-distributions/pipenv-cache.ts @@ -22,10 +22,10 @@ class PipenvCache extends CacheDistributor { } protected async getCacheGlobalDirectories() { - const cachePath = path.join(os.homedir(), this.getVirtualenvsPath()); - core.debug(`Pipenv virtualenvs path is ${cachePath}`); + const resolvedPath = path.join(os.homedir(), this.getVirtualenvsPath()); + core.debug(`global cache directory path is ${resolvedPath}`); - return [cachePath]; + return [resolvedPath]; } protected async computeKeys() { diff --git a/src/cache-save.ts b/src/cache-save.ts index 9779a3d7..bb6092ea 100644 --- a/src/cache-save.ts +++ b/src/cache-save.ts @@ -8,7 +8,7 @@ async function run() { try { const cache = core.getInput('cache'); if (cache) { - await saveCache(); + await saveCache(cache); } } catch (error) { const err = error as Error; @@ -16,14 +16,17 @@ async function run() { } } -async function saveCache() { - const cacheDirPaths = JSON.parse( +async function saveCache(packageManager: string) { + const cachePaths = JSON.parse( core.getState(State.CACHE_PATHS) ) as string[]; - core.debug(`paths for caching are ${cacheDirPaths.join(', ')}`); - if (!isCacheDirectoryExists(cacheDirPaths)) { - throw new Error('Cache directories do not exist'); + + core.debug(`paths for caching are ${cachePaths.join(', ')}`); + + if (!isCacheDirectoryExists(cachePaths)) { + throw new Error(`Cache folder path is retrieved for ${packageManager} but doesn't exist on disk: ${cachePaths.join(', ')}`); } + const primaryKey = core.getState(State.STATE_CACHE_PRIMARY_KEY); const matchedKey = core.getState(State.CACHE_MATCHED_KEY); @@ -37,8 +40,9 @@ async function saveCache() { ); return; } + try { - await cache.saveCache(cacheDirPaths, primaryKey); + await cache.saveCache(cachePaths, primaryKey); core.info(`Cache saved with the key: ${primaryKey}`); } catch (error) { const err = error as Error;