From 6e222bc006315345603d242962ebd2155efeaa37 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Apr 2025 12:45:52 +0200 Subject: [PATCH 1/5] Adapt python-requires versions --- .vscode/settings.json | 3 +++ rsconnect/pyproject.py | 47 +++++++++++++++++++++++++++++--- tests/test_pyproject.py | 60 +++++++++++++++++++++++++++++++++++++---- 3 files changed, 102 insertions(+), 8 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 1055e1ec..6e1ef32f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -24,4 +24,7 @@ "build/**": true, "venv/**": true, }, + "python.analysis.exclude": [ + "tests" + ], } diff --git a/rsconnect/pyproject.py b/rsconnect/pyproject.py index 0e3c0adc..851f8bad 100644 --- a/rsconnect/pyproject.py +++ b/rsconnect/pyproject.py @@ -5,9 +5,10 @@ but not from setup.py due to its dynamic nature. """ +import configparser import pathlib +import re import typing -import configparser try: import tomllib @@ -15,6 +16,9 @@ # Python 3.11+ has tomllib in the standard library import toml as tomllib # type: ignore[no-redef] +PEP440_OPERATORS_REGEX = r"(===|==|!=|<=|>=|<|>|~=)" +VALID_VERSION_REQ_REGEX = rf"^({PEP440_OPERATORS_REGEX}?\d+(\.[\d\*]+)*)+$" + def detect_python_version_requirement(directory: typing.Union[str, pathlib.Path]) -> typing.Optional[str]: """Detect the python version requirement for a project. @@ -103,5 +107,42 @@ def parse_pyversion_python_requires(pyversion_file: pathlib.Path) -> typing.Opti Returns None if the field is not found. """ - content = pyversion_file.read_text() - return content.strip() + return adapt_python_requires(pyversion_file.read_text().strip()) + + +def adapt_python_requires( + python_requires: str, +) -> str: + """Convert a literal python version to a PEP440 constraint. + + Connect expects a PEP440 format, but the .python-version file can contain + plain version numbers and other formats. + + We should convert them to the constraints that connect expects. + """ + current_contraints = python_requires.split(",") + + def _adapt_contraint(constraints: list[str]) -> typing.Generator[str, None, None]: + for constraint in constraints: + constraint = constraint.strip() + if "@" in constraint or "-" in constraint or "/" in constraint: + raise ValueError( + f"Invalid python version, python specific implementations are not supported: {constraint}" + ) + + if "b" in constraint or "rc" in constraint or "a" in constraint: + raise ValueError(f"Invalid python version, pre-release versions are not supported: {constraint}") + + if re.match(VALID_VERSION_REQ_REGEX, constraint) is None: + raise ValueError(f"Invalid python version: {constraint}") + + if re.search(PEP440_OPERATORS_REGEX, constraint): + yield constraint + else: + # Convert to PEP440 format + if "*" in constraint: + yield f"=={constraint}" + else: + yield f"~={constraint.rstrip('0').rstrip('.')}" # Remove trailing zeros and dots + + return ",".join(_adapt_contraint(current_contraints)) diff --git a/tests/test_pyproject.py b/tests/test_pyproject.py index eb5b3f28..b95349e5 100644 --- a/tests/test_pyproject.py +++ b/tests/test_pyproject.py @@ -1,17 +1,18 @@ import os import pathlib +import tempfile + +import pytest from rsconnect.pyproject import ( + detect_python_version_requirement, + get_python_version_requirement_parser, lookup_metadata_file, parse_pyproject_python_requires, - parse_setupcfg_python_requires, parse_pyversion_python_requires, - get_python_version_requirement_parser, - detect_python_version_requirement, + parse_setupcfg_python_requires, ) -import pytest - HERE = os.path.dirname(__file__) PROJECTS_DIRECTORY = os.path.abspath(os.path.join(HERE, "testdata", "python-project")) @@ -142,3 +143,52 @@ def test_detect_python_version_requirement(): assert detect_python_version_requirement(project_dir) == ">=3.8, <3.12" assert detect_python_version_requirement(os.path.join(PROJECTS_DIRECTORY, "empty")) is None + + +@pytest.mark.parametrize( # type: ignore + ["content", "expected"], + [ + ("3.8", "~=3.8"), + ("3.8.0", "~=3.8"), + ("3.8.0b1", ValueError("Invalid python version, pre-release versions are not supported: 3.8.0b1")), + ("3.8.0rc1", ValueError("Invalid python version, pre-release versions are not supported: 3.8.0rc1")), + ("3.8.0a1", ValueError("Invalid python version, pre-release versions are not supported: 3.8.0a1")), + ("3.8.*", "==3.8.*"), + ("3.*", "==3.*"), + ("*", ValueError("Invalid python version: *")), + # This is not perfect, but the added regex complexity doesn't seem worth it. + ("invalid", ValueError("Invalid python version, pre-release versions are not supported: invalid")), + ("pypi@3.1", ValueError("Invalid python version, python specific implementations are not supported: pypi@3.1")), + ( + "cpython-3.12.3-macos-aarch64-none", + ValueError( + "Invalid python version, python specific implementations are not supported: " + "cpython-3.12.3-macos-aarch64-none" + ), + ), + ( + "/usr/bin/python3.8", + ValueError("Invalid python version, python specific implementations are not supported: /usr/bin/python3.8"), + ), + ], +) +def test_python_version_file_adapt(content, expected): + """Test that the python version is correctly converted to a PEP440 format. + + Connect expects a PEP440 format, but the .python-version file can contain + plain version numbers and other formats. + + We should convert them to the constraints that connect expects. + """ + with tempfile.NamedTemporaryFile(mode="w+") as tmpfile: + tmpfile.write(content) + tmpfile.flush() + + versionfile = pathlib.Path(tmpfile.name) + + if isinstance(expected, Exception): + with pytest.raises(expected.__class__) as excinfo: + parse_pyversion_python_requires(versionfile) + assert str(excinfo.value) == expected.args[0] + else: + assert parse_pyversion_python_requires(versionfile) == expected From e5ded034bba1cecf4c090abd0a5c97499f65eb9c Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Apr 2025 12:50:27 +0200 Subject: [PATCH 2/5] add comma and operation tests --- tests/test_pyproject.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_pyproject.py b/tests/test_pyproject.py index b95349e5..f50d1dee 100644 --- a/tests/test_pyproject.py +++ b/tests/test_pyproject.py @@ -170,6 +170,8 @@ def test_detect_python_version_requirement(): "/usr/bin/python3.8", ValueError("Invalid python version, python specific implementations are not supported: /usr/bin/python3.8"), ), + (">=3.8,<3.10", ">=3.8,<3.10"), + (">=3.8, <*", ValueError("Invalid python version: <*")), ], ) def test_python_version_file_adapt(content, expected): From 849fd47516a7d9284fb6be5c2cf54366038b553c Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Apr 2025 12:57:00 +0200 Subject: [PATCH 3/5] tweak test --- rsconnect/pyproject.py | 2 +- tests/test_environment.py | 4 ++-- tests/test_pyproject.py | 30 ++++++++++++++++++------------ 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/rsconnect/pyproject.py b/rsconnect/pyproject.py index 851f8bad..7ef5ac98 100644 --- a/rsconnect/pyproject.py +++ b/rsconnect/pyproject.py @@ -122,7 +122,7 @@ def adapt_python_requires( """ current_contraints = python_requires.split(",") - def _adapt_contraint(constraints: list[str]) -> typing.Generator[str, None, None]: + def _adapt_contraint(constraints: typing.List[str]) -> typing.Generator[str, None, None]: for constraint in constraints: constraint = constraint.strip() if "@" in constraint or "-" in constraint or "/" in constraint: diff --git a/tests/test_environment.py b/tests/test_environment.py index d62befc1..1a1c5a95 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -143,12 +143,12 @@ def test_pyproject_toml(self): def test_python_version(self): env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "using_pyversion")) assert env.python_interpreter == sys.executable - assert env.python_version_requirement == ">=3.8, <3.12" + assert env.python_version_requirement == ">=3.8,<3.12" def test_all_of_them(self): env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "allofthem")) assert env.python_interpreter == sys.executable - assert env.python_version_requirement == ">=3.8, <3.12" + assert env.python_version_requirement == ">=3.8,<3.12" def test_missing(self): env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "empty")) diff --git a/tests/test_pyproject.py b/tests/test_pyproject.py index f50d1dee..ff68a315 100644 --- a/tests/test_pyproject.py +++ b/tests/test_pyproject.py @@ -118,7 +118,7 @@ def test_setupcfg_python_requires(project_dir, expected): @pytest.mark.parametrize( "project_dir, expected", [ - (os.path.join(PROJECTS_DIRECTORY, "using_pyversion"), ">=3.8, <3.12"), + (os.path.join(PROJECTS_DIRECTORY, "using_pyversion"), ">=3.8,<3.12"), ], ids=["option-exists"], ) @@ -140,7 +140,7 @@ def test_detect_python_version_requirement(): version requirement is used. """ project_dir = os.path.join(PROJECTS_DIRECTORY, "allofthem") - assert detect_python_version_requirement(project_dir) == ">=3.8, <3.12" + assert detect_python_version_requirement(project_dir) == ">=3.8,<3.12" assert detect_python_version_requirement(os.path.join(PROJECTS_DIRECTORY, "empty")) is None @@ -182,15 +182,21 @@ def test_python_version_file_adapt(content, expected): We should convert them to the constraints that connect expects. """ - with tempfile.NamedTemporaryFile(mode="w+") as tmpfile: + # delete=False is necessary on windows to reopen the file. + # Otherwise it can't be opened again. + # Ideally delete_on_close=False does what we need, but it's only >=3.12 + with tempfile.NamedTemporaryFile(mode="w+", delete=False) as tmpfile: tmpfile.write(content) tmpfile.flush() - - versionfile = pathlib.Path(tmpfile.name) - - if isinstance(expected, Exception): - with pytest.raises(expected.__class__) as excinfo: - parse_pyversion_python_requires(versionfile) - assert str(excinfo.value) == expected.args[0] - else: - assert parse_pyversion_python_requires(versionfile) == expected + tmpfile.close() # Also needed for windows to allow subsequent reading. + + try: + versionfile = pathlib.Path(tmpfile.name) + if isinstance(expected, Exception): + with pytest.raises(expected.__class__) as excinfo: + parse_pyversion_python_requires(versionfile) + assert str(excinfo.value) == expected.args[0] + else: + assert parse_pyversion_python_requires(versionfile) == expected + finally: + os.remove(tmpfile.name) From 091a01eb3ee39d6e091625cdce1d610c4226c367 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Apr 2025 17:21:12 +0200 Subject: [PATCH 4/5] log invalid versions but continue --- rsconnect/pyproject.py | 22 +++++++++++++++++----- tests/test_pyproject.py | 34 ++++++++++++++++------------------ 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/rsconnect/pyproject.py b/rsconnect/pyproject.py index 7ef5ac98..b8f714e9 100644 --- a/rsconnect/pyproject.py +++ b/rsconnect/pyproject.py @@ -16,6 +16,9 @@ # Python 3.11+ has tomllib in the standard library import toml as tomllib # type: ignore[no-redef] +from .log import logger + + PEP440_OPERATORS_REGEX = r"(===|==|!=|<=|>=|<|>|~=)" VALID_VERSION_REQ_REGEX = rf"^({PEP440_OPERATORS_REGEX}?\d+(\.[\d\*]+)*)+$" @@ -30,7 +33,12 @@ def detect_python_version_requirement(directory: typing.Union[str, pathlib.Path] """ for _, metadata_file in lookup_metadata_file(directory): parser = get_python_version_requirement_parser(metadata_file) - version_constraint = parser(metadata_file) + try: + version_constraint = parser(metadata_file) + except InvalidVersionConstraintError as err: + logger.error(f"Invalid python version constraint in {metadata_file}, ignoring it: {err}") + continue + if version_constraint: return version_constraint @@ -126,15 +134,15 @@ def _adapt_contraint(constraints: typing.List[str]) -> typing.Generator[str, Non for constraint in constraints: constraint = constraint.strip() if "@" in constraint or "-" in constraint or "/" in constraint: - raise ValueError( - f"Invalid python version, python specific implementations are not supported: {constraint}" + raise InvalidVersionConstraintError( + f"python specific implementations are not supported: {constraint}" ) if "b" in constraint or "rc" in constraint or "a" in constraint: - raise ValueError(f"Invalid python version, pre-release versions are not supported: {constraint}") + raise InvalidVersionConstraintError(f"pre-release versions are not supported: {constraint}") if re.match(VALID_VERSION_REQ_REGEX, constraint) is None: - raise ValueError(f"Invalid python version: {constraint}") + raise InvalidVersionConstraintError(f"Invalid python version: {constraint}") if re.search(PEP440_OPERATORS_REGEX, constraint): yield constraint @@ -146,3 +154,7 @@ def _adapt_contraint(constraints: typing.List[str]) -> typing.Generator[str, Non yield f"~={constraint.rstrip('0').rstrip('.')}" # Remove trailing zeros and dots return ",".join(_adapt_contraint(current_contraints)) + + +class InvalidVersionConstraintError(ValueError): + pass diff --git a/tests/test_pyproject.py b/tests/test_pyproject.py index ff68a315..5ee2794e 100644 --- a/tests/test_pyproject.py +++ b/tests/test_pyproject.py @@ -11,6 +11,7 @@ parse_pyproject_python_requires, parse_pyversion_python_requires, parse_setupcfg_python_requires, + InvalidVersionConstraintError ) HERE = os.path.dirname(__file__) @@ -150,25 +151,24 @@ def test_detect_python_version_requirement(): [ ("3.8", "~=3.8"), ("3.8.0", "~=3.8"), - ("3.8.0b1", ValueError("Invalid python version, pre-release versions are not supported: 3.8.0b1")), - ("3.8.0rc1", ValueError("Invalid python version, pre-release versions are not supported: 3.8.0rc1")), - ("3.8.0a1", ValueError("Invalid python version, pre-release versions are not supported: 3.8.0a1")), + ("3.8.0b1", InvalidVersionConstraintError("pre-release versions are not supported: 3.8.0b1")), + ("3.8.0rc1", InvalidVersionConstraintError("pre-release versions are not supported: 3.8.0rc1")), + ("3.8.0a1", InvalidVersionConstraintError("pre-release versions are not supported: 3.8.0a1")), ("3.8.*", "==3.8.*"), ("3.*", "==3.*"), - ("*", ValueError("Invalid python version: *")), + ("*", InvalidVersionConstraintError("Invalid python version: *")), # This is not perfect, but the added regex complexity doesn't seem worth it. - ("invalid", ValueError("Invalid python version, pre-release versions are not supported: invalid")), - ("pypi@3.1", ValueError("Invalid python version, python specific implementations are not supported: pypi@3.1")), + ("invalid", InvalidVersionConstraintError("pre-release versions are not supported: invalid")), + ("pypi@3.1", InvalidVersionConstraintError("python specific implementations are not supported: pypi@3.1")), ( "cpython-3.12.3-macos-aarch64-none", - ValueError( - "Invalid python version, python specific implementations are not supported: " - "cpython-3.12.3-macos-aarch64-none" + InvalidVersionConstraintError( + "python specific implementations are not supported: cpython-3.12.3-macos-aarch64-none" ), ), ( "/usr/bin/python3.8", - ValueError("Invalid python version, python specific implementations are not supported: /usr/bin/python3.8"), + InvalidVersionConstraintError("python specific implementations are not supported: /usr/bin/python3.8"), ), (">=3.8,<3.10", ">=3.8,<3.10"), (">=3.8, <*", ValueError("Invalid python version: <*")), @@ -182,21 +182,19 @@ def test_python_version_file_adapt(content, expected): We should convert them to the constraints that connect expects. """ - # delete=False is necessary on windows to reopen the file. - # Otherwise it can't be opened again. - # Ideally delete_on_close=False does what we need, but it's only >=3.12 - with tempfile.NamedTemporaryFile(mode="w+", delete=False) as tmpfile: - tmpfile.write(content) - tmpfile.flush() - tmpfile.close() # Also needed for windows to allow subsequent reading. + with tempfile.TemporaryDirectory() as tmpdir: + versionfile = pathlib.Path(tmpdir) / ".python-version" + with open(versionfile, "w") as tmpfile: + tmpfile.write(content) try: - versionfile = pathlib.Path(tmpfile.name) if isinstance(expected, Exception): with pytest.raises(expected.__class__) as excinfo: parse_pyversion_python_requires(versionfile) assert str(excinfo.value) == expected.args[0] + assert detect_python_version_requirement(tmpdir) is None else: assert parse_pyversion_python_requires(versionfile) == expected + assert detect_python_version_requirement(tmpdir) == expected finally: os.remove(tmpfile.name) From 5777198c3bfd3c3215d15b683d6f8abfbde96039 Mon Sep 17 00:00:00 2001 From: Alessandro Molina Date: Mon, 28 Apr 2025 17:43:54 +0200 Subject: [PATCH 5/5] format --- rsconnect/pyproject.py | 4 +--- tests/test_pyproject.py | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/rsconnect/pyproject.py b/rsconnect/pyproject.py index b8f714e9..e52c6b7e 100644 --- a/rsconnect/pyproject.py +++ b/rsconnect/pyproject.py @@ -134,9 +134,7 @@ def _adapt_contraint(constraints: typing.List[str]) -> typing.Generator[str, Non for constraint in constraints: constraint = constraint.strip() if "@" in constraint or "-" in constraint or "/" in constraint: - raise InvalidVersionConstraintError( - f"python specific implementations are not supported: {constraint}" - ) + raise InvalidVersionConstraintError(f"python specific implementations are not supported: {constraint}") if "b" in constraint or "rc" in constraint or "a" in constraint: raise InvalidVersionConstraintError(f"pre-release versions are not supported: {constraint}") diff --git a/tests/test_pyproject.py b/tests/test_pyproject.py index 5ee2794e..58e3f643 100644 --- a/tests/test_pyproject.py +++ b/tests/test_pyproject.py @@ -11,7 +11,7 @@ parse_pyproject_python_requires, parse_pyversion_python_requires, parse_setupcfg_python_requires, - InvalidVersionConstraintError + InvalidVersionConstraintError, ) HERE = os.path.dirname(__file__)