Merge pull request #107 from twisted/106-never-raise

Rework pyproject.toml parsing to never raise under setuptools

.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 =

README.rst

@@ -24,15 +24,15 @@ 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"
 
     [project]
     name = "<projectname>"
     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,15 +55,15 @@ 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"
 
     [project]
     name = "<projectname>"
     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
     ]
     # ...
 

src/incremental/__init__.py

@@ -368,20 +368,19 @@ 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
             )
         )
 
 
-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__"]
@@ -392,18 +391,34 @@ 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.
-
-    @param dist: An empty C{setuptools.Distribution} instance to mutate.
+    entry point [1]. Consequently, it is called in all sorts of weird
+    contexts. In setuptools, silent failure is the law.
 
     [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``.
     """
-    config = _load_pyproject_toml("./pyproject.toml")
-    if not config or not config.has_tool_incremental:
+    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.opt_in:
         return
 
-    dist.metadata.version = _existing_version(config.path).public()
+    try:
+        version = _existing_version(config.version_path)
+    except FileNotFoundError:
+        return
+
+    dist.metadata.version = version.public()
 
 
 def _get_distutils_version(dist, keyword, value):  # type: (_Distribution, object, object) -> None
@@ -424,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
@@ -451,52 +466,56 @@ 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
     opted-in to Incremental versioning.
     """
 
     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"""
 
+    @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``
 
     If the [tool.incremental] section is empty we take the project name
     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.
     """
-    try:
-        with open(toml_path, "rb") as f:
-            data = _load_toml(f)
-    except FileNotFoundError:
-        return None
+    with open(toml_path, "rb") as f:
+        data = _load_toml(f)
 
     tool_incremental = _extract_tool_incremental(data)
 
+    # Extract the project name
     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:
             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 tool_incremental is None:
-            return None
+        # We can't proceed without a project name.
         raise ValueError("""\
-Couldn't 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"
@@ -509,11 +528,11 @@ def _load_pyproject_toml(toml_path):  # type: (str) -> Optional[_IncrementalConf
 """)
     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(
-        has_tool_incremental=tool_incremental is not None,
+        opt_in=tool_incremental is not None,
         package=package,
         path=_findPath(os.path.dirname(toml_path), package),
     )

src/incremental/_hatch.py

@@ -21,8 +21,7 @@ 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)
-        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(

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 ignores invalid configuration, such as configuration that doesn't match the content of the working directory.

src/incremental/tests/test_pyproject.py

@@ -44,43 +44,44 @@ 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.assertRaises(FileNotFoundError, _load_pyproject_toml, path)
 
-    def test_configMissing(self):
+    def test_brokenToml(self):
         """
-        A ``pyproject.toml`` that exists but provides no relevant configuration
-        is ignored.
+        Syntactially invalid TOML produces an exception. The specific
+        exception varies by the underlying TOML library.
         """
-        for toml in [
-            "\n",
-            "[tool.notincremental]\n",
-            "[project]\n",
-        ]:
-            self.assertIsNone(self._loadToml(toml))
+        toml = '[project]\nname = "abc'  # Truncated string.
+        self.assertRaises(Exception, self._loadToml, toml)
 
     def test_nameMissing(self):
         """
-        `ValueError` is raised when ``[tool.incremental]`` is present but
-        the project name isn't available.
+        `ValueError` is raised when we can't extract the project name.
         """
         for toml in [
+            "\n",
+            "[tool.notincremental]\n",
+            "[project]\n",
             "[tool.incremental]\n",
             "[project]\n[tool.incremental]\n",
         ]:
             self.assertRaises(ValueError, self._loadToml, toml)
 
-    def test_nameInvalid(self):
+    def test_nameInvalidOptIn(self):
         """
         `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)
+            with self.assertRaisesRegex(TypeError, "The project name must be a string"):
+                self._loadToml(toml)
 
     def test_toolIncrementalInvalid(self):
         """
@@ -125,31 +126,44 @@ def test_setuptoolsOptIn(self):
             self.assertEqual(
                 config,
                 _IncrementalConfig(
-                    has_tool_incremental=True,
+                    opt_in=True,
                     package="Foo",
                     path=str(pkg),
                 ),
             )
 
+    def test_packagePathRequired(self):
+        """
+        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 project "):
+            self._loadToml(
+                '[project]\nname = "foo"\n',
+                path=root / "pyproject.toml",
+            )
+
     def test_noToolIncrementalSection(self):
         """
-        The ``has_tool_incremental`` flag is false when there
-        isn't a ``[tool.incremental]`` section.
+        The ``[tool.incremental]`` table is not strictly required, but its
+        ``opt_in=False`` indicates its absence.
         """
         root = Path(self.mktemp())
-        pkg = root / "foo"
+        pkg = root / "src" / "foo"
         pkg.mkdir(parents=True)
 
         config = self._loadToml(
-            '[project]\nname = "foo"\n',
+            '[project]\nname = "Foo"\n',
             path=root / "pyproject.toml",
         )
 
         self.assertEqual(
             config,
             _IncrementalConfig(
-                has_tool_incremental=False,
-                package="foo",
+                opt_in=False,
+                package="Foo",
                 path=str(pkg),
             ),
         )

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)