Skip to content

Commit

Permalink
Rework pyproject.toml parsing to never raise
Browse files Browse the repository at this point in the history
This is super messy and needs a refactor.
  • Loading branch information
twm committed Jul 27, 2024
1 parent 95769c5 commit 4103f66
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 42 deletions.
83 changes: 62 additions & 21 deletions src/incremental/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -465,38 +470,55 @@ 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``
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.
@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:
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
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"
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/incremental/_hatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()}

Expand Down
66 changes: 46 additions & 20 deletions src/incremental/tests/test_pyproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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]
Expand All @@ -44,56 +44,81 @@ 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",
"[tool]\nincremental = false\n",
"[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):
"""
Expand All @@ -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):
"""
Expand All @@ -120,35 +145,36 @@ 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),
),
)

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"
pkg.mkdir(parents=True)

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),
),
Expand Down

0 comments on commit 4103f66

Please sign in to comment.