commitizen-tools / commitizen

Create committing rules for projects :rocket: auto bump versions :arrow_up: and auto changelog generation :open_file_folder:
https://commitizen-tools.github.io/commitizen/
MIT License
2.41k stars 257 forks source link

Improve referencing __version__ attribute. #913

Open jenstroeger opened 10 months ago

jenstroeger commented 10 months ago

Description

The other day I came across the package version declaration in SQLAlchemy’s setup.cfg here:

version = attr: sqlalchemy.__version__

The handling of special directive attr: is documented here.

I thought this quite useful to ensure that a version is declared in one place only; currently cz relies on version_files to point at attributes that need to be bumped.

This is probably a low-priority item that may or may not make sense. But I wanted to raise it for discussion anyway…

Possible Solution

No response

Additional context

No response

Additional context

No response

woile commented 10 months ago

What about custom version providers? I think you could have one using __version__ as the source of the version

jenstroeger commented 10 months ago

Hmm. That’s a nice idea but I’d have to wire my own blob of code into into pyproject.toml and ship another module, right? Unless we allow folks to stash Python into a TOML file (but that’s getting ugly for little benefit…) 🤔

woile commented 10 months ago

You'd have to publish a package with the custom provider in python, no code goes in pyproject.toml.

Then you'd install:

pip install commitizen cz-attr-version

And then you'd do in commitizen toml:

provider = "attr_version" 
woile commented 10 months ago

Now that I think about it... maybe we could even make the existing provider pep621 read this, right? pep621 is reading from the version field, that means it should be able to interpret accepted values, either a version or an attr 🤔

On the other hand, the recommendation seems to point to dropping the usage of any version information at module leven in favor of just using importlib.metadata.version()

see pep-396-rejection

Thoughts @Lee-W @noirbizarre ?

jenstroeger commented 9 months ago

Now that I think about it... maybe we could even make the existing provider pep621 read this, right?

I do like that.

On the other hand, the recommendation seems to point to dropping the usage of any version information at module leven in favor of just using importlib.metadata.version() see pep-396-rejection

That PEP is from 2011, so I’m tempted to not give it too much weight. Might be worth bringing up for discussion at the forum?

Lee-W commented 9 months ago

Now that I think about it... maybe we could even make the existing provider pep621 read this, right?

like this one as well

noirbizarre commented 9 months ago

This is a complex topic I know quite well (sorry for the long post, but on this topic, this is required).

You need to include in the reasoning that while PEP621 is about exposing static metadata that can be read at packaging time (which is perfect for commitizen as it only imply reading/writing a toml file), both importlib.metadata.version() and <pkg>.__version__ are runtime ways of accessing the version, and I think this is a bit out of scope for commitizen.

Runtime access to both of those is not yet standardized, PEP621 is not about this and I don't think commitizen has any business to to on this. Those are the responsibility:

Note that PEP517 build backend is what makes your project buildable with build and pip-installable from scm even if built with pdm, poetry or any other tool. It also allows proper packaging by other tools in the case a wheel does not exists (because it was never published or because the wheel is platform specific, and you use one for which no wheel has been published)

However, both importlib.metadata.version() and <root_module>.__version__ SHOULD reflect PEP621 versioning. Given the lack of PEP on the topic, there are many ways of handling this.

Here's my recipe to have commitizen properly works with PEP621 metadata and having both runtime access properly work (and my PR on version providers was meant to allow that):

This makes pyproject.toml looks like this:

[project]  # PEP621 standard project metadata
...
dynamic = ["version"]  # standard way of delegating dynamic fields resolution to the PEP517 backend

[build-system]  # PEP517 build backend
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm.version] # Specific to pdm build backend
source = "scm"
write_to = "<pkg>/VERSION"

[tool.commitizen]
version_provider = "scm"

and the <pkg>/__init__.py looks like this:

from __future__ import annotations

from importlib.metadata import PackageNotFoundError, version
from pathlib import Path

def read_version() -> str:
    try:
        return str(version("<pkg-name>"))  # This is the PEP621 `project.name` not the root package
    except (PackageNotFoundError, ImportError):
        version_file = Path(__file__).parent / "VERSION"
        return version_file.read_text() if version_file.is_file() else "0.0.0.dev"

__version__ = read_version()  # should be failsafe

Why not just read the version file ? Because SCM tags don't support PEP440 nor semver build/local identifiers (the +something part which can be set at package time on some package manager), the version file generated by the scm plugin use the build identifier to add a commitish+hash for non tagged version and the real build identifier is set in Core Metadata so importlib.metadata gives the version that was actually packaged, included all dynamic parts especially those that could be set in CI, while the version file give the version generated from the scm.

But this is only one way of doing it properly, my way according to my constraints (one being having runtime traceability of deployed assets versions, event if they are not tagged, which is often the case when you have some CI and you only bump versions you consider stable after trial). Note that this recipe also works with editable installation of your project.

This is very related to how you expect to use those. For many people (especially those coming from the JS world where this is the package.json "normal way"), only tagged versions are relevant and so PEP621 version provider is enough.

TLDR: the PEP621 provider should not read those packaging or runtime only information (meaning they exist at a time commitizen can't read) however documenting how it's possible to properly read this metadata at runtime and supports all those conventions in a standard Python package could be a great addition.

Note: a static tool trying to read the __version__ field should be considered as broken by design, as it is an unspecified runtime-only facility which might rely on a lot of dynamic behavior and will most of the time break on static access. However, setuptools actually loads the module to read the attribute, which works but leads to other chicken and eggs problems (example: this means your __init__.py should not have external imports as it is read before dependency resolution, at metadata extraction time, preventing you to use any root module facade pattern if you have external dependencies, so it might be better to expose version in a dedicated nested module like <pkg>._version and just import it in __init__.py but says to setuptools to read it from the nested module attribute <pkg>._version.__version__ to avoid triggering uninstalled imports).