Skip to content

Improve cache-dependency-path handling to avoid file conflicts and support reliable resolution #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 92 additions & 66 deletions __tests__/setup-python.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
});
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<typeof fs.promises>;
const mockedCore = core as jest.Mocked<typeof core>;
const mockedGetCacheDistributor = getCacheDistributor as jest.Mock;
const {randomUUID} = require('crypto');

Check failure on line 29 in __tests__/setup-python.test.ts

View workflow job for this annotation

GitHub Actions / Basic validation / build (windows-latest)

Require statement not part of import statement

Check failure on line 29 in __tests__/setup-python.test.ts

View workflow job for this annotation

GitHub Actions / Basic validation / build (windows-latest)

A `require()` style import is forbidden

Check failure on line 29 in __tests__/setup-python.test.ts

View workflow job for this annotation

GitHub Actions / Basic validation / build (ubuntu-latest)

Require statement not part of import statement

Check failure on line 29 in __tests__/setup-python.test.ts

View workflow job for this annotation

GitHub Actions / Basic validation / build (ubuntu-latest)

A `require()` style import is forbidden

Check failure on line 29 in __tests__/setup-python.test.ts

View workflow job for this annotation

GitHub Actions / Basic validation / build (macos-latest)

Require statement not part of import statement

Check failure on line 29 in __tests__/setup-python.test.ts

View workflow job for this annotation

GitHub Actions / Basic validation / build (macos-latest)

A `require()` style import is forbidden

describe('cacheDependencies', () => {
const mockRestoreCache = jest.fn();
Expand All @@ -31,119 +35,141 @@
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();
});
});
47 changes: 35 additions & 12 deletions dist/setup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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)
Expand All @@ -96900,29 +96899,53 @@ 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, '/');
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 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();
});
Expand Down
6 changes: 5 additions & 1 deletion docs/advanced-usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading