10000 Use correct Poetry config when collecting Poetry projects (#447) · ifit/setup-python@8b89ef0 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8b89ef0

Browse files
oranavoranav
authored
Use correct Poetry config when collecting Poetry projects (actions#447)
* Use correct Poetry config when collecting Poetry projects When collecting Poetry projects for caching, a '**/poetry.lock' glob is used. However, in order to process the Poetry configuration, the "poetry" command is run from the repo's root directory; this causes Poetry to return an invalid configuration when there is a Poetry project inside an inner directory. Instead of running a single Poetry command, glob for the same pattern, and run a Poetry command for every discovered project. * Fix typo: saveSatetSpy -> saveStateSpy * poetry: Support same virtualenv appearing in multiple projects * Add nested Poetry projects test * poetry: Set up environment for each project individually * tests/cache-restore: Do not look for dependency files outside `data` When the default dependency path is used for cache distributors, they are looking for the dependency file in the project's root (including the source code), which leads to tests taking a significant amount of time, especially on Windows runners. We thus hit sporadic test failures. Change the test cases such that dependency files are always searched for inside of `__tests__/data`, ignoring the rest of the project. * poetry: Simplify `virtualenvs.in-project` boolean check * README: Explain that poetry might create multiple caches * poetry: Run `poetry env use` only after cache is loaded The virtualenv cache might contain invalid entries, such as virtualenvs built in previous, buggy versions of this action. The `poetry env use` command will recreate virtualenvs in case they are invalid, but it has to be run only *after* the cache is loaded. Refactor `CacheDistributor` a bit such that the validation (and possible recreation) of virtualenvs happens only after the cache is loaded. * poetry: Bump cache primary key
1 parent 5ccb29d commit 8b89ef0

File tree

9 files changed

+213
-77
lines changed

9 files changed

+213
-77
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ The action defaults to searching for a dependency file (`requirements.txt` for p
5555

5656
- For `pip`, the action will cache the global cache directory
5757
- For `pipenv`, the action will cache virtualenv directory
58-
- For `poetry`, the action will cache virtualenv directory
58+
- For `poetry`, the action will cache virtualenv directories -- one for each poetry project found
5959

6060
**Caching pip dependencies:**
6161

‎__tests__/cache-restore.test.ts

Lines changed: 81 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import * as path from 'path';
12
import * as core from '@actions/core';
23
import * as cache from '@actions/cache';
34
import * as exec from '@actions/exec';
45
import * as io from '@actions/io';
56
import {getCacheDistributor} from '../src/cache-distributions/cache-factory';
7+
import {State} from '../src/cache-distributions/cache-distributor';
68
import * as utils from './../src/utils';
79

810
describe('restore-cache', () => {
@@ -13,7 +15,7 @@ describe('restore-cache', () => {
1315
const requirementsLinuxHash =
1416
'2d0ff7f46b0e120e3d3294db65768b474934242637b9899b873e6283dfd16d7c';
1517
const poetryLockHash =
16-
'571bf984f8d210e6a97f854e479fdd4a2b5af67b5fdac109ec337a0ea16e7836';
18+
'f24ea1ad73968e6c8d80c16a093ade72d9332c433aeef979a0dd943e6a99b2ab';
1719
const poetryConfigOutput = `
1820
cache-dir = "/Users/patrick/Library/Caches/pypoetry"
1921
experimental.new-installer = false
@@ -27,7 +29,7 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py
2729
let infoSpy: jest.SpyInstance;
2830
let warningSpy: jest.SpyInstance;
2931
let debugSpy: jest.SpyInstance;
30-
let saveSatetSpy: jest.SpyInstance;
32+
let saveStateSpy: jest.SpyInstance;
3133
let getStateSpy: jest.SpyInstance;
3234
let setOutputSpy: jest.SpyInstance;
3335

@@ -52,8 +54,8 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py
5254
debugSpy = jest.spyOn(core, 'debug');
5355
debugSpy.mockImplementation(input => undefined);
5456

55-
saveSatetSpy = jest.spyOn(core, 'saveState');
56-
saveSatetSpy.mockImplementation(input => undefined);
57+
saveStateSpy = jest.spyOn(core, 'saveState');
58+
saveStateSpy.mockImplementation(input => undefined);
5759

5860
getStateSpy = jest.spyOn(core, 'getState');
5961
getStateSpy.mockImplementation(input => undefined);
@@ -100,21 +102,68 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py
100102

101103
describe('Restore dependencies', () => {
102104
it.each([
103-
['pip', '3.8.12', undefined, requirementsHash],
104-
['pip', '3.8.12', '**/requirements-linux.txt', requirementsLinuxHash],
105+
[
106+
'pip',
107+
'3.8.12',
108+
'__tests__/data/**/requirements.txt',
109+
requirementsHash,
110+
undefined
111+
],
112+
[
113+
'pip',
114+
'3.8.12',
115+
'__tests__/data/**/requirements-linux.txt',
116+
requirementsLinuxHash,
117+< F438 /span>
undefined
118+
],
105119
[
106120
'pip',
107121
'3.8.12',
108122
'__tests__/data/requirements-linux.txt',
109-
requirementsLinuxHash
123+
requirementsLinuxHash,
124+
undefined
110125
],
111-
['pip', '3.8.12', '__tests__/data/requirements.txt', requirementsHash],
112-
['pipenv', '3.9.1', undefined, pipFileLockHash],
113-
['pipenv', '3.9.12', '__tests__/data/requirements.txt', requirementsHash],
114-
['poetry', '3.9.1', undefined, poetryLockHash]
126+
[
127+
'pip',
128+
'3.8.12',
129+
'__tests__/data/requirements.txt',
130+
requirementsHash,
131+
undefined
132+
],
133+
[
134+
'pipenv',
135+
'3.9.1',
136+
'__tests__/data/**/Pipfile.lock',
137+
pipFileLockHash,
138+
undefined
139+
],
140+
[
141+
'pipenv',
142+
'3.9.12',
143+
'__tests__/data/requirements.txt',
144+
requirementsHash,
145+
undefined
146+
],
147+
[
148+
'poetry',
149+
'3.9.1',
150+
'__tests__/data/**/poetry.lock',
151+
poetryLockHash,
152+
[
153+
'/Users/patrick/Library/Caches/pypoetry/virtualenvs',
154+
path.join(__dirname, 'data', 'inner', '.venv'),
155+
path.join(__dirname, 'data', '.venv')
156+
]
157+
]
115158
])(
116159
'restored dependencies for %s by primaryKey',
117-
async (packageManager, pythonVersion, dependencyFile, fileHash) => {
160+
async (
161+
packageManager,
162+
pythonVersion,
163+
dependencyFile,
164+
fileHash,
165+
cachePaths
166+
) => {
118167
const cacheDistributor = getCacheDistributor(
119168
packageManager,
120169
pythonVersion,
@@ -123,10 +172,21 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py
123172

124173
await cacheDistributor.restoreCache();
125174

175+
if (cachePaths !== undefined) {
176+
expect(saveStateSpy).toHaveBeenCalledWith(
177+
State.CACHE_PATHS,
178+
cachePaths
179+
);
180+
}
181+
126182
if (process.platform === 'linux' && packageManager === 'pip') {
127183
expect(infoSpy).toHaveBeenCalledWith(
128184
`Cache restored from key: setup-python-${process.env['RUNNER_OS']}-20.04-Ubuntu-python-${pythonVersion}-${packageManager}-${fileHash}`
129185
);
186+
} else if (packageManager === 'poetry') {
187+
expect(infoSpy).toHaveBeenCalledWith(
188+
`Cache restored from key: setup-python-${process.env['RUNNER_OS']}-python-${pythonVersion}-${packageManager}-v2-${fileHash}`
189+
);
130190
} else {
131191
expect(infoSpy).toHaveBeenCalledWith(
132192
`Cache restored from key: setup-python-${process.env['RUNNER_OS']}-python-${pythonVersion}-${packageManager}-${fileHash}`
@@ -164,18 +224,23 @@ virtualenvs.path = "{cache-dir}/virtualenvs" # /Users/patrick/Library/Caches/py
164224

165225
describe('Dependencies changed', () => {
166226
it.each([
167-
['pip', '3.8.12', undefined, pipFileLockHash],
168-
['pip', '3.8.12', '**/requirements-linux.txt', pipFileLockHash],
227+
['pip', '3.8.12', '__tests__/data/**/requirements.txt', pipFileLockHash],
228+
[
229+
'pip',
230+
'3.8.12',
231+
'__tests__/data/**/requirements-linux.txt',
232+
pipFileLockHash
233+
],
169234
[
170235
'pip',
171236
'3.8.12',
172237
'__tests__/data/requirements-linux.txt',
173238
pipFileLockHash
174239
],
175240
['pip', '3.8.12', '__tests__/data/requirements.txt', pipFileLockHash],
176-
['pipenv', '3.9.1', undefined, requirementsHash],
241+
['pipenv', '3.9.1', '__tests__/data/**/Pipfile.lock', requirementsHash],
177242
['pipenv', '3.9.12', '__tests__/data/requirements.txt', requirementsHash],
178-
['poetry', '3.9.1', undefined, requirementsHash]
243+
['poetry', '3.9.1', '__tests__/data/**/poetry.lock', requirementsHash]
179244
])(
180245
'restored dependencies for %s by primaryKey',
181246
async (packageManager, pythonVersion, dependencyFile, fileHash) => {

‎__tests__/cache-save.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ describe('run', () => {
1818
let infoSpy: jest.SpyInstance;
1919
let warningSpy: jest.SpyInstance;
2020
let debugSpy: jest.SpyInstance;
21-
let saveSatetSpy: jest.SpyInstance;
21+
let saveStateSpy: jest.SpyInstance;
2222
let getStateSpy: jest.SpyInstance;
2323
let getInputSpy: jest.SpyInstance;
2424
let setFailedSpy: jest.SpyInstance;
@@ -43,8 +43,8 @@ describe('run', () => {
4343
debugSpy = jest.spyOn(core, 'debug');
4444
debugSpy.mockImplementation(input => undefined);
4545

46-
saveSatetSpy = jest.spyOn(core, 'saveState');
47-
saveSatetSpy.mockImplementation(input => undefined);
46+
saveStateSpy = jest.spyOn(core, 'saveState');
47+
saveStateSpy.mockImplementation(input => undefined);
4848

4949
getStateSpy = jest.spyOn(core, 'getState');
5050
getStateSpy.mockImplementation(input => {

‎__tests__/data/inner/poetry.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎__tests__/data/inner/pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../pyproject.toml

‎dist/cache-save/index.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59711,6 +59711,9 @@ class CacheDistributor {
5971159711
this.cacheDependencyPath = cacheDependencyPath;
5971259712
this.CACHE_KEY_PREFIX = 'setup-python';
5971359713
}
59714+
handleLoadedCache() {
59715+
return __awaiter(this, void 0, void 0, function* () { });
59716+
}
5971459717
restoreCache() {
5971559718
return __awaiter(this, void 0, void 0, function* () {
5971659719
const { primaryKey, restoreKey } = yield this.computeKeys();
@@ -59723,6 +59726,7 @@ class CacheDistributor {
5972359726
core.saveState(State.CACHE_PATHS, cachePath);
5972459727
core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey);
5972559728
const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey);
59729+
yield this.handleLoadedCache();
5972659730
this.handleMatchResult(matchedKey, primaryKey);
5972759731
});
5972859732
}

‎dist/setup/index.js

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -65787,6 +65787,9 @@ class CacheDistributor {
6578765787
this.cacheDependencyPath = cacheDependencyPath;
6578865788
this.CACHE_KEY_PREFIX = 'setup-python';
6578965789
}
65790+
handleLoadedCache() {
65791+
return __awaiter(this, void 0, void 0, function* () { });
65792+
}
6579065793
restoreCache() {
6579165794
return __awaiter(this, void 0, void 0, function* () {
6579265795
const { primaryKey, restoreKey } = yield this.computeKeys();
@@ -65799,6 +65802,7 @@ class CacheDistributor {
6579965802
core.saveState(State.CACHE_PATHS, cachePath);
6580065803
core.saveState(State.STATE_CACHE_PRIMARY_KEY, primaryKey);
6580165804
const matchedKey = yield cache.restoreCache(cachePath, primaryKey, restoreKey);
65805+
yield this.handleLoadedCache();
6580265806
this.handleMatchResult(matchedKey, primaryKey);
6580365807
});
6580465808
}
@@ -66078,6 +66082,13 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
6607866082
step((generator = generator.apply(thisArg, _arguments || [])).next());
6607966083
});
6608066084
};
66085+
var __asyncValues = (this && this.__asyncValues) || function (o) {
66086+
if (!Symbol.asyncIterator) throw new TypeError("Symbol.asyncIterator is not defined.");
66087+
var m = o[Symbol.asyncIterator], i;
66088+
return m ? m.call(o) : (o = typeof __values === "function" ? __values(o) : o[Symbol.iterator](), i = {}, verb("next"), verb("throw"), verb("return"), i[Symbol.asyncIterator] = function () { return this; }, i);
66089+
function verb(n) { i[n] = o[n] && function (v) { return new Promise(function (resolve, reject) { v = o[n](v), settle(resolve, reject, v.done, v.value); }); }; }
66090+
function settle(resolve, reject, d, v) { Promise.resolve(v).then(function(v) { resolve({ value: v, done: d }); }, reject); }
66091+
};
6608166092
var __importDefault = (this && this.__importDefault) || function (mod) {
6608266093
return (mod && mod.__esModule) ? mod : { "default": mod };
6608366094
};
@@ -66090,51 +66101,82 @@ const core = __importStar(__nccwpck_require__(2186));
6609066101
const cache_distributor_1 = __importDefault(__nccwpck_require__(8953));
6609166102
const utils_1 = __nccwpck_require__(1314);
6609266103
class PoetryCache extends cache_distributor_1.default {
66093-
constructor(pythonVersion, patterns = '**/poetry.lock') {
66104+
constructor(pythonVersion, patterns = '**/poetry.lock', poetryProjects = new Set()) {
6609466105
super('poetry', patterns);
6609566106
this.pythonVersion = pythonVersion;
6609666107
this.patterns = patterns;
66108+
this.poetryProjects = poetryProjects;
6609766109
}
6609866110
getCacheGlobalDirectories() {
66111+
var e_1, _a;
6609966112
return __awaiter(this, void 0, void 0, function* () {
66100-
const poetryConfig = yield this.getPoetryConfiguration();
66101-
const cacheDir = poetryConfig['cache-dir'];
66102-
const virtualenvsPath = poetryConfig['virtualenvs.path'].replace('{cache-dir}', cacheDir);
66103-
const paths = [virtualenvsPath];
66104-
if (poetryConfig['virtualenvs.in-project'] === true) {
66105-
paths.push(path.join(process.cwd(), '.venv'));
66106-
}
66107-
const pythonLocation = yield io.which('python');
66108-
if (pythonLocation) {
66109-
core.debug(`pythonLocation is ${pythonLocation}`);
66110-
const { exitCode, stderr } = yield exec.getExecOutput(`poetry env use ${pythonLocation}`, undefined, { ignoreReturnCode: true });
66111-
if (exitCode) {
66112-
utils_1.logWarning(stderr);
66113+
// Same virtualenvs path may appear for different projects, hence we use a Set
66114+
const paths = new Set();
66115+
const globber = yield glob.create(this.patterns);
66116+
try {
66117+
for (var _b = __asyncValues(globber.globGenerator()), _c; _c = yield _b.next(), !_c.done;) {
66118+
const file = _c.value;
66119+
const basedir = path.dirname(file);
66120+
core.debug(`Processing Poetry project at ${basedir}`);
66121+
this.poetryProjects.add(basedir);
66122+
const poetryConfig = yield this.getPoetryConfiguration(basedir);
66123+
const cacheDir = poetryConfig['cache-dir'];
66124+
const virtualenvsPath = poetryConfig['virtualenvs.path'].replace('{cache-dir}', cacheDir);
66125+
paths.add(virtualenvsPath);
66126+
if (poetryConfig['virtualenvs.in-project']) {
66127+
paths.add(path.join(basedir, '.venv'));
66128+
}
6611366129
}
6611466130
}
66115-
else {
66116-
utils_1.logWarning('python binaries were not found in PATH');
66131+
catch (e_1_1) { e_1 = { error: e_1_1 }; }
66132+
finally {
66133+
try {
66134+
if (_c && !_c.done && (_a = _b.return)) yield _a.call(_b);
66135+
}
66136+
finally { if (e_1) throw e_1.error; }
6611766137
}
66118-
return paths;
66138+
return [...paths];
6611966139
});
6612066140
}
6612166141
computeKeys() {
6612266142
return __awaiter(this, void 0, void 0, function* () {
6612366143
const hash = yield glob.hashFiles(this.patterns);
66124-
const primaryKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-python-${this.pythonVersion}-${this.packageManager}-${hash}`;
66144+
// "v2" is here to invalidate old caches of this cache distributor, which were created broken:
66145+
const primaryKey = `${this.CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-python-${this.pythonVersion}-${this.packageManager}-v2-${hash}`;
6612566146
const restoreKey = undefined;
6612666147
return {
6612766148
primaryKey,
6612866149
restoreKey
6612966150
};
6613066151
});
6613166152
}
66132-
getPoetryConfiguration() {
66153+
handleLoadedCache() {
66154+
const _super = Object.create(null, {
66155+
handleLoadedCache: { get: () => super.handleLoadedCache }
66156+
});
66157+
return __awaiter(this, void 0, void 0, function* () {
66158+
yield _super.handleLoadedCache.call(this);
66159+
// After the cache is loaded -- make sure virtualenvs use the correct Python version (the one that we have just installed).
66160+
// This will handle invalid caches, recreating virtualenvs if necessary.
66161+
const pythonLocation = yield io.which('python');
66162+
if (pythonLocation) {
66163+
core.debug(`pythonLocation is ${pythonLocation}`);
66164+
}
66165+
else {
66166+
utils_1.logWarning('python binaries were not found in PATH');
66167+
return;
66168+
}
66169+
for (const poetryProject of this.poetryProjects) {
66170+
const { exitCode, stderr } = yield exec.getExecOutput('poetry', ['env', 'use', pythonLocation], { ignoreReturnCode: true, cwd: poetryProject });
66171+
if (exitCode) {
66172+
utils_1.logWarning(stderr);
66173+
}
66174+
}
66175+
});
66176+
}
66177+
getPoetryConfiguration(basedir) {
6613366178
return __awaiter(this, void 0, void 0, function* () {
66134-
const { stdout, stderr, exitCode } = yield exec.getExecOutput('poetry', [
66135-
'config',
66136-
'--list'
66137-
]);
66179+
const { stdout, stderr, exitCode } = yield exec.getExecOutput('poetry', ['config', '--list'], { cwd: basedir });
6613866180
if (exitCode && stderr) {
6613966181
throw new Error('Could not get cache folder path for poetry package manager');
6614066182
}

‎src/cache-distributions/cache-distributor.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ abstract class CacheDistributor {
1919
primaryKey: string;
2020
restoreKey: string[] | undefined;
2121
}>;
22+
protected async handleLoadedCache() {}
2223

2324
public async restoreCache() {
2425
const {primaryKey, restoreKey} = await this.computeKeys();
@@ -41,6 +42,8 @@ abstract class CacheDistributor {
4142
restoreKey
4243
);
4344

45+
await this.handleLoadedCache();
46+
4447
this.handleMatchResult(matchedKey, primaryKey);
4548
}
4649

0 commit comments

Comments
 (0)
0