From bf549696cd48f7400292cba2ddb974c3b0c6f970 Mon Sep 17 00:00:00 2001 From: Aparna Jyothi Date: Fri, 11 Jul 2025 13:33:16 +0530 Subject: [PATCH] disable-auto-overwriting --- __tests__/setup-python.test.ts | 122 +++++++++++++++++---------------- action.yml | 3 + dist/setup/index.js | 19 +++-- docs/advanced-usage.md | 4 +- src/setup-python.ts | 28 ++++++-- 5 files changed, 105 insertions(+), 71 deletions(-) diff --git a/__tests__/setup-python.test.ts b/__tests__/setup-python.test.ts index bb27289d..318d9a97 100644 --- a/__tests__/setup-python.test.ts +++ b/__tests__/setup-python.test.ts @@ -33,45 +33,26 @@ describe('cacheDependencies', () => { 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); + mockedCore.getBooleanInput.mockReturnValue(false); mockedGetCacheDistributor.mockReturnValue({restoreCache: mockRestoreCache}); + + mockedFsPromises.mkdir.mockResolvedValue(undefined); + mockedFsPromises.copyFile.mockResolvedValue(undefined); }); - 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( - sourcePath, - fs.constants.F_OK - ); - expect(mockedFsPromises.mkdir).toHaveBeenCalledWith( - path.dirname(targetPath), - { - recursive: true - } - ); expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( sourcePath, targetPath @@ -79,15 +60,45 @@ describe('cacheDependencies', () => { 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')); + 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.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 e469b7b2..9a6e2c44 100644 --- a/action.yml +++ b/action.yml @@ -31,6 +31,9 @@ inputs: default: false pip-version: description: "Used to specify the version of pip to install with the Python. Supported format: major[.minor][.patch]." + overwrite: + description: "Whether to overwrite existing files in the workspace when a composite action’s cache-dependency-path points to a file with the same name (e.g., requirements.txt). Defaults to false." + default: 'false' outputs: python-version: description: "The installed Python or PyPy version. Useful when given a version range as input." diff --git a/dist/setup/index.js b/dist/setup/index.js index f8c5d4e7..6fda7a70 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,20 @@ 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) { + const filename = path.basename(cacheDependencyPath); + 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 { + 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}`); diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index 96524823..ca945c8c 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -1,3 +1,4 @@ + # Advanced Usage - [Using the python-version input](advanced-usage.md#using-the-python-version-input) - [Specifying a Python version](advanced-usage.md#specifying-a-python-version) @@ -413,6 +414,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. +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 @@ -662,4 +664,4 @@ The version of Pip should be specified in the format `major`, `major.minor`, or ``` > The `pip-version` input is supported only with standard Python versions. It is not available when using PyPy or GraalPy. -> Using a specific or outdated version of pip may result in compatibility or security issues and can cause job failures. For best practices and guidance, refer to the official [pip documentation](https://pip.pypa.io/en/stable/). \ No newline at end of file +> Using a specific or outdated version of pip may result in compatibility or security issues and can cause job failures. For best practices and guidance, refer to the official [pip documentation](https://pip.pypa.io/en/stable/). diff --git a/src/setup-python.ts b/src/setup-python.ts index 106b415a..e09231f9 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -21,11 +21,12 @@ 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 = undefined; + const overwrite = + core.getBooleanInput('overwrite', {required: false}) ?? false; if (cacheDependencyPath) { const actionPath = process.env.GITHUB_ACTION_PATH || ''; @@ -48,11 +49,27 @@ 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) { + const filename = path.basename(cacheDependencyPath); + 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 { + 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}` @@ -81,7 +98,6 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { ); await cacheDistributor.restoreCache(); } - function resolveVersionInputFromDefaultFile(): string[] { const couples: [string, (versionFile: string) => string[]][] = [ ['.python-version', getVersionsInputFromPlainFile]