diff --git a/.coveragerc b/.coveragerc index bfe9d69b..d905bf23 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/README.rst b/README.rst index 56baba2f..0d74e8a8 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 ] # ... diff --git a/src/incremental/__init__.py b/src/incremental/__init__.py index 7a29fd7b..aa960bbe 100644 --- a/src/incremental/__init__.py +++ b/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,7 +466,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 @@ -459,13 +474,18 @@ 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""" + @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`` @@ -473,30 +493,29 @@ 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. """ - 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), ) diff --git a/src/incremental/_hatch.py b/src/incremental/_hatch.py index 8d677c4b..b7e53250 100644 --- a/src/incremental/_hatch.py +++ b/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( diff --git a/src/incremental/newsfragments/106.bugfix b/src/incremental/newsfragments/106.bugfix new file mode 100644 index 00000000..4697af24 --- /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 ignores invalid configuration, such as configuration that doesn't match the content of the working directory. diff --git a/src/incremental/tests/test_pyproject.py b/src/incremental/tests/test_pyproject.py index 036f5a06..0d1f021c 100644 --- a/src/incremental/tests/test_pyproject.py +++ b/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), ), ) diff --git a/src/incremental/tests/test_version.py b/src/incremental/tests/test_version.py index da66503b..b9bd2a46 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) diff --git a/src/incremental/update.py b/src/incremental/update.py index f3e33ee0..0c92e777 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 f9ad2c80..5920c9d0 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 import ProjectBuilder, BuildBackendException +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): @@ -50,6 +69,120 @@ 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_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 surfaces syntax errors in ``_version.py``. + """ + 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") + + with self.assertRaises(BuildBackendException): + # This also spews a SyntaxError traceback to stdout. + build_and_install(src) + def test_hatchling_get_version(self): """ example_hatchling has a version of 24.7.0. diff --git a/tox.ini b/tox.ini index f0f662b5..d6437e28 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