From 12648859835f68b273febdd9aab9972bbb624d8c Mon Sep 17 00:00:00 2001 From: aparnajyothi-y <147696841+aparnajyothi-y@users.noreply.github.com> Date: Wed, 25 Jun 2025 10:10:44 +0530 Subject: [PATCH] Enhance cache-dependency-path handling to support files outside the workspace root (#1128) * ehnace cache dependency path handling * logic update * npm run format-check * update cacheDependencies tests to cover resolved paths and copy edge cases * check failure fix * depricate-windows-2019 * refactored the code * Check failure fix --- .github/workflows/test-pypy.yml | 1 - __tests__/setup-python.test.ts | 149 ++++++++++++++++++++++++++++++++ dist/setup/index.js | 43 ++++++++- docs/advanced-usage.md | 2 +- src/setup-python.ts | 53 +++++++++++- 5 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 __tests__/setup-python.test.ts diff --git a/.github/workflows/test-pypy.yml b/.github/workflows/test-pypy.yml index 54466e52..6433b7c5 100644 --- a/.github/workflows/test-pypy.yml +++ b/.github/workflows/test-pypy.yml @@ -88,7 +88,6 @@ jobs: - macos-13 - macos-14 - macos-15 - - windows-2019 - windows-2022 - windows-2025 - ubuntu-22.04 diff --git a/__tests__/setup-python.test.ts b/__tests__/setup-python.test.ts new file mode 100644 index 00000000..bb27289d --- /dev/null +++ b/__tests__/setup-python.test.ts @@ -0,0 +1,149 @@ +import * as core from '@actions/core'; +import * as fs from 'fs'; +import * as path from 'path'; +import {cacheDependencies} from '../src/setup-python'; +import {getCacheDistributor} from '../src/cache-distributions/cache-factory'; + +jest.mock('fs', () => { + const actualFs = jest.requireActual('fs'); + return { + ...actualFs, + promises: { + access: jest.fn(), + mkdir: jest.fn(), + copyFile: jest.fn(), + writeFile: jest.fn(), + appendFile: jest.fn() + } + }; +}); +jest.mock('@actions/core'); +jest.mock('../src/cache-distributions/cache-factory'); + +const mockedFsPromises = fs.promises as jest.Mocked; +const mockedCore = core as jest.Mocked; +const mockedGetCacheDistributor = getCacheDistributor as jest.Mock; + +describe('cacheDependencies', () => { + const mockRestoreCache = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + process.env.GITHUB_ACTION_PATH = '/github/action'; + process.env.GITHUB_WORKSPACE = '/github/workspace'; + + mockedCore.getInput.mockReturnValue('nested/deps.lock'); + + // Simulate file exists by resolving access without error + mockedFsPromises.access.mockImplementation(async p => { + const pathStr = typeof p === 'string' ? p : p.toString(); + if (pathStr === '/github/action/nested/deps.lock') { + return Promise.resolve(); + } + // Simulate directory doesn't exist to test mkdir + if (pathStr === path.dirname('/github/workspace/nested/deps.lock')) { + return Promise.reject(new Error('no dir')); + } + return Promise.resolve(); + }); + + // Simulate mkdir success + mockedFsPromises.mkdir.mockResolvedValue(undefined); + + // Simulate copyFile success + mockedFsPromises.copyFile.mockResolvedValue(undefined); + + mockedGetCacheDistributor.mockReturnValue({restoreCache: mockRestoreCache}); + }); + + it('copies the dependency file and resolves the path with directory structure', async () => { + await cacheDependencies('pip', '3.12'); + + const sourcePath = path.resolve('/github/action', 'nested/deps.lock'); + const targetPath = path.resolve('/github/workspace', 'nested/deps.lock'); + + expect(mockedFsPromises.access).toHaveBeenCalledWith( + sourcePath, + fs.constants.F_OK + ); + expect(mockedFsPromises.mkdir).toHaveBeenCalledWith( + path.dirname(targetPath), + { + recursive: true + } + ); + expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( + sourcePath, + targetPath + ); + expect(mockedCore.info).toHaveBeenCalledWith( + `Copied ${sourcePath} to ${targetPath}` + ); + expect(mockedCore.info).toHaveBeenCalledWith( + `Resolved cache-dependency-path: nested/deps.lock` + ); + expect(mockRestoreCache).toHaveBeenCalled(); + }); + + it('warns if the dependency file does not exist', async () => { + // Simulate file does not exist by rejecting access + mockedFsPromises.access.mockRejectedValue(new Error('file not found')); + + await cacheDependencies('pip', '3.12'); + + expect(mockedCore.warning).toHaveBeenCalledWith( + expect.stringContaining('does not exist') + ); + expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); + expect(mockRestoreCache).toHaveBeenCalled(); + }); + + it('warns if file copy fails', async () => { + // Simulate copyFile failure + mockedFsPromises.copyFile.mockRejectedValue(new Error('copy failed')); + + await cacheDependencies('pip', '3.12'); + + expect(mockedCore.warning).toHaveBeenCalledWith( + expect.stringContaining('Failed to copy file') + ); + expect(mockRestoreCache).toHaveBeenCalled(); + }); + + it('skips path logic if no input is provided', async () => { + mockedCore.getInput.mockReturnValue(''); + + await cacheDependencies('pip', '3.12'); + + expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); + expect(mockedCore.warning).not.toHaveBeenCalled(); + expect(mockRestoreCache).toHaveBeenCalled(); + }); + + it('does not copy if dependency file is already inside the workspace but still sets resolved path', async () => { + // Simulate cacheDependencyPath inside workspace + mockedCore.getInput.mockReturnValue('deps.lock'); + + // Override sourcePath and targetPath to be equal + const actionPath = '/github/workspace'; // same path for action and workspace + process.env.GITHUB_ACTION_PATH = actionPath; + process.env.GITHUB_WORKSPACE = actionPath; + + // access resolves to simulate file exists + mockedFsPromises.access.mockResolvedValue(); + + await cacheDependencies('pip', '3.12'); + + const sourcePath = path.resolve(actionPath, 'deps.lock'); + const targetPath = sourcePath; // same path + + expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); + expect(mockedCore.info).toHaveBeenCalledWith( + `Dependency file is already inside the workspace: ${sourcePath}` + ); + expect(mockedCore.info).toHaveBeenCalledWith( + `Resolved cache-dependency-path: deps.lock` + ); + expect(mockRestoreCache).toHaveBeenCalled(); + }); +}); diff --git a/dist/setup/index.js b/dist/setup/index.js index d8fae2a0..c2f220c0 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96844,6 +96844,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) { return (mod && mod.__esModule) ? mod : { "default": mod }; }; Object.defineProperty(exports, "__esModule", ({ value: true })); +exports.cacheDependencies = void 0; const core = __importStar(__nccwpck_require__(7484)); const finder = __importStar(__nccwpck_require__(6843)); const finderPyPy = __importStar(__nccwpck_require__(2625)); @@ -96862,10 +96863,50 @@ function isGraalPyVersion(versionSpec) { function cacheDependencies(cache, pythonVersion) { return __awaiter(this, void 0, void 0, function* () { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; - const cacheDistributor = (0, cache_factory_1.getCacheDistributor)(cache, pythonVersion, cacheDependencyPath); + let resolvedDependencyPath = undefined; + if (cacheDependencyPath) { + const actionPath = process.env.GITHUB_ACTION_PATH || ''; + const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); + const sourcePath = path.resolve(actionPath, cacheDependencyPath); + const relativePath = path.relative(actionPath, sourcePath); + const targetPath = path.resolve(workspace, relativePath); + try { + const sourceExists = yield fs_1.default.promises + .access(sourcePath, fs_1.default.constants.F_OK) + .then(() => true) + .catch(() => false); + if (!sourceExists) { + core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); + } + else { + if (sourcePath !== targetPath) { + const targetDir = path.dirname(targetPath); + // Create target directory if it doesn't exist + yield fs_1.default.promises.mkdir(targetDir, { recursive: true }); + // Copy file asynchronously + yield fs_1.default.promises.copyFile(sourcePath, targetPath); + core.info(`Copied ${sourcePath} to ${targetPath}`); + } + else { + core.info(`Dependency file is already inside the workspace: ${sourcePath}`); + } + resolvedDependencyPath = path + .relative(workspace, targetPath) + .replace(/\\/g, '/'); + core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); + } + } + catch (error) { + core.warning(`Failed to copy file from ${sourcePath} to ${targetPath}: ${error}`); + } + } + // Pass resolvedDependencyPath if available, else fallback to original input + const dependencyPathForCache = resolvedDependencyPath !== null && resolvedDependencyPath !== void 0 ? resolvedDependencyPath : cacheDependencyPath; + const cacheDistributor = (0, cache_factory_1.getCacheDistributor)(cache, pythonVersion, dependencyPathForCache); yield cacheDistributor.restoreCache(); }); } +exports.cacheDependencies = cacheDependencies; function resolveVersionInputFromDefaultFile() { const couples = [ ['.python-version', utils_1.getVersionsInputFromPlainFile] diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index 7a8f1187..96524823 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -412,7 +412,7 @@ steps: - run: pip install -e . # Or pip install -e '.[test]' to install test dependencies ``` - +Note: cache-dependency-path supports files located outside the workspace root by copying them into the workspace to enable proper caching. # Outputs and environment variables ## Outputs diff --git a/src/setup-python.ts b/src/setup-python.ts index 5d585d73..106b415a 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -22,13 +22,62 @@ function isGraalPyVersion(versionSpec: string) { return versionSpec.startsWith('graalpy'); } -async function cacheDependencies(cache: string, pythonVersion: string) { +export async function cacheDependencies(cache: string, pythonVersion: string) { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; + let resolvedDependencyPath: string | undefined = undefined; + + if (cacheDependencyPath) { + const actionPath = process.env.GITHUB_ACTION_PATH || ''; + const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); + + const sourcePath = path.resolve(actionPath, cacheDependencyPath); + const relativePath = path.relative(actionPath, sourcePath); + const targetPath = path.resolve(workspace, relativePath); + + try { + const sourceExists = await fs.promises + .access(sourcePath, fs.constants.F_OK) + .then(() => true) + .catch(() => false); + + if (!sourceExists) { + core.warning( + `The resolved cache-dependency-path does not exist: ${sourcePath}` + ); + } else { + if (sourcePath !== targetPath) { + const targetDir = path.dirname(targetPath); + // Create target directory if it doesn't exist + await fs.promises.mkdir(targetDir, {recursive: true}); + // Copy file asynchronously + await fs.promises.copyFile(sourcePath, targetPath); + core.info(`Copied ${sourcePath} to ${targetPath}`); + } else { + core.info( + `Dependency file is already inside the workspace: ${sourcePath}` + ); + } + + resolvedDependencyPath = path + .relative(workspace, targetPath) + .replace(/\\/g, '/'); + core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); + } + } catch (error) { + core.warning( + `Failed to copy file from ${sourcePath} to ${targetPath}: ${error}` + ); + } + } + + // Pass resolvedDependencyPath if available, else fallback to original input + const dependencyPathForCache = resolvedDependencyPath ?? cacheDependencyPath; + const cacheDistributor = getCacheDistributor( cache, pythonVersion, - cacheDependencyPath + dependencyPathForCache ); await cacheDistributor.restoreCache(); }