From 8626f8c1ae5ee1fd5a62cfd48c3271a2442c53f3 Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Fri, 4 Jul 2025 18:39:37 +0530 Subject: [PATCH 01/16] Add overwite input to avoid auto overwriting --- __tests__/setup-python.test.ts | 110 +++++++++++++++++---------------- action.yml | 8 ++- dist/setup/index.js | 17 +++-- src/setup-python.ts | 23 +++++-- 4 files changed, 92 insertions(+), 66 deletions(-) diff --git a/__tests__/setup-python.test.ts b/__tests__/setup-python.test.ts index bb27289de..318d9a97b 100644 --- a/__tests__/setup-python.test.ts +++ b/__tests__/setup-python.test.ts @@ -33,61 +33,72 @@ describe('cacheDependencies', () => { process.env.GITHUB_WORKSPACE = '/github/workspace'; mockedCore.getInput.mockReturnValue('nested/deps.lock'); + mockedCore.getBooleanInput.mockReturnValue(false); - // 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(); - }); + mockedGetCacheDistributor.mockReturnValue({restoreCache: mockRestoreCache}); - // 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 () => { + it('copies the file if source exists and target does not', async () => { + mockedFsPromises.access.mockImplementation(async filePath => { + if (filePath === '/github/action/nested/deps.lock') + return Promise.resolve(); // source + throw new Error('target does not exist'); // target + }); + await cacheDependencies('pip', '3.12'); - const sourcePath = path.resolve('/github/action', 'nested/deps.lock'); - const targetPath = path.resolve('/github/workspace', 'nested/deps.lock'); + const sourcePath = '/github/action/nested/deps.lock'; + const targetPath = '/github/workspace/nested/deps.lock'; - expect(mockedFsPromises.access).toHaveBeenCalledWith( + expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( sourcePath, - fs.constants.F_OK + targetPath ); - expect(mockedFsPromises.mkdir).toHaveBeenCalledWith( - path.dirname(targetPath), - { - recursive: true - } + expect(mockedCore.info).toHaveBeenCalledWith( + `Copied ${sourcePath} to ${targetPath}` ); + }); + + it('overwrites file if target exists and overwrite is true', async () => { + mockedCore.getBooleanInput.mockReturnValue(true); + mockedFsPromises.access.mockResolvedValue(); // both source and target exist + + await cacheDependencies('pip', '3.12'); + + const sourcePath = '/github/action/nested/deps.lock'; + const targetPath = '/github/workspace/nested/deps.lock'; + expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( sourcePath, targetPath ); expect(mockedCore.info).toHaveBeenCalledWith( - `Copied ${sourcePath} to ${targetPath}` + `Overwrote ${sourcePath} to ${targetPath}` ); + }); + + it('skips copy if file exists and overwrite is false', async () => { + mockedCore.getBooleanInput.mockReturnValue(false); + mockedFsPromises.access.mockResolvedValue(); // both source and target exist + + await cacheDependencies('pip', '3.12'); + + expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); expect(mockedCore.info).toHaveBeenCalledWith( - `Resolved cache-dependency-path: nested/deps.lock` + expect.stringContaining('Skipped copying') ); - 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')); + it('logs warning if source file does not exist', async () => { + mockedFsPromises.access.mockImplementation(async filePath => { + if (filePath === '/github/action/nested/deps.lock') { + throw new Error('source not found'); + } + return Promise.resolve(); // fallback for others + }); await cacheDependencies('pip', '3.12'); @@ -95,11 +106,15 @@ describe('cacheDependencies', () => { expect.stringContaining('does not exist') ); expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); - expect(mockRestoreCache).toHaveBeenCalled(); }); - it('warns if file copy fails', async () => { - // Simulate copyFile failure + it('logs warning if copyFile fails', async () => { + mockedFsPromises.access.mockImplementation(async filePath => { + if (filePath === '/github/action/nested/deps.lock') + return Promise.resolve(); + throw new Error('target does not exist'); + }); + mockedFsPromises.copyFile.mockRejectedValue(new Error('copy failed')); await cacheDependencies('pip', '3.12'); @@ -107,43 +122,30 @@ describe('cacheDependencies', () => { expect(mockedCore.warning).toHaveBeenCalledWith( expect.stringContaining('Failed to copy file') ); - expect(mockRestoreCache).toHaveBeenCalled(); }); - it('skips path logic if no input is provided', async () => { + it('skips everything if cache-dependency-path is not 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 + it('does not copy if source and target are the same path', async () => { mockedCore.getInput.mockReturnValue('deps.lock'); + process.env.GITHUB_ACTION_PATH = '/github/workspace'; + process.env.GITHUB_WORKSPACE = '/github/workspace'; - // 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 - + const sourcePath = '/github/workspace/deps.lock'; 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/action.yml b/action.yml index e469b7b2b..e870e8dc5 100644 --- a/action.yml +++ b/action.yml @@ -20,6 +20,10 @@ inputs: default: ${{ github.server_url == 'https://github.com' && github.token || '' }} cache-dependency-path: description: "Used to specify the path to dependency files. Supports wildcards or a list of file names for caching multiple dependencies." + overwrite: + description: Whether to overwrite existing file in workspace + required: false + default: 'false' update-environment: description: "Set this option if you want the action to update environment variables." default: true @@ -29,8 +33,6 @@ inputs: freethreaded: description: "When 'true', use the freethreaded version of Python." default: false - pip-version: - description: "Used to specify the version of pip to install with the Python. Supported format: major[.minor][.patch]." outputs: python-version: description: "The installed Python or PyPy version. Useful when given a version range as input." @@ -45,4 +47,4 @@ runs: post-if: success() branding: icon: 'code' - color: 'yellow' + color: 'yellow' \ No newline at end of file diff --git a/dist/setup/index.js b/dist/setup/index.js index f8c5d4e72..7852eee38 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96883,8 +96883,10 @@ function isGraalPyVersion(versionSpec) { } function cacheDependencies(cache, pythonVersion) { return __awaiter(this, void 0, void 0, function* () { + var _a; const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; let resolvedDependencyPath = undefined; + const overwrite = (_a = core.getBooleanInput('overwrite', { required: false })) !== null && _a !== void 0 ? _a : false; if (cacheDependencyPath) { const actionPath = process.env.GITHUB_ACTION_PATH || ''; const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); @@ -96902,11 +96904,18 @@ function cacheDependencies(cache, pythonVersion) { 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}`); + const targetExists = yield fs_1.default.promises + .access(targetPath, fs_1.default.constants.F_OK) + .then(() => true) + .catch(() => false); + if (!targetExists || overwrite) { + yield fs_1.default.promises.copyFile(sourcePath, targetPath); + core.info(`${targetExists ? 'Overwrote' : 'Copied'} ${sourcePath} to ${targetPath}`); + } + else { + core.info(`Skipped copying ${sourcePath} — target already exists at ${targetPath}`); + } } else { core.info(`Dependency file is already inside the workspace: ${sourcePath}`); diff --git a/src/setup-python.ts b/src/setup-python.ts index 106b415a6..ea77897a1 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -26,7 +26,8 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; let resolvedDependencyPath: string | undefined = undefined; - + const overwrite = + core.getBooleanInput('overwrite', {required: false}) ?? false; if (cacheDependencyPath) { const actionPath = process.env.GITHUB_ACTION_PATH || ''; const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); @@ -48,11 +49,23 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { } 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}`); + + const targetExists = await fs.promises + .access(targetPath, fs.constants.F_OK) + .then(() => true) + .catch(() => false); + + if (!targetExists || overwrite) { + await fs.promises.copyFile(sourcePath, targetPath); + core.info( + `${targetExists ? 'Overwrote' : 'Copied'} ${sourcePath} to ${targetPath}` + ); + } else { + core.info( + `Skipped copying ${sourcePath} — target already exists at ${targetPath}` + ); + } } else { core.info( `Dependency file is already inside the workspace: ${sourcePath}` From a8ee5f78aab23c78f8944ddfc653af00bedb02ab Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Fri, 4 Jul 2025 18:47:33 +0530 Subject: [PATCH 02/16] documentation update --- docs/advanced-usage.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index 96524823e..a901ed581 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -413,6 +413,7 @@ steps: # 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. +To avoid unintentionally overwriting existing files in the workspace (especially when using composite actions with common file names like requirements.txt), a new input overwrite has been added. By default, files will not be copied if a file with the same path already exists in the workspace unless overwrite: true is explicitly set. # Outputs and environment variables ## Outputs From 762137b466994032eb546052d56fc7b8153f6ea8 Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 16:12:27 +0530 Subject: [PATCH 03/16] updated logic --- __tests__/setup-python.test.ts | 83 ++++++---------------------------- dist/setup/index.js | 36 ++++----------- src/setup-python.ts | 50 ++++++-------------- 3 files changed, 37 insertions(+), 132 deletions(-) diff --git a/__tests__/setup-python.test.ts b/__tests__/setup-python.test.ts index 318d9a97b..b010ed522 100644 --- a/__tests__/setup-python.test.ts +++ b/__tests__/setup-python.test.ts @@ -1,6 +1,7 @@ import * as core from '@actions/core'; import * as fs from 'fs'; import * as path from 'path'; +import * as os from 'os'; import {cacheDependencies} from '../src/setup-python'; import {getCacheDistributor} from '../src/cache-distributions/cache-factory'; @@ -11,6 +12,7 @@ jest.mock('fs', () => { promises: { access: jest.fn(), mkdir: jest.fn(), + mkdtemp: jest.fn(), copyFile: jest.fn(), writeFile: jest.fn(), appendFile: jest.fn() @@ -33,72 +35,38 @@ describe('cacheDependencies', () => { process.env.GITHUB_WORKSPACE = '/github/workspace'; mockedCore.getInput.mockReturnValue('nested/deps.lock'); - mockedCore.getBooleanInput.mockReturnValue(false); mockedGetCacheDistributor.mockReturnValue({restoreCache: mockRestoreCache}); - - mockedFsPromises.mkdir.mockResolvedValue(undefined); mockedFsPromises.copyFile.mockResolvedValue(undefined); + mockedFsPromises.mkdtemp.mockResolvedValue( + '/tmp/setup-python-cache-abc123' + ); }); - it('copies the file if source exists and target does not', async () => { + it('copies the file to a temp directory if source exists', async () => { mockedFsPromises.access.mockImplementation(async filePath => { - if (filePath === '/github/action/nested/deps.lock') - return Promise.resolve(); // source - throw new Error('target does not exist'); // target + if (filePath === '/github/action/nested/deps.lock') { + return Promise.resolve(); + } + throw new Error('not found'); }); await cacheDependencies('pip', '3.12'); const sourcePath = '/github/action/nested/deps.lock'; - const targetPath = '/github/workspace/nested/deps.lock'; + const expectedTarget = '/tmp/setup-python-cache-abc123/deps.lock'; expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( sourcePath, - targetPath + expectedTarget ); expect(mockedCore.info).toHaveBeenCalledWith( - `Copied ${sourcePath} to ${targetPath}` - ); - }); - - it('overwrites file if target exists and overwrite is true', async () => { - mockedCore.getBooleanInput.mockReturnValue(true); - mockedFsPromises.access.mockResolvedValue(); // both source and target exist - - await cacheDependencies('pip', '3.12'); - - const sourcePath = '/github/action/nested/deps.lock'; - const targetPath = '/github/workspace/nested/deps.lock'; - - expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( - sourcePath, - targetPath - ); - expect(mockedCore.info).toHaveBeenCalledWith( - `Overwrote ${sourcePath} to ${targetPath}` - ); - }); - - it('skips copy if file exists and overwrite is false', async () => { - mockedCore.getBooleanInput.mockReturnValue(false); - mockedFsPromises.access.mockResolvedValue(); // both source and target exist - - await cacheDependencies('pip', '3.12'); - - expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); - expect(mockedCore.info).toHaveBeenCalledWith( - expect.stringContaining('Skipped copying') + `Copied ${sourcePath} to isolated temp location: ${expectedTarget}` ); }); it('logs warning if source file does not exist', async () => { - mockedFsPromises.access.mockImplementation(async filePath => { - if (filePath === '/github/action/nested/deps.lock') { - throw new Error('source not found'); - } - return Promise.resolve(); // fallback for others - }); + mockedFsPromises.access.mockRejectedValue(new Error('not found')); await cacheDependencies('pip', '3.12'); @@ -109,12 +77,7 @@ describe('cacheDependencies', () => { }); it('logs warning if copyFile fails', async () => { - mockedFsPromises.access.mockImplementation(async filePath => { - if (filePath === '/github/action/nested/deps.lock') - return Promise.resolve(); - throw new Error('target does not exist'); - }); - + mockedFsPromises.access.mockResolvedValue(); mockedFsPromises.copyFile.mockRejectedValue(new Error('copy failed')); await cacheDependencies('pip', '3.12'); @@ -132,20 +95,4 @@ describe('cacheDependencies', () => { expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); expect(mockedCore.warning).not.toHaveBeenCalled(); }); - - it('does not copy if source and target are the same path', async () => { - mockedCore.getInput.mockReturnValue('deps.lock'); - process.env.GITHUB_ACTION_PATH = '/github/workspace'; - process.env.GITHUB_WORKSPACE = '/github/workspace'; - - mockedFsPromises.access.mockResolvedValue(); - - await cacheDependencies('pip', '3.12'); - - const sourcePath = '/github/workspace/deps.lock'; - expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); - expect(mockedCore.info).toHaveBeenCalledWith( - `Dependency file is already inside the workspace: ${sourcePath}` - ); - }); }); diff --git a/dist/setup/index.js b/dist/setup/index.js index 7852eee38..0daf64690 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96883,16 +96883,12 @@ function isGraalPyVersion(versionSpec) { } function cacheDependencies(cache, pythonVersion) { return __awaiter(this, void 0, void 0, function* () { - var _a; const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; let resolvedDependencyPath = undefined; - const overwrite = (_a = core.getBooleanInput('overwrite', { required: false })) !== null && _a !== void 0 ? _a : false; if (cacheDependencyPath) { - const actionPath = process.env.GITHUB_ACTION_PATH || ''; const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); + const actionPath = process.env.GITHUB_ACTION_PATH || workspace; 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) @@ -96902,35 +96898,19 @@ function cacheDependencies(cache, pythonVersion) { core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - if (sourcePath !== targetPath) { - const targetDir = path.dirname(targetPath); - yield fs_1.default.promises.mkdir(targetDir, { recursive: true }); - const targetExists = yield fs_1.default.promises - .access(targetPath, fs_1.default.constants.F_OK) - .then(() => true) - .catch(() => false); - if (!targetExists || overwrite) { - yield fs_1.default.promises.copyFile(sourcePath, targetPath); - core.info(`${targetExists ? 'Overwrote' : 'Copied'} ${sourcePath} to ${targetPath}`); - } - else { - core.info(`Skipped copying ${sourcePath} — target already exists at ${targetPath}`); - } - } - else { - core.info(`Dependency file is already inside the workspace: ${sourcePath}`); - } - resolvedDependencyPath = path - .relative(workspace, targetPath) - .replace(/\\/g, '/'); + // Create a unique temp directory to avoid polluting the workspace + const tempDir = yield fs_1.default.promises.mkdtemp(path.join(os.tmpdir(), 'setup-python-cache-')); + const targetPath = path.join(tempDir, path.basename(sourcePath)); + yield fs_1.default.promises.copyFile(sourcePath, targetPath); + core.info(`Copied ${sourcePath} to isolated temp location: ${targetPath}`); + resolvedDependencyPath = targetPath; core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); } } catch (error) { - core.warning(`Failed to copy file from ${sourcePath} to ${targetPath}: ${error}`); + core.warning(`Failed to copy file from ${sourcePath} to temporary location: ${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(); diff --git a/src/setup-python.ts b/src/setup-python.ts index ea77897a1..d0a15cf9a 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -26,15 +26,11 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; let resolvedDependencyPath: string | undefined = undefined; - const overwrite = - core.getBooleanInput('overwrite', {required: false}) ?? false; + if (cacheDependencyPath) { - const actionPath = process.env.GITHUB_ACTION_PATH || ''; const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); - + const actionPath = process.env.GITHUB_ACTION_PATH || workspace; const sourcePath = path.resolve(actionPath, cacheDependencyPath); - const relativePath = path.relative(actionPath, sourcePath); - const targetPath = path.resolve(workspace, relativePath); try { const sourceExists = await fs.promises @@ -47,44 +43,27 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { `The resolved cache-dependency-path does not exist: ${sourcePath}` ); } else { - if (sourcePath !== targetPath) { - const targetDir = path.dirname(targetPath); - await fs.promises.mkdir(targetDir, {recursive: true}); - - const targetExists = await fs.promises - .access(targetPath, fs.constants.F_OK) - .then(() => true) - .catch(() => false); - - if (!targetExists || overwrite) { - await fs.promises.copyFile(sourcePath, targetPath); - core.info( - `${targetExists ? 'Overwrote' : 'Copied'} ${sourcePath} to ${targetPath}` - ); - } else { - core.info( - `Skipped copying ${sourcePath} — target already exists at ${targetPath}` - ); - } - } else { - core.info( - `Dependency file is already inside the workspace: ${sourcePath}` - ); - } + // Create a unique temp directory to avoid polluting the workspace + const tempDir = await fs.promises.mkdtemp( + path.join(os.tmpdir(), 'setup-python-cache-') + ); + const targetPath = path.join(tempDir, path.basename(sourcePath)); - resolvedDependencyPath = path - .relative(workspace, targetPath) - .replace(/\\/g, '/'); + await fs.promises.copyFile(sourcePath, targetPath); + core.info( + `Copied ${sourcePath} to isolated temp location: ${targetPath}` + ); + + resolvedDependencyPath = targetPath; core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); } } catch (error) { core.warning( - `Failed to copy file from ${sourcePath} to ${targetPath}: ${error}` + `Failed to copy file from ${sourcePath} to temporary location: ${error}` ); } } - // Pass resolvedDependencyPath if available, else fallback to original input const dependencyPathForCache = resolvedDependencyPath ?? cacheDependencyPath; const cacheDistributor = getCacheDistributor( @@ -94,7 +73,6 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { ); await cacheDistributor.restoreCache(); } - function resolveVersionInputFromDefaultFile(): string[] { const couples: [string, (versionFile: string) => string[]][] = [ ['.python-version', getVersionsInputFromPlainFile] From 90b2b96f70f865868144b15ee2c240b1f0e7fe87 Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 16:13:24 +0530 Subject: [PATCH 04/16] updated ovewrite input --- action.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/action.yml b/action.yml index e870e8dc5..1ea9def02 100644 --- a/action.yml +++ b/action.yml @@ -20,10 +20,6 @@ inputs: default: ${{ github.server_url == 'https://github.com' && github.token || '' }} cache-dependency-path: description: "Used to specify the path to dependency files. Supports wildcards or a list of file names for caching multiple dependencies." - overwrite: - description: Whether to overwrite existing file in workspace - required: false - default: 'false' update-environment: description: "Set this option if you want the action to update environment variables." default: true From f545f6b31999d6408bd51d00714c6e71c33e010b Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 16:27:24 +0530 Subject: [PATCH 05/16] updated logic --- dist/setup/index.js | 9 ++++++--- src/setup-python.ts | 12 ++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index 0daf64690..6e5708179 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96898,12 +96898,15 @@ function cacheDependencies(cache, pythonVersion) { core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - // Create a unique temp directory to avoid polluting the workspace - const tempDir = yield fs_1.default.promises.mkdtemp(path.join(os.tmpdir(), 'setup-python-cache-')); + // ✅ Create isolated temp dir inside workspace to stay compatible with cache matchers + const tempDir = yield fs_1.default.promises.mkdtemp(path.join(workspace, '.tmp-setup-python-')); const targetPath = path.join(tempDir, path.basename(sourcePath)); yield fs_1.default.promises.copyFile(sourcePath, targetPath); core.info(`Copied ${sourcePath} to isolated temp location: ${targetPath}`); - resolvedDependencyPath = targetPath; + // Use relative path from workspace so cache tools can pick it up + resolvedDependencyPath = path + .relative(workspace, targetPath) + .replace(/\\/g, '/'); core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); } } diff --git a/src/setup-python.ts b/src/setup-python.ts index d0a15cf9a..5bf37c139 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -21,7 +21,6 @@ function isPyPyVersion(versionSpec: string) { function isGraalPyVersion(versionSpec: string) { return versionSpec.startsWith('graalpy'); } - export async function cacheDependencies(cache: string, pythonVersion: string) { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; @@ -43,9 +42,9 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { `The resolved cache-dependency-path does not exist: ${sourcePath}` ); } else { - // Create a unique temp directory to avoid polluting the workspace + // ✅ Create isolated temp dir inside workspace to stay compatible with cache matchers const tempDir = await fs.promises.mkdtemp( - path.join(os.tmpdir(), 'setup-python-cache-') + path.join(workspace, '.tmp-setup-python-') ); const targetPath = path.join(tempDir, path.basename(sourcePath)); @@ -54,7 +53,11 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { `Copied ${sourcePath} to isolated temp location: ${targetPath}` ); - resolvedDependencyPath = targetPath; + // Use relative path from workspace so cache tools can pick it up + resolvedDependencyPath = path + .relative(workspace, targetPath) + .replace(/\\/g, '/'); + core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); } } catch (error) { @@ -73,6 +76,7 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { ); await cacheDistributor.restoreCache(); } + function resolveVersionInputFromDefaultFile(): string[] { const couples: [string, (versionFile: string) => string[]][] = [ ['.python-version', getVersionsInputFromPlainFile] From 28a1ebcd9ceba052cb676419580f026f8d28c67f Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 16:39:26 +0530 Subject: [PATCH 06/16] logic pdate --- dist/setup/index.js | 15 ++++++++------- src/setup-python.ts | 22 ++++++++++------------ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index 6e5708179..7695f7106 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96887,7 +96887,7 @@ function cacheDependencies(cache, pythonVersion) { let resolvedDependencyPath = undefined; if (cacheDependencyPath) { const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); - const actionPath = process.env.GITHUB_ACTION_PATH || workspace; + const actionPath = path.resolve(__dirname, '..'); // Reliable in both JS/composite const sourcePath = path.resolve(actionPath, cacheDependencyPath); try { const sourceExists = yield fs_1.default.promises @@ -96898,12 +96898,13 @@ function cacheDependencies(cache, pythonVersion) { core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - // ✅ Create isolated temp dir inside workspace to stay compatible with cache matchers - const tempDir = yield fs_1.default.promises.mkdtemp(path.join(workspace, '.tmp-setup-python-')); - const targetPath = path.join(tempDir, path.basename(sourcePath)); + // ✅ Place inside workspace, but in a non-conflicting subfolder + const targetDir = path.join(workspace, '.setup-python-cache'); + yield fs_1.default.promises.mkdir(targetDir, { recursive: true }); + const targetPath = path.join(targetDir, path.basename(sourcePath)); yield fs_1.default.promises.copyFile(sourcePath, targetPath); - core.info(`Copied ${sourcePath} to isolated temp location: ${targetPath}`); - // Use relative path from workspace so cache tools can pick it up + core.info(`Copied ${sourcePath} to ${targetPath}`); + // ✅ Return relative path for caching resolvedDependencyPath = path .relative(workspace, targetPath) .replace(/\\/g, '/'); @@ -96911,7 +96912,7 @@ function cacheDependencies(cache, pythonVersion) { } } catch (error) { - core.warning(`Failed to copy file from ${sourcePath} to temporary location: ${error}`); + core.warning(`Failed to copy file from ${sourcePath} to target: ${error}`); } } const dependencyPathForCache = resolvedDependencyPath !== null && resolvedDependencyPath !== void 0 ? resolvedDependencyPath : cacheDependencyPath; diff --git a/src/setup-python.ts b/src/setup-python.ts index 5bf37c139..f21e7a6df 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -21,6 +21,7 @@ function isPyPyVersion(versionSpec: string) { function isGraalPyVersion(versionSpec: string) { return versionSpec.startsWith('graalpy'); } + export async function cacheDependencies(cache: string, pythonVersion: string) { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; @@ -28,7 +29,7 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { if (cacheDependencyPath) { const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); - const actionPath = process.env.GITHUB_ACTION_PATH || workspace; + const actionPath = path.resolve(__dirname, '..'); // Reliable in both JS/composite const sourcePath = path.resolve(actionPath, cacheDependencyPath); try { @@ -42,18 +43,16 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { `The resolved cache-dependency-path does not exist: ${sourcePath}` ); } else { - // ✅ Create isolated temp dir inside workspace to stay compatible with cache matchers - const tempDir = await fs.promises.mkdtemp( - path.join(workspace, '.tmp-setup-python-') - ); - const targetPath = path.join(tempDir, path.basename(sourcePath)); + // ✅ Place inside workspace, but in a non-conflicting subfolder + const targetDir = path.join(workspace, '.setup-python-cache'); + await fs.promises.mkdir(targetDir, {recursive: true}); + const targetPath = path.join(targetDir, path.basename(sourcePath)); await fs.promises.copyFile(sourcePath, targetPath); - core.info( - `Copied ${sourcePath} to isolated temp location: ${targetPath}` - ); - // Use relative path from workspace so cache tools can pick it up + core.info(`Copied ${sourcePath} to ${targetPath}`); + + // ✅ Return relative path for caching resolvedDependencyPath = path .relative(workspace, targetPath) .replace(/\\/g, '/'); @@ -62,7 +61,7 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { } } catch (error) { core.warning( - `Failed to copy file from ${sourcePath} to temporary location: ${error}` + `Failed to copy file from ${sourcePath} to target: ${error}` ); } } @@ -76,7 +75,6 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { ); await cacheDistributor.restoreCache(); } - function resolveVersionInputFromDefaultFile(): string[] { const couples: [string, (versionFile: string) => string[]][] = [ ['.python-version', getVersionsInputFromPlainFile] From b2eee9bdacbaed9bb0d3e720d16e7511f55a8f9f Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 16:49:44 +0530 Subject: [PATCH 07/16] updated logic --- dist/setup/index.js | 18 +++++++----------- src/setup-python.ts | 30 ++++++++++-------------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index 7695f7106..d3fe99762 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96886,8 +96886,8 @@ function cacheDependencies(cache, pythonVersion) { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; let resolvedDependencyPath = undefined; if (cacheDependencyPath) { + const actionPath = process.env.GITHUB_ACTION_PATH || ''; const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); - const actionPath = path.resolve(__dirname, '..'); // Reliable in both JS/composite const sourcePath = path.resolve(actionPath, cacheDependencyPath); try { const sourceExists = yield fs_1.default.promises @@ -96898,21 +96898,17 @@ function cacheDependencies(cache, pythonVersion) { core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - // ✅ Place inside workspace, but in a non-conflicting subfolder - const targetDir = path.join(workspace, '.setup-python-cache'); - yield fs_1.default.promises.mkdir(targetDir, { recursive: true }); - const targetPath = path.join(targetDir, path.basename(sourcePath)); + const safeFolder = path.join(workspace, '.setup-python-deps'); + yield fs_1.default.promises.mkdir(safeFolder, { recursive: true }); + const targetPath = path.join(safeFolder, path.basename(sourcePath)); yield fs_1.default.promises.copyFile(sourcePath, targetPath); - core.info(`Copied ${sourcePath} to ${targetPath}`); - // ✅ Return relative path for caching - resolvedDependencyPath = path - .relative(workspace, targetPath) - .replace(/\\/g, '/'); + core.info(`Copied ${sourcePath} to safe location: ${targetPath}`); + 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 target: ${error}`); + core.warning(`Failed to copy file from ${sourcePath} to safe location: ${error}`); } } const dependencyPathForCache = resolvedDependencyPath !== null && resolvedDependencyPath !== void 0 ? resolvedDependencyPath : cacheDependencyPath; diff --git a/src/setup-python.ts b/src/setup-python.ts index f21e7a6df..ecb660cf5 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -23,13 +23,13 @@ function isGraalPyVersion(versionSpec: string) { } export async function cacheDependencies(cache: string, pythonVersion: string) { - const cacheDependencyPath = - core.getInput('cache-dependency-path') || undefined; + 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 actionPath = path.resolve(__dirname, '..'); // Reliable in both JS/composite + const sourcePath = path.resolve(actionPath, cacheDependencyPath); try { @@ -39,30 +39,20 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { .catch(() => false); if (!sourceExists) { - core.warning( - `The resolved cache-dependency-path does not exist: ${sourcePath}` - ); + core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - // ✅ Place inside workspace, but in a non-conflicting subfolder - const targetDir = path.join(workspace, '.setup-python-cache'); - await fs.promises.mkdir(targetDir, {recursive: true}); + const safeFolder = path.join(workspace, '.setup-python-deps'); + await fs.promises.mkdir(safeFolder, { recursive: true }); - const targetPath = path.join(targetDir, path.basename(sourcePath)); + const targetPath = path.join(safeFolder, path.basename(sourcePath)); await fs.promises.copyFile(sourcePath, targetPath); + core.info(`Copied ${sourcePath} to safe location: ${targetPath}`); - core.info(`Copied ${sourcePath} to ${targetPath}`); - - // ✅ Return relative path for caching - resolvedDependencyPath = path - .relative(workspace, targetPath) - .replace(/\\/g, '/'); - + 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 target: ${error}` - ); + core.warning(`Failed to copy file from ${sourcePath} to safe location: ${error}`); } } From 8fc5f6f497f8c0d959a6099fdd6957a0f75fab9c Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 16:55:32 +0530 Subject: [PATCH 08/16] updated logic --- dist/setup/index.js | 28 +++++++++++++++++----------- src/setup-python.ts | 30 ++++++++++++++++++------------ 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index d3fe99762..c752812f5 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96884,11 +96884,13 @@ function isGraalPyVersion(versionSpec) { function cacheDependencies(cache, pythonVersion) { return __awaiter(this, void 0, void 0, function* () { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; - let resolvedDependencyPath = undefined; + let resolvedDependencyPath; if (cacheDependencyPath) { - const actionPath = process.env.GITHUB_ACTION_PATH || ''; + const actionPath = process.env.GITHUB_ACTION_PATH || path.resolve(__dirname, '..'); const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); const sourcePath = path.resolve(actionPath, cacheDependencyPath); + const safeFolder = path.join(workspace, '.setup-python-deps'); + const safeTargetPath = path.join(safeFolder, path.basename(sourcePath)); try { const sourceExists = yield fs_1.default.promises .access(sourcePath, fs_1.default.constants.F_OK) @@ -96898,21 +96900,25 @@ function cacheDependencies(cache, pythonVersion) { core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - const safeFolder = path.join(workspace, '.setup-python-deps'); yield fs_1.default.promises.mkdir(safeFolder, { recursive: true }); - const targetPath = path.join(safeFolder, path.basename(sourcePath)); - yield fs_1.default.promises.copyFile(sourcePath, targetPath); - core.info(`Copied ${sourcePath} to safe location: ${targetPath}`); - resolvedDependencyPath = path.relative(workspace, targetPath).replace(/\\/g, '/'); + yield fs_1.default.promises.copyFile(sourcePath, safeTargetPath); + core.info(`Copied ${sourcePath} to safe location: ${safeTargetPath}`); + resolvedDependencyPath = path + .relative(workspace, safeTargetPath) + .replace(/\\/g, '/'); core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); } } - catch (error) { - core.warning(`Failed to copy file from ${sourcePath} to safe location: ${error}`); + catch (err) { + core.warning(`Failed to copy dependency file: ${err}`); } } - const dependencyPathForCache = resolvedDependencyPath !== null && resolvedDependencyPath !== void 0 ? resolvedDependencyPath : cacheDependencyPath; - const cacheDistributor = (0, cache_factory_1.getCacheDistributor)(cache, pythonVersion, dependencyPathForCache); + const finalDependencyPath = resolvedDependencyPath; + if (!finalDependencyPath) { + core.setFailed('Failed to resolve dependency path from action.'); + return; + } + const cacheDistributor = (0, cache_factory_1.getCacheDistributor)(cache, pythonVersion, finalDependencyPath); yield cacheDistributor.restoreCache(); }); } diff --git a/src/setup-python.ts b/src/setup-python.ts index ecb660cf5..e8872802e 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -24,14 +24,16 @@ function isGraalPyVersion(versionSpec: string) { export async function cacheDependencies(cache: string, pythonVersion: string) { const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; - let resolvedDependencyPath: string | undefined = undefined; + let resolvedDependencyPath: string | undefined; if (cacheDependencyPath) { - const actionPath = process.env.GITHUB_ACTION_PATH || ''; + const actionPath = process.env.GITHUB_ACTION_PATH || path.resolve(__dirname, '..'); const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); - const sourcePath = path.resolve(actionPath, cacheDependencyPath); + const safeFolder = path.join(workspace, '.setup-python-deps'); + const safeTargetPath = path.join(safeFolder, path.basename(sourcePath)); + try { const sourceExists = await fs.promises .access(sourcePath, fs.constants.F_OK) @@ -41,27 +43,31 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { if (!sourceExists) { core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - const safeFolder = path.join(workspace, '.setup-python-deps'); await fs.promises.mkdir(safeFolder, { recursive: true }); + await fs.promises.copyFile(sourcePath, safeTargetPath); + core.info(`Copied ${sourcePath} to safe location: ${safeTargetPath}`); - const targetPath = path.join(safeFolder, path.basename(sourcePath)); - await fs.promises.copyFile(sourcePath, targetPath); - core.info(`Copied ${sourcePath} to safe location: ${targetPath}`); + resolvedDependencyPath = path + .relative(workspace, safeTargetPath) + .replace(/\\/g, '/'); - 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 safe location: ${error}`); + } catch (err) { + core.warning(`Failed to copy dependency file: ${err}`); } } - const dependencyPathForCache = resolvedDependencyPath ?? cacheDependencyPath; + const finalDependencyPath = resolvedDependencyPath; + if (!finalDependencyPath) { + core.setFailed('Failed to resolve dependency path from action.'); + return; + } const cacheDistributor = getCacheDistributor( cache, pythonVersion, - dependencyPathForCache + finalDependencyPath ); await cacheDistributor.restoreCache(); } From 3380c81707b274dbfdc5d4b95012fcc02021ca4c Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 17:05:15 +0530 Subject: [PATCH 09/16] logic update --- __tests__/setup-python.test.ts | 83 ++++++++++++++++++++++++++++------ action.yml | 4 ++ dist/setup/index.js | 49 +++++++++++++------- src/setup-python.ts | 70 +++++++++++++++++++--------- 4 files changed, 154 insertions(+), 52 deletions(-) diff --git a/__tests__/setup-python.test.ts b/__tests__/setup-python.test.ts index b010ed522..318d9a97b 100644 --- a/__tests__/setup-python.test.ts +++ b/__tests__/setup-python.test.ts @@ -1,7 +1,6 @@ import * as core from '@actions/core'; import * as fs from 'fs'; import * as path from 'path'; -import * as os from 'os'; import {cacheDependencies} from '../src/setup-python'; import {getCacheDistributor} from '../src/cache-distributions/cache-factory'; @@ -12,7 +11,6 @@ jest.mock('fs', () => { promises: { access: jest.fn(), mkdir: jest.fn(), - mkdtemp: jest.fn(), copyFile: jest.fn(), writeFile: jest.fn(), appendFile: jest.fn() @@ -35,38 +33,72 @@ describe('cacheDependencies', () => { process.env.GITHUB_WORKSPACE = '/github/workspace'; mockedCore.getInput.mockReturnValue('nested/deps.lock'); + mockedCore.getBooleanInput.mockReturnValue(false); mockedGetCacheDistributor.mockReturnValue({restoreCache: mockRestoreCache}); + + mockedFsPromises.mkdir.mockResolvedValue(undefined); mockedFsPromises.copyFile.mockResolvedValue(undefined); - mockedFsPromises.mkdtemp.mockResolvedValue( - '/tmp/setup-python-cache-abc123' - ); }); - it('copies the file to a temp directory if source exists', async () => { + it('copies the file if source exists and target does not', async () => { mockedFsPromises.access.mockImplementation(async filePath => { - if (filePath === '/github/action/nested/deps.lock') { - return Promise.resolve(); - } - throw new Error('not found'); + if (filePath === '/github/action/nested/deps.lock') + return Promise.resolve(); // source + throw new Error('target does not exist'); // target }); await cacheDependencies('pip', '3.12'); const sourcePath = '/github/action/nested/deps.lock'; - const expectedTarget = '/tmp/setup-python-cache-abc123/deps.lock'; + const targetPath = '/github/workspace/nested/deps.lock'; expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( sourcePath, - expectedTarget + targetPath ); expect(mockedCore.info).toHaveBeenCalledWith( - `Copied ${sourcePath} to isolated temp location: ${expectedTarget}` + `Copied ${sourcePath} to ${targetPath}` + ); + }); + + it('overwrites file if target exists and overwrite is true', async () => { + mockedCore.getBooleanInput.mockReturnValue(true); + mockedFsPromises.access.mockResolvedValue(); // both source and target exist + + await cacheDependencies('pip', '3.12'); + + const sourcePath = '/github/action/nested/deps.lock'; + const targetPath = '/github/workspace/nested/deps.lock'; + + expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( + sourcePath, + targetPath + ); + expect(mockedCore.info).toHaveBeenCalledWith( + `Overwrote ${sourcePath} to ${targetPath}` + ); + }); + + it('skips copy if file exists and overwrite is false', async () => { + mockedCore.getBooleanInput.mockReturnValue(false); + mockedFsPromises.access.mockResolvedValue(); // both source and target exist + + await cacheDependencies('pip', '3.12'); + + expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); + expect(mockedCore.info).toHaveBeenCalledWith( + expect.stringContaining('Skipped copying') ); }); it('logs warning if source file does not exist', async () => { - mockedFsPromises.access.mockRejectedValue(new Error('not found')); + mockedFsPromises.access.mockImplementation(async filePath => { + if (filePath === '/github/action/nested/deps.lock') { + throw new Error('source not found'); + } + return Promise.resolve(); // fallback for others + }); await cacheDependencies('pip', '3.12'); @@ -77,7 +109,12 @@ describe('cacheDependencies', () => { }); it('logs warning if copyFile fails', async () => { - mockedFsPromises.access.mockResolvedValue(); + mockedFsPromises.access.mockImplementation(async filePath => { + if (filePath === '/github/action/nested/deps.lock') + return Promise.resolve(); + throw new Error('target does not exist'); + }); + mockedFsPromises.copyFile.mockRejectedValue(new Error('copy failed')); await cacheDependencies('pip', '3.12'); @@ -95,4 +132,20 @@ describe('cacheDependencies', () => { expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); expect(mockedCore.warning).not.toHaveBeenCalled(); }); + + it('does not copy if source and target are the same path', async () => { + mockedCore.getInput.mockReturnValue('deps.lock'); + process.env.GITHUB_ACTION_PATH = '/github/workspace'; + process.env.GITHUB_WORKSPACE = '/github/workspace'; + + mockedFsPromises.access.mockResolvedValue(); + + await cacheDependencies('pip', '3.12'); + + const sourcePath = '/github/workspace/deps.lock'; + expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); + expect(mockedCore.info).toHaveBeenCalledWith( + `Dependency file is already inside the workspace: ${sourcePath}` + ); + }); }); diff --git a/action.yml b/action.yml index 1ea9def02..e870e8dc5 100644 --- a/action.yml +++ b/action.yml @@ -20,6 +20,10 @@ inputs: default: ${{ github.server_url == 'https://github.com' && github.token || '' }} cache-dependency-path: description: "Used to specify the path to dependency files. Supports wildcards or a list of file names for caching multiple dependencies." + overwrite: + description: Whether to overwrite existing file in workspace + required: false + default: 'false' update-environment: description: "Set this option if you want the action to update environment variables." default: true diff --git a/dist/setup/index.js b/dist/setup/index.js index c752812f5..3a26375c2 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96883,14 +96883,16 @@ function isGraalPyVersion(versionSpec) { } function cacheDependencies(cache, pythonVersion) { return __awaiter(this, void 0, void 0, function* () { + var _a; const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; - let resolvedDependencyPath; + let resolvedDependencyPath = undefined; + const overwrite = (_a = core.getBooleanInput('overwrite', { required: false })) !== null && _a !== void 0 ? _a : false; if (cacheDependencyPath) { - const actionPath = process.env.GITHUB_ACTION_PATH || path.resolve(__dirname, '..'); + const actionPath = process.env.GITHUB_ACTION_PATH || ''; const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); const sourcePath = path.resolve(actionPath, cacheDependencyPath); - const safeFolder = path.join(workspace, '.setup-python-deps'); - const safeTargetPath = path.join(safeFolder, path.basename(sourcePath)); + 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) @@ -96900,25 +96902,40 @@ function cacheDependencies(cache, pythonVersion) { core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); } else { - yield fs_1.default.promises.mkdir(safeFolder, { recursive: true }); - yield fs_1.default.promises.copyFile(sourcePath, safeTargetPath); - core.info(`Copied ${sourcePath} to safe location: ${safeTargetPath}`); + if (sourcePath !== targetPath) { + const targetDir = path.dirname(targetPath); + yield fs_1.default.promises.mkdir(targetDir, { recursive: true }); + const targetExists = yield fs_1.default.promises + .access(targetPath, fs_1.default.constants.F_OK) + .then(() => true) + .catch(() => false); + if (targetExists && !overwrite) { + core.warning(`A file with the same name already exists in the workspace at ${targetPath}. ` + + `Since 'overwrite' is false, the existing file will be used instead of the one provided by the action. ` + + `To use the action's file, consider renaming one of the files or setting 'overwrite: true'.`); + core.info(`Skipped copying ${sourcePath} — target already exists at ${targetPath}`); + } + else { + yield fs_1.default.promises.copyFile(sourcePath, targetPath); + core.info(`${targetExists ? 'Overwrote' : 'Copied'} ${sourcePath} to ${targetPath}`); + } + } + else { + core.info(`Dependency file is already inside the workspace: ${sourcePath}`); + } resolvedDependencyPath = path - .relative(workspace, safeTargetPath) + .relative(workspace, targetPath) .replace(/\\/g, '/'); core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); } } - catch (err) { - core.warning(`Failed to copy dependency file: ${err}`); + catch (error) { + core.warning(`Failed to copy file from ${sourcePath} to ${targetPath}: ${error}`); } } - const finalDependencyPath = resolvedDependencyPath; - if (!finalDependencyPath) { - core.setFailed('Failed to resolve dependency path from action.'); - return; - } - const cacheDistributor = (0, cache_factory_1.getCacheDistributor)(cache, pythonVersion, finalDependencyPath); + // 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(); }); } diff --git a/src/setup-python.ts b/src/setup-python.ts index e8872802e..62eda0947 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -21,18 +21,20 @@ function isPyPyVersion(versionSpec: string) { function isGraalPyVersion(versionSpec: string) { return versionSpec.startsWith('graalpy'); } - export async function cacheDependencies(cache: string, pythonVersion: string) { - const cacheDependencyPath = core.getInput('cache-dependency-path') || undefined; - let resolvedDependencyPath: string | undefined; + const cacheDependencyPath = + core.getInput('cache-dependency-path') || undefined; + let resolvedDependencyPath: string | undefined = undefined; + const overwrite = + core.getBooleanInput('overwrite', {required: false}) ?? false; if (cacheDependencyPath) { - const actionPath = process.env.GITHUB_ACTION_PATH || path.resolve(__dirname, '..'); + const actionPath = process.env.GITHUB_ACTION_PATH || ''; const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); - const sourcePath = path.resolve(actionPath, cacheDependencyPath); - const safeFolder = path.join(workspace, '.setup-python-deps'); - const safeTargetPath = path.join(safeFolder, path.basename(sourcePath)); + const sourcePath = path.resolve(actionPath, cacheDependencyPath); + const relativePath = path.relative(actionPath, sourcePath); + const targetPath = path.resolve(workspace, relativePath); try { const sourceExists = await fs.promises @@ -41,33 +43,59 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { .catch(() => false); if (!sourceExists) { - core.warning(`The resolved cache-dependency-path does not exist: ${sourcePath}`); + core.warning( + `The resolved cache-dependency-path does not exist: ${sourcePath}` + ); } else { - await fs.promises.mkdir(safeFolder, { recursive: true }); - await fs.promises.copyFile(sourcePath, safeTargetPath); - core.info(`Copied ${sourcePath} to safe location: ${safeTargetPath}`); + if (sourcePath !== targetPath) { + const targetDir = path.dirname(targetPath); + await fs.promises.mkdir(targetDir, {recursive: true}); + + const targetExists = await fs.promises + .access(targetPath, fs.constants.F_OK) + .then(() => true) + .catch(() => false); + + if (targetExists && !overwrite) { + core.warning( + `A file with the same name already exists in the workspace at ${targetPath}. ` + + `Since 'overwrite' is false, the existing file will be used instead of the one provided by the action. ` + + `To use the action's file, consider renaming one of the files or setting 'overwrite: true'.` + ); + core.info( + `Skipped copying ${sourcePath} — target already exists at ${targetPath}` + ); + } else { + await fs.promises.copyFile(sourcePath, targetPath); + core.info( + `${targetExists ? 'Overwrote' : 'Copied'} ${sourcePath} to ${targetPath}` + ); + } + } else { + core.info( + `Dependency file is already inside the workspace: ${sourcePath}` + ); + } resolvedDependencyPath = path - .relative(workspace, safeTargetPath) + .relative(workspace, targetPath) .replace(/\\/g, '/'); - core.info(`Resolved cache-dependency-path: ${resolvedDependencyPath}`); } - } catch (err) { - core.warning(`Failed to copy dependency file: ${err}`); + } catch (error) { + core.warning( + `Failed to copy file from ${sourcePath} to ${targetPath}: ${error}` + ); } } - const finalDependencyPath = resolvedDependencyPath; - if (!finalDependencyPath) { - core.setFailed('Failed to resolve dependency path from action.'); - return; - } + // Pass resolvedDependencyPath if available, else fallback to original input + const dependencyPathForCache = resolvedDependencyPath ?? cacheDependencyPath; const cacheDistributor = getCacheDistributor( cache, pythonVersion, - finalDependencyPath + dependencyPathForCache ); await cacheDistributor.restoreCache(); } From 36a0016df744188d08f0576199911ec2682d7d3b Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Mon, 7 Jul 2025 19:55:50 +0530 Subject: [PATCH 10/16] error message update --- dist/setup/index.js | 6 +++--- src/setup-python.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index 3a26375c2..67f7d2422 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96910,9 +96910,9 @@ function cacheDependencies(cache, pythonVersion) { .then(() => true) .catch(() => false); if (targetExists && !overwrite) { - core.warning(`A file with the same name already exists in the workspace at ${targetPath}. ` + - `Since 'overwrite' is false, the existing file will be used instead of the one provided by the action. ` + - `To use the action's file, consider renaming one of the files or setting 'overwrite: true'.`); + core.warning(`build + A file named 'requirements.txt' exists in both the composite action and the workspace. Using the action's file. To avoid ambiguity, consider renaming one of the files. +`); core.info(`Skipped copying ${sourcePath} — target already exists at ${targetPath}`); } else { diff --git a/src/setup-python.ts b/src/setup-python.ts index 62eda0947..8646a932f 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -58,9 +58,9 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { if (targetExists && !overwrite) { core.warning( - `A file with the same name already exists in the workspace at ${targetPath}. ` + - `Since 'overwrite' is false, the existing file will be used instead of the one provided by the action. ` + - `To use the action's file, consider renaming one of the files or setting 'overwrite: true'.` + `build + A file named 'requirements.txt' exists in both the composite action and the workspace. Using the action's file. To avoid ambiguity, consider renaming one of the files. +` ); core.info( `Skipped copying ${sourcePath} — target already exists at ${targetPath}` From 4b170ce46c3abad1a17219236165f9a5b41b001a Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Tue, 8 Jul 2025 11:30:33 +0530 Subject: [PATCH 11/16] updated error message --- dist/setup/index.js | 4 ++-- src/setup-python.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index 67f7d2422..bdce9ff10 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96910,9 +96910,9 @@ function cacheDependencies(cache, pythonVersion) { .then(() => true) .catch(() => false); if (targetExists && !overwrite) { + const filename = path.basename(cacheDependencyPath); core.warning(`build - A file named 'requirements.txt' exists in both the composite action and the workspace. Using the action's file. To avoid ambiguity, consider renaming one of the files. -`); + A file named '${filename}' exists in both the composite action and the workspace. The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.`); core.info(`Skipped copying ${sourcePath} — target already exists at ${targetPath}`); } else { diff --git a/src/setup-python.ts b/src/setup-python.ts index 8646a932f..b38b472f6 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -57,10 +57,10 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { .catch(() => false); if (targetExists && !overwrite) { + const filename = path.basename(cacheDependencyPath); core.warning( `build - A file named 'requirements.txt' exists in both the composite action and the workspace. Using the action's file. To avoid ambiguity, consider renaming one of the files. -` + A file named '${filename}' exists in both the composite action and the workspace. The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.` ); core.info( `Skipped copying ${sourcePath} — target already exists at ${targetPath}` From 66b988aa938241b95254ecfff9f6e9f53d7b4dbb Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Tue, 8 Jul 2025 11:45:00 +0530 Subject: [PATCH 12/16] error message format update --- dist/setup/index.js | 4 ++-- src/setup-python.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index bdce9ff10..7dda78304 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96911,8 +96911,8 @@ function cacheDependencies(cache, pythonVersion) { .catch(() => false); if (targetExists && !overwrite) { const filename = path.basename(cacheDependencyPath); - core.warning(`build - A file named '${filename}' exists in both the composite action and the workspace. The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.`); + core.warning(`A file named '${filename}' exists in both the composite action and the workspace.\n` + + `The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.`); core.info(`Skipped copying ${sourcePath} — target already exists at ${targetPath}`); } else { diff --git a/src/setup-python.ts b/src/setup-python.ts index b38b472f6..9c997ceac 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -59,8 +59,8 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { if (targetExists && !overwrite) { const filename = path.basename(cacheDependencyPath); core.warning( - `build - A file named '${filename}' exists in both the composite action and the workspace. The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.` + `A file named '${filename}' exists in both the composite action and the workspace.\n` + + `The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.` ); core.info( `Skipped copying ${sourcePath} — target already exists at ${targetPath}` From 0c1f12ec4307a15bb3e09e6787461fc6207cb566 Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Tue, 8 Jul 2025 11:56:06 +0530 Subject: [PATCH 13/16] error message format update --- dist/setup/index.js | 3 +-- src/setup-python.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dist/setup/index.js b/dist/setup/index.js index 7dda78304..6fda7a706 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96911,8 +96911,7 @@ function cacheDependencies(cache, pythonVersion) { .catch(() => false); if (targetExists && !overwrite) { const filename = path.basename(cacheDependencyPath); - core.warning(`A file named '${filename}' exists in both the composite action and the workspace.\n` + - `The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.`); + core.warning(`A file named '${filename}' exists in both the composite action and the workspace. The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.`); core.info(`Skipped copying ${sourcePath} — target already exists at ${targetPath}`); } else { diff --git a/src/setup-python.ts b/src/setup-python.ts index 9c997ceac..e09231f9d 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -59,8 +59,7 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { if (targetExists && !overwrite) { const filename = path.basename(cacheDependencyPath); core.warning( - `A file named '${filename}' exists in both the composite action and the workspace.\n` + - `The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.` + `A file named '${filename}' exists in both the composite action and the workspace. The file in the workspace will be used. To avoid ambiguity, consider renaming one of the files or setting 'overwrite: true'.` ); core.info( `Skipped copying ${sourcePath} — target already exists at ${targetPath}` From ebc324d6cae036bf99d48acaf3c386d3cd48f75c Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Tue, 8 Jul 2025 18:22:10 +0530 Subject: [PATCH 14/16] String update --- action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/action.yml b/action.yml index e870e8dc5..eb83fa8e8 100644 --- a/action.yml +++ b/action.yml @@ -23,7 +23,7 @@ inputs: overwrite: description: Whether to overwrite existing file in workspace required: false - default: 'false' + default: false update-environment: description: "Set this option if you want the action to update environment variables." default: true From b642e81cee7d47ca190ac88ad3cc162cfb5d657a Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Fri, 11 Jul 2025 13:09:56 +0530 Subject: [PATCH 15/16] documentation update --- action.yml | 5 ++--- docs/advanced-usage.md | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/action.yml b/action.yml index eb83fa8e8..a2c6ad22c 100644 --- a/action.yml +++ b/action.yml @@ -21,9 +21,8 @@ inputs: cache-dependency-path: description: "Used to specify the path to dependency files. Supports wildcards or a list of file names for caching multiple dependencies." overwrite: - description: Whether to overwrite existing file in workspace - required: false - default: false + description: "When 'true', allows files from the composite action’s cache-dependency-path to overwrite existing files in the workspace with the same name. Useful for common filenames like 'requirements.txt'." + default: false update-environment: description: "Set this option if you want the action to update environment variables." default: true diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index a901ed581..7934f0b44 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -413,7 +413,7 @@ steps: # 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. -To avoid unintentionally overwriting existing files in the workspace (especially when using composite actions with common file names like requirements.txt), a new input overwrite has been added. By default, files will not be copied if a file with the same path already exists in the workspace unless overwrite: true is explicitly set. +A new input overwrite has been introduced to prevent accidental overwriting of existing files in the workspace when a composite action’s cache-dependency-path refers to common filenames like requirements.txt. By default, if a file with the same path already exists in the workspace, it will not be copied from the action unless overwrite: true is explicitly set. # Outputs and environment variables ## Outputs From 4ca86e45d18222a321b453df9c992dee663b9b30 Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Fri, 11 Jul 2025 13:14:27 +0530 Subject: [PATCH 16/16] update --- action.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/action.yml b/action.yml index a2c6ad22c..e870e8dc5 100644 --- a/action.yml +++ b/action.yml @@ -21,8 +21,9 @@ inputs: cache-dependency-path: description: "Used to specify the path to dependency files. Supports wildcards or a list of file names for caching multiple dependencies." overwrite: - description: "When 'true', allows files from the composite action’s cache-dependency-path to overwrite existing files in the workspace with the same name. Useful for common filenames like 'requirements.txt'." - default: false + description: Whether to overwrite existing file in workspace + required: false + default: 'false' update-environment: description: "Set this option if you want the action to update environment variables." default: true