Skip to content
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

Add support to .tool-version file format #588

Open
wants to merge 3 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
21 changes: 21 additions & 0 deletions __tests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as cache from '@actions/cache';
import * as core from '@actions/core';
import {
parsePythonVersionFile,
validateVersion,
validatePythonVersionFormatForPyPy,
isCacheFeatureAvailable
Expand All @@ -9,6 +10,26 @@ import {
jest.mock('@actions/cache');
jest.mock('@actions/core');

describe('parsePythonVersionFile', () => {
it('handle the content of a .python-version file', () => {
expect(parsePythonVersionFile('3.6')).toEqual('3.6');
});

it('trims extra spaces at the end of the content', () => {
expect(parsePythonVersionFile('3.7 ')).toEqual('3.7');
});

it('parses correctly the content of .tool-version files', () => {
expect(parsePythonVersionFile('python 3.7')).toEqual('3.7');
});

it('parses correctly pypy version content of the .tool-version files', () => {
expect(parsePythonVersionFile('python pypy3.9-7.3.10')).toEqual(
'pypy3.9-7.3.10'
);
});
});

describe('validatePythonVersionFormatForPyPy', () => {
it.each([
['3.6', true],
Expand Down
3 changes: 2 additions & 1 deletion docs/advanced-usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ jobs:

## Using the `python-version-file` input

`setup-python` action can read Python or PyPy version from a version file. `python-version-file` input is used for specifying the path to the version file. If the file that was supplied to `python-version-file` input doesn't exist, the action will fail with error.
The python-version-file input accepts a path to a file containing the version of Python to be used by a project, for example .python-version, or .tool-versions.
If both the python-version and the python-version-file inputs are provided then the python-version input is used.

>In case both `python-version` and `python-version-file` inputs are supplied, the `python-version-file` input will be ignored due to its lower priority.

Expand Down
19 changes: 11 additions & 8 deletions src/find-pypy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,17 @@ export async function findPyPyVersion(
));

if (!installDir) {
({installDir, resolvedPythonVersion, resolvedPyPyVersion} =
await pypyInstall.installPyPy(
pypyVersionSpec.pypyVersion,
pypyVersionSpec.pythonVersion,
architecture,
allowPreReleases,
releases
));
({
installDir,
resolvedPythonVersion,
resolvedPyPyVersion
} = await pypyInstall.installPyPy(
pypyVersionSpec.pypyVersion,
pypyVersionSpec.pythonVersion,
architecture,
allowPreReleases,
releases
));
}

const pipDir = IS_WINDOWS ? 'Scripts' : 'bin';
Expand Down
15 changes: 13 additions & 2 deletions src/setup-python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import * as path from 'path';
import * as os from 'os';
import fs from 'fs';
import {getCacheDistributor} from './cache-distributions/cache-factory';
import {isCacheFeatureAvailable, logWarning, IS_MAC} from './utils';
import {
parsePythonVersionFile,
isCacheFeatureAvailable,
logWarning,
IS_MAC
} from './utils';

function isPyPyVersion(versionSpec: string) {
return versionSpec.startsWith('pypy');
Expand Down Expand Up @@ -42,15 +47,21 @@ function resolveVersionInput() {
`The specified python version file at: ${versionFile} doesn't exist.`
);
}
const version = fs.readFileSync(versionFile, 'utf8');

const version = parsePythonVersionFile(
fs.readFileSync(versionFile, 'utf8')
);
core.info(`Resolved ${versionFile} as ${version}`);

return [version];
}

logWarning(
"Neither 'python-version' nor 'python-version-file' inputs were supplied. Attempting to find '.python-version' file."
);

versionFile = '.python-version';

if (fs.existsSync(versionFile)) {
const version = fs.readFileSync(versionFile, 'utf8');
core.info(`Resolved ${versionFile} as ${version}`);
Expand Down
16 changes: 16 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ export interface IPyPyManifestRelease {
files: IPyPyManifestAsset[];
}

export function parsePythonVersionFile(contents: string): string {
let pythonVersion: string | undefined;

// Try to find the version in tool-version file
const found = contents.match(/^(?:python\s+)?v?(?<version>[^\s]+)$/m);
Copy link
Contributor

@IvanZosimov IvanZosimov Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the regular expression we used in line 34 won't work with PyPy versions correctly. In the .tool-version, PyPy format doesn't have v: python pypy3.9-3.7.11, but the action's python-version input should include v before the PyPy version: python pypy3.9-v3.7.11. I think we can adapt the matched version of PyPy (basically just add v before the PyPy version) and add some e2e tests both for Python and PyPy installation from .tool-version.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I saw from pypy version of python in asdf-vm you have the following version

python pypy3.9-3.7.11

Is that not correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's correct version syntax for .tool-version file, but incorrect python-version syntax for setup-python. Check the supported syntax here and, please, pay attention to the letter v.

.tool-version content your regex result setup-python requirement
python pypy3.9-3.7.11 pypy3.9-3.7.11 pypy3.9-v3.7.11

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elpic may be could add a if/else condition here https://github.com/actions/setup-python/pull/588/files#diff-0ac8bdebd0ca13af3640499ecf1203abc825161011f13f9ca45c734bfe80128eR51 so that we parse format only for .tool-verions file

pythonVersion = found?.groups?.version;

// In the case of an unknown format,
// return as is and evaluate the version separately.
if (!pythonVersion) {
pythonVersion = contents.trim();
}

return pythonVersion as string;
}

/** create Symlinks for downloaded PyPy
* It should be executed only for downloaded versions in runtime, because
* toolcache versions have this setup.
Expand Down