diff --git a/__tests__/setup-python.test.ts b/__tests__/setup-python.test.ts index bb27289de..2ef043004 100644 --- a/__tests__/setup-python.test.ts +++ b/__tests__/setup-python.test.ts @@ -19,10 +19,14 @@ jest.mock('fs', () => { }); jest.mock('@actions/core'); jest.mock('../src/cache-distributions/cache-factory'); +jest.mock('crypto', () => ({ + randomUUID: jest.fn(() => '12345678-9abc-def0-1234-56789abcdef0') +})); const mockedFsPromises = fs.promises as jest.Mocked; const mockedCore = core as jest.Mocked; const mockedGetCacheDistributor = getCacheDistributor as jest.Mock; +const {randomUUID} = require('crypto'); describe('cacheDependencies', () => { const mockRestoreCache = jest.fn(); @@ -31,119 +35,141 @@ describe('cacheDependencies', () => { 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}); + (randomUUID as jest.Mock).mockReturnValue( + '12345678-9abc-def0-1234-56789abcdef0' + ); }); - it('copies the dependency file and resolves the path with directory structure', async () => { + it('copies to temp folder on conflict and resolves correct path', async () => { + mockedFsPromises.access.mockImplementation(async p => { + const filePath = typeof p === 'string' ? p : p.toString(); + if ( + filePath === '/github/action/nested/deps.lock' || + filePath === '/github/workspace/deps.lock' + ) { + return Promise.resolve(); // Source and conflict path exists + } + if (filePath === '/github/workspace/.tmp-cache-deps-12345678/deps.lock') { + return Promise.resolve(); // File appears after copy + } + return Promise.reject(new Error('Not found')); + }); + await cacheDependencies('pip', '3.12'); - const sourcePath = path.resolve('/github/action', 'nested/deps.lock'); - const targetPath = path.resolve('/github/workspace', 'nested/deps.lock'); + const source = '/github/action/nested/deps.lock'; + const target = '/github/workspace/.tmp-cache-deps-12345678/deps.lock'; - expect(mockedFsPromises.access).toHaveBeenCalledWith( - sourcePath, - fs.constants.F_OK - ); - expect(mockedFsPromises.mkdir).toHaveBeenCalledWith( - path.dirname(targetPath), - { - recursive: true - } + expect(mockedFsPromises.copyFile).toHaveBeenCalledWith(source, target); + expect(mockedCore.info).toHaveBeenCalledWith( + `Copied ${source} to ${target}` ); - expect(mockedFsPromises.copyFile).toHaveBeenCalledWith( - sourcePath, - targetPath + expect(mockedCore.info).toHaveBeenCalledWith( + `Resolved cache-dependency-path: .tmp-cache-deps-12345678/deps.lock` ); + expect(mockRestoreCache).toHaveBeenCalled(); + }); + + it('copies preserving relative structure if no conflict', async () => { + mockedFsPromises.access.mockImplementation(async p => { + const filePath = typeof p === 'string' ? p : p.toString(); + if (filePath === '/github/action/nested/deps.lock') { + return Promise.resolve(); // Source exists + } + if (filePath === '/github/workspace/deps.lock') { + return Promise.reject(new Error('No conflict')); // No conflict + } + if (filePath === '/github/workspace/nested/deps.lock') { + return Promise.resolve(); // Exists after copy + } + return Promise.reject(new Error('Not found')); + }); + + await cacheDependencies('pip', '3.12'); + + const target = '/github/workspace/nested/deps.lock'; + expect(mockedFsPromises.copyFile).toHaveBeenCalled(); expect(mockedCore.info).toHaveBeenCalledWith( - `Copied ${sourcePath} to ${targetPath}` + expect.stringContaining('Copied') ); expect(mockedCore.info).toHaveBeenCalledWith( - `Resolved cache-dependency-path: nested/deps.lock` + '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('warns if file does not exist', async () => { + mockedFsPromises.access.mockRejectedValue(new Error('Not found')); await cacheDependencies('pip', '3.12'); expect(mockedCore.warning).toHaveBeenCalledWith( expect.stringContaining('does not exist') ); - expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); - expect(mockRestoreCache).toHaveBeenCalled(); + expect(mockRestoreCache).not.toHaveBeenCalled(); + expect(mockedCore.setFailed).toHaveBeenCalledWith( + expect.stringContaining('Dependency file does not exist') + ); }); - it('warns if file copy fails', async () => { - // Simulate copyFile failure - mockedFsPromises.copyFile.mockRejectedValue(new Error('copy failed')); + it('handles copy error gracefully and still validates fallback path', async () => { + mockedFsPromises.access.mockImplementation(async p => { + const filePath = typeof p === 'string' ? p : p.toString(); + if (filePath === '/github/action/nested/deps.lock') { + return Promise.resolve(); + } + if (filePath === '/github/workspace/nested/deps.lock') { + return Promise.resolve(); // Used as fallback after failed copy + } + return Promise.reject(new Error('No access')); + }); + + mockedFsPromises.copyFile.mockRejectedValue(new Error('Permission denied')); await cacheDependencies('pip', '3.12'); expect(mockedCore.warning).toHaveBeenCalledWith( expect.stringContaining('Failed to copy file') ); + expect(mockedCore.setOutput).toHaveBeenCalledWith( + 'resolvedDependencyPath', + 'nested/deps.lock' + ); expect(mockRestoreCache).toHaveBeenCalled(); }); - it('skips path logic if no input is provided', async () => { + it('skips all logic if input not provided', async () => { mockedCore.getInput.mockReturnValue(''); await cacheDependencies('pip', '3.12'); expect(mockedFsPromises.copyFile).not.toHaveBeenCalled(); - expect(mockedCore.warning).not.toHaveBeenCalled(); - expect(mockRestoreCache).toHaveBeenCalled(); + expect(mockRestoreCache).not.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(); + it('fails if final dependency path check fails', async () => { + mockedFsPromises.access.mockImplementation(async p => { + const filePath = typeof p === 'string' ? p : p.toString(); + if (filePath === '/github/action/nested/deps.lock') { + return Promise.resolve(); + } + if (filePath === '/github/workspace/nested/deps.lock') { + return Promise.reject(new Error('Does not exist')); + } + return Promise.reject(new Error('no access')); + }); 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.setFailed).toHaveBeenCalledWith( + expect.stringContaining('Dependency file does not exist at:') ); - expect(mockedCore.info).toHaveBeenCalledWith( - `Resolved cache-dependency-path: deps.lock` - ); - expect(mockRestoreCache).toHaveBeenCalled(); + expect(mockRestoreCache).not.toHaveBeenCalled(); }); }); diff --git a/dist/setup/index.js b/dist/setup/index.js index f8c5d4e72..d3cd80940 100644 --- a/dist/setup/index.js +++ b/dist/setup/index.js @@ -96874,6 +96874,7 @@ const path = __importStar(__nccwpck_require__(6928)); const os = __importStar(__nccwpck_require__(857)); const fs_1 = __importDefault(__nccwpck_require__(9896)); const cache_factory_1 = __nccwpck_require__(665); +const crypto_1 = __nccwpck_require__(6982); const utils_1 = __nccwpck_require__(1798); function isPyPyVersion(versionSpec) { return versionSpec.startsWith('pypy'); @@ -96889,8 +96890,6 @@ function cacheDependencies(cache, pythonVersion) { 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) @@ -96900,17 +96899,28 @@ function cacheDependencies(cache, pythonVersion) { 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}`); + const filename = path.basename(sourcePath); + const workspaceConflictPath = path.resolve(workspace, filename); + let targetPath; + // Check for conflict at root of workspace + const conflictExists = yield fs_1.default.promises + .access(workspaceConflictPath, fs_1.default.constants.F_OK) + .then(() => true) + .catch(() => false); + if (conflictExists) { + // Create a temporary unique folder inside workspace + const tempDir = path.join(workspace, `.tmp-cache-deps-${(0, crypto_1.randomUUID)().slice(0, 8)}`); + yield fs_1.default.promises.mkdir(tempDir, { recursive: true }); + targetPath = path.join(tempDir, filename); } else { - core.info(`Dependency file is already inside the workspace: ${sourcePath}`); + // Default behavior — mirror directory structure from action + const relativePath = path.relative(actionPath, sourcePath); + targetPath = path.resolve(workspace, relativePath); + yield fs_1.default.promises.mkdir(path.dirname(targetPath), { recursive: true }); } + yield fs_1.default.promises.copyFile(sourcePath, targetPath); + core.info(`Copied ${sourcePath} to ${targetPath}`); resolvedDependencyPath = path .relative(workspace, targetPath) .replace(/\\/g, '/'); @@ -96918,11 +96928,24 @@ function cacheDependencies(cache, pythonVersion) { } } catch (error) { - core.warning(`Failed to copy file from ${sourcePath} to ${targetPath}: ${error}`); + core.warning(`Failed to copy file from ${sourcePath} to workspace: ${error}`); } } - // Pass resolvedDependencyPath if available, else fallback to original input + // Prefer resolvedDependencyPath if set, else fallback to cacheDependencyPath const dependencyPathForCache = resolvedDependencyPath !== null && resolvedDependencyPath !== void 0 ? resolvedDependencyPath : cacheDependencyPath; + // Validate that the path exists before proceeding + const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); + const absoluteDepPath = path.resolve(workspace, dependencyPathForCache || ''); + const depExists = yield fs_1.default.promises + .access(absoluteDepPath, fs_1.default.constants.F_OK) + .then(() => true) + .catch(() => false); + if (!depExists) { + core.setFailed(`Dependency file does not exist at: ${dependencyPathForCache} (resolved to ${absoluteDepPath})`); + return; + } + // Set output for downstream workflow steps + core.setOutput('resolvedDependencyPath', dependencyPathForCache); const cacheDistributor = (0, cache_factory_1.getCacheDistributor)(cache, pythonVersion, dependencyPathForCache); yield cacheDistributor.restoreCache(); }); diff --git a/docs/advanced-usage.md b/docs/advanced-usage.md index 96524823e..16e4b5bd0 100644 --- a/docs/advanced-usage.md +++ b/docs/advanced-usage.md @@ -412,7 +412,11 @@ 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. +Note: +cache-dependency-path supports files located outside the workspace root by copying them into the workspace to enable proper caching. + +Additionally, if a dependency file (such as requirements.txt) with the same name already exists at the workspace root, the action avoids overwriting it by copying the incoming file to a uniquely named temporary directory within the workspace (e.g., .tmp-cache-deps-xxxx/requirements.txt). This ensures caching can proceed safely without interfering with user-managed files. + # Outputs and environment variables ## Outputs diff --git a/src/setup-python.ts b/src/setup-python.ts index 106b415a6..13d5fa5c9 100644 --- a/src/setup-python.ts +++ b/src/setup-python.ts @@ -6,6 +6,7 @@ import * as path from 'path'; import * as os from 'os'; import fs from 'fs'; import {getCacheDistributor} from './cache-distributions/cache-factory'; +import {randomUUID} from 'crypto'; import { isCacheFeatureAvailable, logWarning, @@ -32,8 +33,6 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { 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 @@ -46,19 +45,34 @@ 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); - // 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}` + const filename = path.basename(sourcePath); + const workspaceConflictPath = path.resolve(workspace, filename); + + let targetPath: string; + + // Check for conflict at root of workspace + const conflictExists = await fs.promises + .access(workspaceConflictPath, fs.constants.F_OK) + .then(() => true) + .catch(() => false); + + if (conflictExists) { + // Create a temporary unique folder inside workspace + const tempDir = path.join( + workspace, + `.tmp-cache-deps-${randomUUID().slice(0, 8)}` ); + await fs.promises.mkdir(tempDir, {recursive: true}); + targetPath = path.join(tempDir, filename); + } else { + // Default behavior — mirror directory structure from action + const relativePath = path.relative(actionPath, sourcePath); + targetPath = path.resolve(workspace, relativePath); + await fs.promises.mkdir(path.dirname(targetPath), {recursive: true}); } + await fs.promises.copyFile(sourcePath, targetPath); + core.info(`Copied ${sourcePath} to ${targetPath}`); resolvedDependencyPath = path .relative(workspace, targetPath) .replace(/\\/g, '/'); @@ -66,14 +80,32 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { } } catch (error) { core.warning( - `Failed to copy file from ${sourcePath} to ${targetPath}: ${error}` + `Failed to copy file from ${sourcePath} to workspace: ${error}` ); } } - // Pass resolvedDependencyPath if available, else fallback to original input + // Prefer resolvedDependencyPath if set, else fallback to cacheDependencyPath const dependencyPathForCache = resolvedDependencyPath ?? cacheDependencyPath; + // Validate that the path exists before proceeding + const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); + const absoluteDepPath = path.resolve(workspace, dependencyPathForCache || ''); + const depExists = await fs.promises + .access(absoluteDepPath, fs.constants.F_OK) + .then(() => true) + .catch(() => false); + + if (!depExists) { + core.setFailed( + `Dependency file does not exist at: ${dependencyPathForCache} (resolved to ${absoluteDepPath})` + ); + return; + } + + // Set output for downstream workflow steps + core.setOutput('resolvedDependencyPath', dependencyPathForCache); + const cacheDistributor = getCacheDistributor( cache, pythonVersion, @@ -81,7 +113,6 @@ export async function cacheDependencies(cache: string, pythonVersion: string) { ); await cacheDistributor.restoreCache(); } - function resolveVersionInputFromDefaultFile(): string[] { const couples: [string, (versionFile: string) => string[]][] = [ ['.python-version', getVersionsInputFromPlainFile]