From 4103f6657e38b5a0cf278f15097818efd669efcf Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 12:10:55 -0700 Subject: [PATCH 01/12] Rework pyproject.toml parsing to never raise This is super messy and needs a refactor. --- src/incremental/__init__.py | 83 ++++++++++++++++++------- src/incremental/_hatch.py | 3 +- src/incremental/tests/test_pyproject.py | 66 ++++++++++++++------ 3 files changed, 110 insertions(+), 42 deletions(-) diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index 7a29fd7..b95c464 100644 --- a/src/incremental/__init__.py +++ b/src/incremental/__init__.py @@ -392,15 +392,20 @@ def _get_setuptools_version(dist): # type: (_Distribution) -> None Setuptools integration: load the version from the working directory This function is registered as a setuptools.finalize_distribution_options - entry point [1]. It is a no-op unless there is a pyproject.toml containing - an empty [tool.incremental] section. + entry point [1]. Consequently, it is called in all sorts of weird + contexts, so it strives to not raise unless there is a pyproject.toml + containing a [tool.incremental] section. - @param dist: An empty C{setuptools.Distribution} instance to mutate. + @param dist: + A (possibly) empty C{setuptools.Distribution} instance to mutate. + There may be some metadata here if a `setup.py` called `setup()`, + but this hook is always called before setuptools loads anything + from ``pyproject.toml``. [1]: https://setuptools.pypa.io/en/latest/userguide/extension.html#customizing-distribution-options """ - config = _load_pyproject_toml("./pyproject.toml") - if not config or not config.has_tool_incremental: + config = _load_pyproject_toml("./pyproject.toml", opt_in=False) + if not config or not config.opt_in: return dist.metadata.version = _existing_version(config.path).public() @@ -451,7 +456,7 @@ class _IncrementalConfig: Configuration loaded from a ``pyproject.toml`` file. """ - has_tool_incremental: bool + opt_in: bool """ Does the pyproject.toml file contain a [tool.incremental] section? This indicates that the package has explicitly @@ -465,7 +470,7 @@ class _IncrementalConfig: """Path to the package root""" -def _load_pyproject_toml(toml_path): # type: (str) -> Optional[_IncrementalConfig] +def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_IncrementalConfig] """ Load Incremental configuration from a ``pyproject.toml`` @@ -473,19 +478,37 @@ def _load_pyproject_toml(toml_path): # type: (str) -> Optional[_IncrementalConf from the [project] section. Otherwise we require only a C{name} key specifying the project name. Other keys are forbidden to allow future extension and catch typos. + + @param toml_path: + Path to the ``pyproject.toml`` to load. + + @param opt_in: + Are we operating in a context where Incremental has been + affirmatively requested? + + Otherwise we do our best to *never* raise an exception until we + find a ``[tool.incremental]`` opt-in. This is useful in a setuptools + context because """ try: with open(toml_path, "rb") as f: data = _load_toml(f) - except FileNotFoundError: + except Exception: + if opt_in: + raise return None - tool_incremental = _extract_tool_incremental(data) + tool_incremental = _extract_tool_incremental(data, opt_in) + + # Do we have an affirmative opt-in to use Incremental? Otherwise + # we must *never* raise exceptions. + opt_in = opt_in or tool_incremental is not None package = None if tool_incremental is not None and "name" in tool_incremental: package = tool_incremental["name"] if package is None: + # Try to fall back to [project] try: package = data["project"]["name"] except KeyError: @@ -493,10 +516,9 @@ def _load_pyproject_toml(toml_path): # type: (str) -> Optional[_IncrementalConf if package is None: # We can't proceed without a project name, but that's only an error # if [tool.incremental] is present. - if tool_incremental is None: - return None - raise ValueError("""\ -Couldn't extract the package name from pyproject.toml. Specify it like: + if opt_in: + raise ValueError("""\ +Incremental faied to extract the package name from pyproject.toml. Specify it like: [project] name = "Foo" @@ -507,30 +529,49 @@ def _load_pyproject_toml(toml_path): # type: (str) -> Optional[_IncrementalConf name = "Foo" """) + else: + return None + if not isinstance(package, str): - raise TypeError( - "Package name must be a string, but found {}".format(type(package)) - ) + if opt_in: + raise TypeError( + "Package name must be a string, but found {}".format(type(package)) + ) + else: + return None + + try: + path = _findPath(os.path.dirname(toml_path), package) + except ValueError: + if opt_in: + raise + else: + return None return _IncrementalConfig( - has_tool_incremental=tool_incremental is not None, + opt_in=opt_in, package=package, - path=_findPath(os.path.dirname(toml_path), package), + path=path, ) -def _extract_tool_incremental(data): # type: (Dict[str, object]) -> Optional[Dict[str, object]] +def _extract_tool_incremental(data, opt_in): # type: (Dict[str, object], bool) -> Optional[Dict[str, object]] if "tool" not in data: return None if not isinstance(data["tool"], dict): - raise ValueError("[tool] must be a table") + if opt_in: + raise ValueError("[tool] must be a table") + return None if "incremental" not in data["tool"]: return None tool_incremental = data["tool"]["incremental"] if not isinstance(tool_incremental, dict): - raise ValueError("[tool.incremental] must be a table") + if opt_in: + raise ValueError("[tool.incremental] must be a table") + return None + # At this point we've found a [tool.incremental] table, so we have opt_in if not {"name"}.issuperset(tool_incremental.keys()): raise ValueError("Unexpected key(s) in [tool.incremental]") return tool_incremental diff --git a/src/incremental/_hatch.py b/src/incremental/_hatch.py index 8d677c4..0305073 100644 --- a/src/incremental/_hatch.py +++ b/src/incremental/_hatch.py @@ -20,7 +20,8 @@ class IncrementalVersionSource(VersionSourceInterface): def get_version_data(self) -> _VersionData: # type: ignore[override] path = os.path.join(self.root, "./pyproject.toml") - config = _load_pyproject_toml(path) + # If the Hatch plugin is running at all we've already opted in. + config = _load_pyproject_toml(path, opt_in=True) assert config is not None, "Failed to read {}".format(path) return {"version": _existing_version(config.path).public()} diff --git a/src/incremental/tests/test_pyproject.py b/src/incremental/tests/test_pyproject.py index 036f5a0..998e0b3 100644 --- a/src/incremental/tests/test_pyproject.py +++ b/src/incremental/tests/test_pyproject.py @@ -15,7 +15,7 @@ class VerifyPyprojectDotTomlTests(TestCase): """Test the `_load_pyproject_toml` helper function""" def _loadToml( - self, toml: str, *, path: Union[Path, str, None] = None + self, toml: str, opt_in: bool, *, path: Union[Path, str, None] = None ) -> Optional[_IncrementalConfig]: """ Read a TOML snipped from a temporary file with `_load_pyproject_toml` @@ -34,7 +34,7 @@ def _loadToml( f.write(toml) try: - return _load_pyproject_toml(path_) + return _load_pyproject_toml(path_, opt_in) except Exception as e: if hasattr(e, "add_note"): e.add_note( # type: ignore[attr-defined] @@ -44,48 +44,72 @@ def _loadToml( def test_fileNotFound(self): """ - An absent ``pyproject.toml`` file produces no result + An absent ``pyproject.toml`` file produces no result unless + there is opt-in. """ path = os.path.join(cast(str, self.mktemp()), "pyproject.toml") - self.assertIsNone(_load_pyproject_toml(path)) + self.assertIsNone(_load_pyproject_toml(path, False)) + self.assertRaises(FileNotFoundError, _load_pyproject_toml, path, True) + + def test_brokenToml(self): + """ + Syntactially invalid TOML is ignored unless there's an opt-in. + """ + toml = '[project]\nname = "abc' # truncated + + self.assertIsNone(self._loadToml(toml, False)) + self.assertRaises(Exception, self._loadToml, toml, True) def test_configMissing(self): """ A ``pyproject.toml`` that exists but provides no relevant configuration - is ignored. + is ignored unless opted in. """ for toml in [ "\n", "[tool.notincremental]\n", "[project]\n", ]: - self.assertIsNone(self._loadToml(toml)) + self.assertIsNone(self._loadToml(toml, False)) def test_nameMissing(self): """ `ValueError` is raised when ``[tool.incremental]`` is present but - the project name isn't available. + the project name isn't available. The ``[tool.incremental]`` + section counts as opt-in. """ for toml in [ "[tool.incremental]\n", "[project]\n[tool.incremental]\n", ]: - self.assertRaises(ValueError, self._loadToml, toml) + self.assertRaises(ValueError, self._loadToml, toml, False) + self.assertRaises(ValueError, self._loadToml, toml, True) + + def test_nameInvalidNoOptIn(self): + """ + An invalid project name is ignored when there's no opt-in. + """ + self.assertIsNone( + self._loadToml("[project]\nname = false\n", False), + ) - def test_nameInvalid(self): + def test_nameInvalidOptIn(self): """ - `TypeError` is raised when the project name isn't a string. + Once opted in, `TypeError` is raised when the project name + isn't a string. """ for toml in [ + "[project]\nname = false\n", "[tool.incremental]\nname = -1\n", "[tool.incremental]\n[project]\nname = 1.0\n", ]: - self.assertRaises(TypeError, self._loadToml, toml) + self.assertRaises(TypeError, self._loadToml, toml, True) def test_toolIncrementalInvalid(self): """ - `ValueError` is raised when the ``[tool]`` or ``[tool.incremental]`` - isn't a table. + When ``[tool]`` or ``[tool.incremental]`` isn't a table the + ``pyproject.toml`` it's an error if opted-in, otherwise the + file is ignored. """ for toml in [ "tool = false\n", @@ -93,7 +117,8 @@ def test_toolIncrementalInvalid(self): "[tool]\nincremental = 123\n", "[tool]\nincremental = null\n", ]: - self.assertRaises(ValueError, self._loadToml, toml) + self.assertIsNone(self._loadToml(toml, False)) + self.assertRaises(ValueError, self._loadToml, toml, True) def test_toolIncrementalUnexpecteKeys(self): """ @@ -104,7 +129,7 @@ def test_toolIncrementalUnexpecteKeys(self): "[tool.incremental]\nfoo = false\n", '[tool.incremental]\nname = "OK"\nother = false\n', ]: - self.assertRaises(ValueError, self._loadToml, toml) + self.assertRaises(ValueError, self._loadToml, toml, False) def test_setuptoolsOptIn(self): """ @@ -120,12 +145,12 @@ def test_setuptoolsOptIn(self): '[project]\nname = "Foo"\n[tool.incremental]\n', '[tool.incremental]\nname = "Foo"\n', ]: - config = self._loadToml(toml, path=root / "pyproject.toml") + config = self._loadToml(toml, False, path=root / "pyproject.toml") self.assertEqual( config, _IncrementalConfig( - has_tool_incremental=True, + opt_in=True, package="Foo", path=str(pkg), ), @@ -133,8 +158,8 @@ def test_setuptoolsOptIn(self): def test_noToolIncrementalSection(self): """ - The ``has_tool_incremental`` flag is false when there - isn't a ``[tool.incremental]`` section. + The ``opt_in`` flag is false when there isn't a + ``[tool.incremental]`` section. """ root = Path(self.mktemp()) pkg = root / "foo" @@ -142,13 +167,14 @@ def test_noToolIncrementalSection(self): config = self._loadToml( '[project]\nname = "foo"\n', + opt_in=False, path=root / "pyproject.toml", ) self.assertEqual( config, _IncrementalConfig( - has_tool_incremental=False, + opt_in=False, package="foo", path=str(pkg), ), From f821c9a149278e195d1bed8fb2f853a3d64d1a1b Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 12:53:44 -0700 Subject: [PATCH 02/12] Fix docstring syntax --- src/incremental/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index b95c464..12ec2fa 100644 --- a/src/incremental/__init__.py +++ b/src/incremental/__init__.py @@ -396,13 +396,13 @@ def _get_setuptools_version(dist): # type: (_Distribution) -> None contexts, so it strives to not raise unless there is a pyproject.toml containing a [tool.incremental] section. + [1]: https://setuptools.pypa.io/en/latest/userguide/extension.html#customizing-distribution-options + @param dist: A (possibly) empty C{setuptools.Distribution} instance to mutate. There may be some metadata here if a `setup.py` called `setup()`, but this hook is always called before setuptools loads anything from ``pyproject.toml``. - - [1]: https://setuptools.pypa.io/en/latest/userguide/extension.html#customizing-distribution-options """ config = _load_pyproject_toml("./pyproject.toml", opt_in=False) if not config or not config.opt_in: From 47af87a9fded1eaf58ee4515781798ba2e88ba07 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 13:30:01 -0700 Subject: [PATCH 03/12] Simplify --- src/incremental/__init__.py | 33 ++++++------------- src/incremental/tests/test_pyproject.py | 44 ++++++++++++++++++++++--- 2 files changed, 49 insertions(+), 28 deletions(-) diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index 12ec2fa..7a84a9e 100644 --- a/src/incremental/__init__.py +++ b/src/incremental/__init__.py @@ -503,7 +503,10 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I # Do we have an affirmative opt-in to use Incremental? Otherwise # we must *never* raise exceptions. opt_in = opt_in or tool_incremental is not None + if not opt_in: + return None + # Extract the project name package = None if tool_incremental is not None and "name" in tool_incremental: package = tool_incremental["name"] @@ -514,11 +517,9 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I except KeyError: pass if package is None: - # We can't proceed without a project name, but that's only an error - # if [tool.incremental] is present. - if opt_in: - raise ValueError("""\ -Incremental faied to extract the package name from pyproject.toml. Specify it like: + # We can't proceed without a project name. + raise ValueError("""\ +Incremental failed to extract the package name from pyproject.toml. Specify it like: [project] name = "Foo" @@ -529,29 +530,15 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I name = "Foo" """) - else: - return None - if not isinstance(package, str): - if opt_in: - raise TypeError( - "Package name must be a string, but found {}".format(type(package)) - ) - else: - return None - - try: - path = _findPath(os.path.dirname(toml_path), package) - except ValueError: - if opt_in: - raise - else: - return None + raise TypeError( + "Package name must be a string, but found {}".format(type(package)) + ) return _IncrementalConfig( opt_in=opt_in, package=package, - path=path, + path=_findPath(os.path.dirname(toml_path), package), ) diff --git a/src/incremental/tests/test_pyproject.py b/src/incremental/tests/test_pyproject.py index 998e0b3..939d8b8 100644 --- a/src/incremental/tests/test_pyproject.py +++ b/src/incremental/tests/test_pyproject.py @@ -158,11 +158,13 @@ def test_setuptoolsOptIn(self): def test_noToolIncrementalSection(self): """ - The ``opt_in`` flag is false when there isn't a - ``[tool.incremental]`` section. + We don't produce config unless we find opt-in. + + The ``[project]`` section doesn't imply opt-in, even if we can + recover the project name from it. """ root = Path(self.mktemp()) - pkg = root / "foo" + pkg = root / "foo" # A valid package directory. pkg.mkdir(parents=True) config = self._loadToml( @@ -171,11 +173,43 @@ def test_noToolIncrementalSection(self): path=root / "pyproject.toml", ) + self.assertIsNone(config) + + def test_pathNotFoundOptIn(self): + """ + Once opted in, raise `ValueError` when the package root can't + be resolved. + """ + root = Path(self.mktemp()) + root.mkdir() # Contains no package directory. + + with self.assertRaisesRegex(ValueError, "Can't find the directory of package"): + self._loadToml( + '[project]\nname = "foo"\n', + opt_in=True, + path=root / "pyproject.toml", + ) + + def test_noToolIncrementalSectionOptIn(self): + """ + If opted in (i.e. in the Hatch plugin) then the [tool.incremental] + table is completely optional. + """ + root = Path(self.mktemp()) + pkg = root / "src" / "foo" + pkg.mkdir(parents=True) + + config = self._loadToml( + '[project]\nname = "Foo"\n', + opt_in=True, + path=root / "pyproject.toml", + ) + self.assertEqual( config, _IncrementalConfig( - opt_in=False, - package="foo", + opt_in=True, + package="Foo", path=str(pkg), ), ) From 6bfc5ba0f8a1d928ae4c9066f3518d58a85c7fb7 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 14:59:52 -0700 Subject: [PATCH 04/12] Restore coverage from the examples The examples seemed to be running against a different version of Incremental (possibly from pip's cache?). Ensure they run against exactly the version of Incremental we're trying to test against by pre-installing it in an isolated environment. Since this introduces an isolated environment, also install coverage-p to enable coverage of the subprocess. --- .coveragerc | 9 +++------ tests/test_examples.py | 29 ++++++++++++++++++++++++----- tox.ini | 5 +++-- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/.coveragerc b/.coveragerc index bfe9d69..d905bf2 100644 --- a/.coveragerc +++ b/.coveragerc @@ -5,15 +5,12 @@ source_pkgs = incremental source = tests branch = True parallel = True -# This must be an absolute path because the example tests -# run Python processes with alternative working directories. -data_file = ${TOX_INI_DIR-.}/.coverage [paths] source = - src/ - .tox/*/lib/python*/site-packages/ - .tox/pypy*/site-packages/ + src/incremental + */src/incremental + */site-packages/incremental [report] exclude_lines = diff --git a/tests/test_examples.py b/tests/test_examples.py index f9ad2c8..efe8be0 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -8,8 +8,10 @@ import os from importlib import metadata from subprocess import run +from tempfile import TemporaryDirectory from build import ProjectBuilder +from build.env import DefaultIsolatedEnv from twisted.python.filepath import FilePath from twisted.trial.unittest import TestCase @@ -20,11 +22,28 @@ def build_and_install(path): # type: (FilePath) -> None - builder = ProjectBuilder(path.path) - pkgfile = builder.build("wheel", output_directory=os.environ["PIP_FIND_LINKS"]) - - # Force reinstall in case tox reused the venv. - run(["pip", "install", "--force-reinstall", pkgfile], check=True) + with TemporaryDirectory(prefix="dist") as dist_dir: + with DefaultIsolatedEnv(installer="pip") as env: + env.install( + { + # Install the *exact* version of Incremental under test. + # Otherwise pip might select a different version from + # its cache. + # + # These are formally PEP 508 markers, so we pass a + # file URL. + "incremental @ file://" + os.environ["TOX_PACKAGE"], + # A .pth file so that subprocess generate coverage. + "coverage-p", + } + ) + builder = ProjectBuilder.from_isolated_env(env, path.path) + env.install(builder.build_system_requires) + env.install(builder.get_requires_for_build("wheel", {})) + pkgfile = builder.build("wheel", output_directory=dist_dir) + + # Force reinstall in case tox reused the venv. + run(["pip", "install", "--force-reinstall", pkgfile], check=True) class ExampleTests(TestCase): diff --git a/tox.ini b/tox.ini index f0f662b..d6437e2 100644 --- a/tox.ini +++ b/tox.ini @@ -30,9 +30,10 @@ setenv = ; Suppress pointless chatter in the output. PIP_DISABLE_PIP_VERSION_CHECK=yes - tests: TOX_INI_DIR={toxinidir} tests: COVERAGE_PROCESS_START={toxinidir}/.coveragerc - tests: PIP_FIND_LINKS={env:PIP_FIND_LINKS:{distdir}} + ; This must be an absolute path because the example tests + ; run Python processes with alternative working directories. + tests: COVERAGE_FILE={toxinidir}/.coverage commands = python -V From b5ad0545b572ede441d73c35d8ff882f430e64d1 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 15:10:42 -0700 Subject: [PATCH 05/12] 100% coverage, why not? --- src/incremental/tests/test_version.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/incremental/tests/test_version.py b/src/incremental/tests/test_version.py index da66503..b9bd2a4 100644 --- a/src/incremental/tests/test_version.py +++ b/src/incremental/tests/test_version.py @@ -282,14 +282,24 @@ def test_comparingDevAndRCDifferent(self): self.assertTrue(vb == Version("whatever", 1, 0, 0, release_candidate=2, dev=1)) self.assertTrue(va == va) - def test_infComparison(self): + def test_infComparisonSelf(self): """ L{_inf} is equal to L{_inf}. This is a regression test. """ - o = object() self.assertEqual(_inf, _inf) + self.assertFalse(_inf < _inf) + self.assertFalse(_inf > _inf) + self.assertTrue(_inf >= _inf) + self.assertTrue(_inf <= _inf) + self.assertFalse(_inf != _inf) + + def test_infComparison(self): + """ + L{_inf} is greater than any other object. + """ + o = object() self.assertTrue(_inf > o) self.assertFalse(_inf < o) self.assertTrue(_inf >= o) From 4adf23bb48cfbc35e75d44c4863b2dc73d4e9563 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 15:32:13 -0700 Subject: [PATCH 06/12] Add newsfragment --- src/incremental/newsfragments/106.bugfix | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 src/incremental/newsfragments/106.bugfix diff --git a/src/incremental/newsfragments/106.bugfix b/src/incremental/newsfragments/106.bugfix new file mode 100644 index 0000000..63749ba --- /dev/null +++ b/src/incremental/newsfragments/106.bugfix @@ -0,0 +1,3 @@ +Incremental could mis-identify that a project had opted in to version management. + +If a ``pyproject.toml`` in the current directory contained a ``[project]`` table with a ``name`` key, but did not contain the opt-in ``[tool.incremental]`` table, Incremental would still treat the file as if the opt-in were present and attempt to validate the configuration. This could happen in contexts outside of packaging, such as when creating a virtualenv. When operating as a setuptools plugin Incremental now always requires the ``[tool.incremental]`` opt-in. Additionally, it suppresses any exceptions that occur while attempting to read ``pyproject.toml`` until it finds a valid ``[tool.incremental]`` table. From 0f7001c2bf5cc3d24f6d273067b2e88866bff6d1 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 15:49:43 -0700 Subject: [PATCH 07/12] Tidy up some comments --- src/incremental/__init__.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index 7a84a9e..d42e4d5 100644 --- a/src/incremental/__init__.py +++ b/src/incremental/__init__.py @@ -404,6 +404,8 @@ def _get_setuptools_version(dist): # type: (_Distribution) -> None but this hook is always called before setuptools loads anything from ``pyproject.toml``. """ + # When operating in a packaging context (i.e. building an sdist or wheel) + # pyproject.toml will always be found in the current working directory. config = _load_pyproject_toml("./pyproject.toml", opt_in=False) if not config or not config.opt_in: return @@ -487,8 +489,10 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I affirmatively requested? Otherwise we do our best to *never* raise an exception until we - find a ``[tool.incremental]`` opt-in. This is useful in a setuptools - context because + find a ``[tool.incremental]`` opt-in. This is important when + operating within a setuptools entry point because those hooks + are invoked anytime the L{Distribution} class is initialized, + which happens in non-packaging contexts that don't match the """ try: with open(toml_path, "rb") as f: @@ -500,8 +504,7 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I tool_incremental = _extract_tool_incremental(data, opt_in) - # Do we have an affirmative opt-in to use Incremental? Otherwise - # we must *never* raise exceptions. + # Do we have an affirmative opt-in to use Incremental? opt_in = opt_in or tool_incremental is not None if not opt_in: return None From 11ad4133e2857b37f140bfee835135f8cf1b6ad0 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 15:57:30 -0700 Subject: [PATCH 08/12] Update the readme --- README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 56baba2..0d74e8a 100644 --- a/README.rst +++ b/README.rst @@ -24,7 +24,7 @@ Add Incremental to your ``pyproject.toml``: [build-system] requires = [ "setuptools", - "incremental>=24.7.0", # ← Add incremental as a build dependency + "incremental>=24.7.2", # ← Add incremental as a build dependency ] build-backend = "setuptools.build_meta" @@ -32,7 +32,7 @@ Add Incremental to your ``pyproject.toml``: name = "" dynamic = ["version"] # ← Mark the version dynamic dependencies = [ - "incremental>=24.7.0", # ← Depend on incremental at runtime + "incremental>=24.7.2", # ← Depend on incremental at runtime ] # ... @@ -55,7 +55,7 @@ activate Incremental's Hatchling plugin by altering your ``pyproject.toml``: [build-system] requires = [ "hatchling", - "incremental>=24.7.0", # ← Add incremental as a build dependency + "incremental>=24.7.2", # ← Add incremental as a build dependency ] build-backend = "hatchling.build" @@ -63,7 +63,7 @@ activate Incremental's Hatchling plugin by altering your ``pyproject.toml``: name = "" dynamic = ["version"] # ← Mark the version dynamic dependencies = [ - "incremental>=24.7.0", # ← Depend on incremental at runtime + "incremental>=24.7.2", # ← Depend on incremental at runtime ] # ... From d659ea0ed0f8518608f40065aa5e6ad6dd2a69be Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 16:37:24 -0700 Subject: [PATCH 09/12] There and back again --- src/incremental/__init__.py | 59 ++++++------------ src/incremental/_hatch.py | 2 +- src/incremental/tests/test_pyproject.py | 83 ++++++------------------- 3 files changed, 39 insertions(+), 105 deletions(-) diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index d42e4d5..7da0359 100644 --- a/src/incremental/__init__.py +++ b/src/incremental/__init__.py @@ -368,7 +368,7 @@ def _findPath(path, package): # type: (str, str) -> str return current_dir else: raise ValueError( - "Can't find the directory of package {}: I looked in {} and {}".format( + "Can't find the directory of project {}: I looked in {} and {}".format( package, src_dir, current_dir ) ) @@ -404,9 +404,13 @@ def _get_setuptools_version(dist): # type: (_Distribution) -> None but this hook is always called before setuptools loads anything from ``pyproject.toml``. """ - # When operating in a packaging context (i.e. building an sdist or wheel) - # pyproject.toml will always be found in the current working directory. - config = _load_pyproject_toml("./pyproject.toml", opt_in=False) + try: + # When operating in a packaging context (i.e. building an sdist or + # wheel) pyproject.toml will always be found in the current working + # directory. + config = _load_pyproject_toml("./pyproject.toml") + except Exception: + return if not config or not config.opt_in: return @@ -466,13 +470,13 @@ class _IncrementalConfig: """ package: str - """The package name, capitalized as in the package metadata.""" + """The project name, capitalized as in the project metadata.""" path: str """Path to the package root""" -def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_IncrementalConfig] +def _load_pyproject_toml(toml_path): # type: (str) -> Optional[_IncrementalConfig] """ Load Incremental configuration from a ``pyproject.toml`` @@ -483,31 +487,11 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I @param toml_path: Path to the ``pyproject.toml`` to load. - - @param opt_in: - Are we operating in a context where Incremental has been - affirmatively requested? - - Otherwise we do our best to *never* raise an exception until we - find a ``[tool.incremental]`` opt-in. This is important when - operating within a setuptools entry point because those hooks - are invoked anytime the L{Distribution} class is initialized, - which happens in non-packaging contexts that don't match the """ - try: - with open(toml_path, "rb") as f: - data = _load_toml(f) - except Exception: - if opt_in: - raise - return None - - tool_incremental = _extract_tool_incremental(data, opt_in) + with open(toml_path, "rb") as f: + data = _load_toml(f) - # Do we have an affirmative opt-in to use Incremental? - opt_in = opt_in or tool_incremental is not None - if not opt_in: - return None + tool_incremental = _extract_tool_incremental(data) # Extract the project name package = None @@ -522,7 +506,7 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I if package is None: # We can't proceed without a project name. raise ValueError("""\ -Incremental failed to extract the package name from pyproject.toml. Specify it like: +Incremental failed to extract the project name from pyproject.toml. Specify it like: [project] name = "Foo" @@ -535,33 +519,28 @@ def _load_pyproject_toml(toml_path, opt_in): # type: (str, bool) -> Optional[_I """) if not isinstance(package, str): raise TypeError( - "Package name must be a string, but found {}".format(type(package)) + "The project name must be a string, but found {}".format(type(package)) ) return _IncrementalConfig( - opt_in=opt_in, + opt_in=tool_incremental is not None, package=package, path=_findPath(os.path.dirname(toml_path), package), ) -def _extract_tool_incremental(data, opt_in): # type: (Dict[str, object], bool) -> Optional[Dict[str, object]] +def _extract_tool_incremental(data): # type: (Dict[str, object]) -> Optional[Dict[str, object]] if "tool" not in data: return None if not isinstance(data["tool"], dict): - if opt_in: - raise ValueError("[tool] must be a table") - return None + raise ValueError("[tool] must be a table") if "incremental" not in data["tool"]: return None tool_incremental = data["tool"]["incremental"] if not isinstance(tool_incremental, dict): - if opt_in: - raise ValueError("[tool.incremental] must be a table") - return None + raise ValueError("[tool.incremental] must be a table") - # At this point we've found a [tool.incremental] table, so we have opt_in if not {"name"}.issuperset(tool_incremental.keys()): raise ValueError("Unexpected key(s) in [tool.incremental]") return tool_incremental diff --git a/src/incremental/_hatch.py b/src/incremental/_hatch.py index 0305073..7777adc 100644 --- a/src/incremental/_hatch.py +++ b/src/incremental/_hatch.py @@ -21,7 +21,7 @@ class IncrementalVersionSource(VersionSourceInterface): def get_version_data(self) -> _VersionData: # type: ignore[override] path = os.path.join(self.root, "./pyproject.toml") # If the Hatch plugin is running at all we've already opted in. - config = _load_pyproject_toml(path, opt_in=True) + config = _load_pyproject_toml(path) assert config is not None, "Failed to read {}".format(path) return {"version": _existing_version(config.path).public()} diff --git a/src/incremental/tests/test_pyproject.py b/src/incremental/tests/test_pyproject.py index 939d8b8..d0f1c0b 100644 --- a/src/incremental/tests/test_pyproject.py +++ b/src/incremental/tests/test_pyproject.py @@ -15,7 +15,7 @@ class VerifyPyprojectDotTomlTests(TestCase): """Test the `_load_pyproject_toml` helper function""" def _loadToml( - self, toml: str, opt_in: bool, *, path: Union[Path, str, None] = None + self, toml: str, *, path: Union[Path, str, None] = None ) -> Optional[_IncrementalConfig]: """ Read a TOML snipped from a temporary file with `_load_pyproject_toml` @@ -34,7 +34,7 @@ def _loadToml( f.write(toml) try: - return _load_pyproject_toml(path_, opt_in) + return _load_pyproject_toml(path_) except Exception as e: if hasattr(e, "add_note"): e.add_note( # type: ignore[attr-defined] @@ -48,50 +48,27 @@ def test_fileNotFound(self): there is opt-in. """ path = os.path.join(cast(str, self.mktemp()), "pyproject.toml") - self.assertIsNone(_load_pyproject_toml(path, False)) - self.assertRaises(FileNotFoundError, _load_pyproject_toml, path, True) + self.assertRaises(FileNotFoundError, _load_pyproject_toml, path) def test_brokenToml(self): """ Syntactially invalid TOML is ignored unless there's an opt-in. """ toml = '[project]\nname = "abc' # truncated + self.assertRaises(Exception, self._loadToml, toml) - self.assertIsNone(self._loadToml(toml, False)) - self.assertRaises(Exception, self._loadToml, toml, True) - - def test_configMissing(self): + def test_nameMissing(self): """ - A ``pyproject.toml`` that exists but provides no relevant configuration - is ignored unless opted in. + `ValueError` is raised when we can't extract the project name. """ for toml in [ "\n", "[tool.notincremental]\n", "[project]\n", - ]: - self.assertIsNone(self._loadToml(toml, False)) - - def test_nameMissing(self): - """ - `ValueError` is raised when ``[tool.incremental]`` is present but - the project name isn't available. The ``[tool.incremental]`` - section counts as opt-in. - """ - for toml in [ "[tool.incremental]\n", "[project]\n[tool.incremental]\n", ]: - self.assertRaises(ValueError, self._loadToml, toml, False) - self.assertRaises(ValueError, self._loadToml, toml, True) - - def test_nameInvalidNoOptIn(self): - """ - An invalid project name is ignored when there's no opt-in. - """ - self.assertIsNone( - self._loadToml("[project]\nname = false\n", False), - ) + self.assertRaises(ValueError, self._loadToml, toml) def test_nameInvalidOptIn(self): """ @@ -103,7 +80,8 @@ def test_nameInvalidOptIn(self): "[tool.incremental]\nname = -1\n", "[tool.incremental]\n[project]\nname = 1.0\n", ]: - self.assertRaises(TypeError, self._loadToml, toml, True) + with self.assertRaisesRegex(TypeError, "The project name must be a string"): + self._loadToml(toml) def test_toolIncrementalInvalid(self): """ @@ -117,8 +95,7 @@ def test_toolIncrementalInvalid(self): "[tool]\nincremental = 123\n", "[tool]\nincremental = null\n", ]: - self.assertIsNone(self._loadToml(toml, False)) - self.assertRaises(ValueError, self._loadToml, toml, True) + self.assertRaises(ValueError, self._loadToml, toml) def test_toolIncrementalUnexpecteKeys(self): """ @@ -129,7 +106,7 @@ def test_toolIncrementalUnexpecteKeys(self): "[tool.incremental]\nfoo = false\n", '[tool.incremental]\nname = "OK"\nother = false\n', ]: - self.assertRaises(ValueError, self._loadToml, toml, False) + self.assertRaises(ValueError, self._loadToml, toml) def test_setuptoolsOptIn(self): """ @@ -145,7 +122,7 @@ def test_setuptoolsOptIn(self): '[project]\nname = "Foo"\n[tool.incremental]\n', '[tool.incremental]\nname = "Foo"\n', ]: - config = self._loadToml(toml, False, path=root / "pyproject.toml") + config = self._loadToml(toml, path=root / "pyproject.toml") self.assertEqual( config, @@ -156,44 +133,23 @@ def test_setuptoolsOptIn(self): ), ) - def test_noToolIncrementalSection(self): + def test_packagePathRequired(self): """ - We don't produce config unless we find opt-in. - - The ``[project]`` section doesn't imply opt-in, even if we can - recover the project name from it. - """ - root = Path(self.mktemp()) - pkg = root / "foo" # A valid package directory. - pkg.mkdir(parents=True) - - config = self._loadToml( - '[project]\nname = "foo"\n', - opt_in=False, - path=root / "pyproject.toml", - ) - - self.assertIsNone(config) - - def test_pathNotFoundOptIn(self): - """ - Once opted in, raise `ValueError` when the package root can't - be resolved. + Raise `ValueError` when the package root can't be resolved. """ root = Path(self.mktemp()) root.mkdir() # Contains no package directory. - with self.assertRaisesRegex(ValueError, "Can't find the directory of package"): + with self.assertRaisesRegex(ValueError, "Can't find the directory of project "): self._loadToml( '[project]\nname = "foo"\n', - opt_in=True, path=root / "pyproject.toml", ) - def test_noToolIncrementalSectionOptIn(self): + def test_noToolIncrementalSection(self): """ - If opted in (i.e. in the Hatch plugin) then the [tool.incremental] - table is completely optional. + The ``[tool.incremental]`` table is not strictly required, but its + ``opt_in=False`` indicates its absence. """ root = Path(self.mktemp()) pkg = root / "src" / "foo" @@ -201,14 +157,13 @@ def test_noToolIncrementalSectionOptIn(self): config = self._loadToml( '[project]\nname = "Foo"\n', - opt_in=True, path=root / "pyproject.toml", ) self.assertEqual( config, _IncrementalConfig( - opt_in=True, + opt_in=False, package="Foo", path=str(pkg), ), From d2fe36fa5bb5bb13bfbeb3461154fadd5d709993 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 16:51:24 -0700 Subject: [PATCH 10/12] Defense in depth It seems we must rely on heuristics, so let's go all the way. --- src/incremental/__init__.py | 31 +++++---- src/incremental/_hatch.py | 2 +- src/incremental/newsfragments/106.bugfix | 2 +- src/incremental/update.py | 14 ++-- tests/test_examples.py | 81 ++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 20 deletions(-) diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index 7da0359..9ded25c 100644 --- a/src/incremental/__init__.py +++ b/src/incremental/__init__.py @@ -374,14 +374,13 @@ def _findPath(path, package): # type: (str, str) -> str ) -def _existing_version(path): # type: (str) -> Version +def _existing_version(version_path): # type: (str) -> Version """ - Load the current version from {path}/_version.py. + Load the current version from a ``_version.py`` file. """ version_info = {} # type: Dict[str, Version] - versionpath = os.path.join(path, "_version.py") - with open(versionpath, "r") as f: + with open(version_path, "r") as f: exec(f.read(), version_info) return version_info["__version__"] @@ -393,8 +392,7 @@ def _get_setuptools_version(dist): # type: (_Distribution) -> None This function is registered as a setuptools.finalize_distribution_options entry point [1]. Consequently, it is called in all sorts of weird - contexts, so it strives to not raise unless there is a pyproject.toml - containing a [tool.incremental] section. + contexts. In setuptools, silent failure is the law. [1]: https://setuptools.pypa.io/en/latest/userguide/extension.html#customizing-distribution-options @@ -411,10 +409,16 @@ def _get_setuptools_version(dist): # type: (_Distribution) -> None config = _load_pyproject_toml("./pyproject.toml") except Exception: return - if not config or not config.opt_in: + + if not config.opt_in: return - dist.metadata.version = _existing_version(config.path).public() + try: + version = _existing_version(config.version_path) + except Exception: + return + + dist.metadata.version = version.public() def _get_distutils_version(dist, keyword, value): # type: (_Distribution, object, object) -> None @@ -435,8 +439,8 @@ def _get_distutils_version(dist, keyword, value): # type: (_Distribution, objec for item in sp_command.find_all_modules(): if item[1] == "_version": - package_path = os.path.dirname(item[2]) - dist.metadata.version = _existing_version(package_path).public() + version_path = os.path.join(os.path.dirname(item[2]), "_version.py") + dist.metadata.version = _existing_version(version_path).public() return raise Exception("No _version.py found.") # pragma: no cover @@ -475,8 +479,13 @@ class _IncrementalConfig: path: str """Path to the package root""" + @property + def version_path(self): # type: () -> str + """Path of the ``_version.py`` file. May not exist.""" + return os.path.join(self.path, "_version.py") + -def _load_pyproject_toml(toml_path): # type: (str) -> Optional[_IncrementalConfig] +def _load_pyproject_toml(toml_path): # type: (str) -> _IncrementalConfig """ Load Incremental configuration from a ``pyproject.toml`` diff --git a/src/incremental/_hatch.py b/src/incremental/_hatch.py index 7777adc..39ddcb7 100644 --- a/src/incremental/_hatch.py +++ b/src/incremental/_hatch.py @@ -23,7 +23,7 @@ def get_version_data(self) -> _VersionData: # type: ignore[override] # If the Hatch plugin is running at all we've already opted in. config = _load_pyproject_toml(path) assert config is not None, "Failed to read {}".format(path) - return {"version": _existing_version(config.path).public()} + return {"version": _existing_version(config.version_path).public()} def set_version(self, version: str, version_data: Dict[Any, Any]) -> None: raise NotImplementedError( diff --git a/src/incremental/newsfragments/106.bugfix b/src/incremental/newsfragments/106.bugfix index 63749ba..4697af2 100644 --- a/src/incremental/newsfragments/106.bugfix +++ b/src/incremental/newsfragments/106.bugfix @@ -1,3 +1,3 @@ Incremental could mis-identify that a project had opted in to version management. -If a ``pyproject.toml`` in the current directory contained a ``[project]`` table with a ``name`` key, but did not contain the opt-in ``[tool.incremental]`` table, Incremental would still treat the file as if the opt-in were present and attempt to validate the configuration. This could happen in contexts outside of packaging, such as when creating a virtualenv. When operating as a setuptools plugin Incremental now always requires the ``[tool.incremental]`` opt-in. Additionally, it suppresses any exceptions that occur while attempting to read ``pyproject.toml`` until it finds a valid ``[tool.incremental]`` table. +If a ``pyproject.toml`` in the current directory contained a ``[project]`` table with a ``name`` key, but did not contain the opt-in ``[tool.incremental]`` table, Incremental would still treat the file as if the opt-in were present and attempt to validate the configuration. This could happen in contexts outside of packaging, such as when creating a virtualenv. When operating as a setuptools plugin Incremental now always ignores invalid configuration, such as configuration that doesn't match the content of the working directory. diff --git a/src/incremental/update.py b/src/incremental/update.py index f3e33ee..0c92e77 100644 --- a/src/incremental/update.py +++ b/src/incremental/update.py @@ -77,10 +77,11 @@ def _run( ): raise ValueError("Only give --create") + versionpath = os.path.join(path, "_version.py") if newversion: from pkg_resources import parse_version - existing = _existing_version(path) + existing = _existing_version(versionpath) st_version = parse_version(newversion)._version # type: ignore[attr-defined] release = list(st_version.release) @@ -109,7 +110,7 @@ def _run( existing = v elif rc and not patch: - existing = _existing_version(path) + existing = _existing_version(versionpath) if existing.release_candidate: v = Version( @@ -123,7 +124,7 @@ def _run( v = Version(package, _date.year - _YEAR_START, _date.month, 0, 1) elif patch: - existing = _existing_version(path) + existing = _existing_version(versionpath) v = Version( package, existing.major, @@ -133,7 +134,7 @@ def _run( ) elif post: - existing = _existing_version(path) + existing = _existing_version(versionpath) if existing.post is None: _post = 0 @@ -143,7 +144,7 @@ def _run( v = Version(package, existing.major, existing.minor, existing.micro, post=_post) elif dev: - existing = _existing_version(path) + existing = _existing_version(versionpath) if existing.dev is None: _dev = 0 @@ -160,7 +161,7 @@ def _run( ) else: - existing = _existing_version(path) + existing = _existing_version(versionpath) if existing.release_candidate: v = Version(package, existing.major, existing.minor, existing.micro) @@ -212,7 +213,6 @@ def _run( with open(filepath, "wb") as f: f.write(content) - versionpath = os.path.join(path, "_version.py") _print("Updating %s" % (versionpath,)) with open(versionpath, "wb") as f: f.write( diff --git a/tests/test_examples.py b/tests/test_examples.py index efe8be0..879e2c5 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -69,6 +69,87 @@ def test_setuptools_version(self): self.assertEqual(example_setuptools.__version__.base(), "2.3.4") self.assertEqual(metadata.version("example_setuptools"), "2.3.4") + def test_setuptools_no_optin(self): + """ + The setuptools plugin is a no-op when there isn't a + [tool.incremental] table in pyproject.toml. + """ + src = FilePath(self.mktemp()) + src.makedirs() + src.child("pyproject.toml").setContent( + b"""\ +[project] +name = "example_no_optin" +version = "0.0.0" +""" + ) + package_dir = src.child("example_no_optin") + package_dir.makedirs() + package_dir.child("__init__.py").setContent(b"") + package_dir.child("_version.py").setContent( + b"from incremental import Version\n" + b'__version__ = Version("example_no_optin", 24, 7, 0)\n' + ) + + build_and_install(src) + + self.assertEqual(metadata.version("example_no_optin"), "0.0.0") + + def test_setuptools_no_package(self): + """ + The setuptools plugin is a no-op when there isn't a + package directory that matches the project name. + """ + src = FilePath(self.mktemp()) + src.makedirs() + src.child("pyproject.toml").setContent( + b"""\ +[project] +name = "example_no_package" +version = "0.0.0" + +[tool.incremental] +""" + ) + + build_and_install(src) + + self.assertEqual(metadata.version("example_no_package"), "0.0.0") + + def test_setuptools_bad_versionpy(self): + """ + The setuptools plugin is a no-op when reading the version + from ``_version.py`` fails. + """ + src = FilePath(self.mktemp()) + src.makedirs() + src.child("setup.py").setContent( + b"""\ +from setuptools import setup + +setup( + name="example_bad_versionpy", + version="0.1.2", + packages=["example_bad_versionpy"], + zip_safe=False, +) +""" + ) + src.child("pyproject.toml").setContent( + b"""\ +[tool.incremental] +name = "example_bad_versionpy" +""" + ) + package_dir = src.child("example_bad_versionpy") + package_dir.makedirs() + package_dir.child("_version.py").setContent(b"bad version.py") + + build_and_install(src) + + # The version from setup.py wins. + self.assertEqual(metadata.version("example_bad_versionpy"), "0.1.2") + def test_hatchling_get_version(self): """ example_hatchling has a version of 24.7.0. From 3d2cdb172c7d5c7e27903c0949128f0aa7888741 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 27 Jul 2024 20:19:34 -0700 Subject: [PATCH 11/12] Cleanups to shorten the diff --- src/incremental/_hatch.py | 2 -- src/incremental/tests/test_pyproject.py | 13 ++++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/incremental/_hatch.py b/src/incremental/_hatch.py index 39ddcb7..b7e5325 100644 --- a/src/incremental/_hatch.py +++ b/src/incremental/_hatch.py @@ -20,9 +20,7 @@ class IncrementalVersionSource(VersionSourceInterface): def get_version_data(self) -> _VersionData: # type: ignore[override] path = os.path.join(self.root, "./pyproject.toml") - # If the Hatch plugin is running at all we've already opted in. config = _load_pyproject_toml(path) - assert config is not None, "Failed to read {}".format(path) return {"version": _existing_version(config.version_path).public()} def set_version(self, version: str, version_data: Dict[Any, Any]) -> None: diff --git a/src/incremental/tests/test_pyproject.py b/src/incremental/tests/test_pyproject.py index d0f1c0b..0d1f021 100644 --- a/src/incremental/tests/test_pyproject.py +++ b/src/incremental/tests/test_pyproject.py @@ -52,9 +52,10 @@ def test_fileNotFound(self): def test_brokenToml(self): """ - Syntactially invalid TOML is ignored unless there's an opt-in. + Syntactially invalid TOML produces an exception. The specific + exception varies by the underlying TOML library. """ - toml = '[project]\nname = "abc' # truncated + toml = '[project]\nname = "abc' # Truncated string. self.assertRaises(Exception, self._loadToml, toml) def test_nameMissing(self): @@ -72,8 +73,7 @@ def test_nameMissing(self): def test_nameInvalidOptIn(self): """ - Once opted in, `TypeError` is raised when the project name - isn't a string. + `TypeError` is raised when the project name isn't a string. """ for toml in [ "[project]\nname = false\n", @@ -85,9 +85,8 @@ def test_nameInvalidOptIn(self): def test_toolIncrementalInvalid(self): """ - When ``[tool]`` or ``[tool.incremental]`` isn't a table the - ``pyproject.toml`` it's an error if opted-in, otherwise the - file is ignored. + `ValueError` is raised when the ``[tool]`` or ``[tool.incremental]`` + isn't a table. """ for toml in [ "tool = false\n", From a559f5c770ec93c95abaadfc8693d7ae654c6e39 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Mon, 29 Jul 2024 12:25:28 -0700 Subject: [PATCH 12/12] Allow syntax errors to propagate --- src/incremental/__init__.py | 4 ++-- tests/test_examples.py | 47 +++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index 9ded25c..aa960bb 100644 --- a/src/incremental/__init__.py +++ b/src/incremental/__init__.py @@ -415,7 +415,7 @@ def _get_setuptools_version(dist): # type: (_Distribution) -> None try: version = _existing_version(config.version_path) - except Exception: + except FileNotFoundError: return dist.metadata.version = version.public() @@ -480,7 +480,7 @@ class _IncrementalConfig: """Path to the package root""" @property - def version_path(self): # type: () -> str + def version_path(self): # type: () -> str """Path of the ``_version.py`` file. May not exist.""" return os.path.join(self.path, "_version.py") diff --git a/tests/test_examples.py b/tests/test_examples.py index 879e2c5..5920c9d 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -10,7 +10,7 @@ from subprocess import run from tempfile import TemporaryDirectory -from build import ProjectBuilder +from build import ProjectBuilder, BuildBackendException from build.env import DefaultIsolatedEnv from twisted.python.filepath import FilePath from twisted.trial.unittest import TestCase @@ -116,10 +116,44 @@ def test_setuptools_no_package(self): self.assertEqual(metadata.version("example_no_package"), "0.0.0") + def test_setuptools_missing_versionpy(self): + """ + The setuptools plugin is a no-op when the ``_version.py`` file + isn't present. + """ + src = FilePath(self.mktemp()) + src.makedirs() + src.child("setup.py").setContent( + b"""\ +from setuptools import setup + +setup( + name="example_missing_versionpy", + version="0.0.1", + packages=["example_missing_versionpy"], + zip_safe=False, +) +""" + ) + src.child("pyproject.toml").setContent( + b"""\ +[tool.incremental] +name = "example_missing_versionpy" +""" + ) + package_dir = src.child("example_missing_versionpy") + package_dir.makedirs() + package_dir.child("__init__.py").setContent(b"") + # No _version.py exists + + build_and_install(src) + + # The version from setup.py wins. + self.assertEqual(metadata.version("example_missing_versionpy"), "0.0.1") + def test_setuptools_bad_versionpy(self): """ - The setuptools plugin is a no-op when reading the version - from ``_version.py`` fails. + The setuptools plugin surfaces syntax errors in ``_version.py``. """ src = FilePath(self.mktemp()) src.makedirs() @@ -145,10 +179,9 @@ def test_setuptools_bad_versionpy(self): package_dir.makedirs() package_dir.child("_version.py").setContent(b"bad version.py") - build_and_install(src) - - # The version from setup.py wins. - self.assertEqual(metadata.version("example_bad_versionpy"), "0.1.2") + with self.assertRaises(BuildBackendException): + # This also spews a SyntaxError traceback to stdout. + build_and_install(src) def test_hatchling_get_version(self): """