Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid auto-increment by setuptools_scm #3

Closed
dornech opened this issue Dec 11, 2024 · 19 comments
Closed

avoid auto-increment by setuptools_scm #3

dornech opened this issue Dec 11, 2024 · 19 comments

Comments

@dornech
Copy link

dornech commented Dec 11, 2024

I had trouble with setuptools_scm incrementing the version number leading to a mismatch with the verison displayed by Hatch and the internal Hatch call of setuptools_scm. I guess this has to do with some config setting snot properly aligned.

I thougth: why not using the output of calling "hatch version"?
Well here you go:

# libraries
from hatch.cli import hatch
from click.testing import CliRunner
# determine version via hatch
runner= CliRunner()
result = runner.invoke(hatch, ["version"])
__version__ = result.output.strip()
@maresb
Copy link
Owner

maresb commented Dec 12, 2024

It's the default behavior of setuptools-scm to increment the version number, see this part of the docs. (It used to be featured a lot more prominently, directly in the README, but now it seems quite hidden away.)

To prevent the increment, I used to add the following to my pyproject.toml:

[tool.hatch.version.raw-options]
# See configuration details in
# <https://setuptools-scm.readthedocs.io/en/latest/extending/#version-number-construction>
version_scheme = "no-guess-dev"

Regarding the idea of running hatch version, one disadvantage I see is that it assumes that the hatch CLI is installed. I personally like to use the hatchling build backend, but I don't use hatch, so this wouldn't work in my case.

Also, some quick experiments seemed to show that hatch version uses the existing versioning scheme, so I assume your two points (1. version increment, 2. suggest hatch version) are unrelated?

@maresb
Copy link
Owner

maresb commented Dec 12, 2024

BTW, I really like the idea of checking out what the hatch cli is doing for comparison. The code seems to be https://github.com/pypa/hatch/blob/master/src/hatch/cli/version/__init__.py

It looks like what they do is to run

python -u -m hatchling version
# or equivalently just
hatchling version

I was getting

hatchling.plugin.exceptions.UnknownPluginError: Unknown version source: vcs

and then I realized that the build-system requires hatch-vcs, so I installed that, and it worked.

The Hatchling CLI defines version as:
https://github.com/pypa/hatch/blob/master/backend/src/hatchling/cli/version/__init__.py

And it pulls the version from this source interface:

https://github.com/pypa/hatch/blob/master/backend/src/hatchling/version/source/plugin/interface.py#L6

And when using hatch-vcs, it's get_version_data as defined in https://github.com/ofek/hatch-vcs/blob/master/hatch_vcs/version_source.py

And that's compared to what we do with

__version__ = get_version(root="..", relative_to=__file__)

Ooooohhhhhh, now I get it 😆

We're ignoring the config arguments like version_scheme. Hmm, this is an interesting dilemma...

@maresb
Copy link
Owner

maresb commented Dec 12, 2024

Summary

Suppose we set an option like version_scheme = "no-guess-dev" in pyproject.toml. Then the way we currently compute __version__ ignores this setting, because when we call setuptools-scm's get_version we don't propagate the corresponding argument.

In order to properly propagate configuration arguments to get_version, we need to use or duplicate this functionality from hatch-vcs.

So instead of using setuptools.get_version, we really should instead be using hatch-vcs or hatchling. Your solution calls hatchling via hatch. Note that hatch-vcs depends on hatchling, so given that hatch-vcs is required to do this right, we can assume hatchling.

Unfortunately the version_impl function for the hatchling CLI prints the version instead of returning it. And since hatchling doesn't seem to use Click, so I don't think your click.testing.CliRunner trick is possible.

I can think of two ways to proceed that avoid the hatch dependency:

Use the hatchling library to replicate version_impl:

from pathlib import Path
from hatchling.plugin.manager import PluginManager
from hatchling.metadata.core import ProjectMetadata


def get_version(root: Path) -> str:
    plugin_manager = PluginManager()
    metadata = ProjectMetadata(str(root), plugin_manager)
    __version__ = metadata.core.version
    if __version__ is None:
        # Version is not statically set, so compute it dynamically.
        source = metadata.hatch.version.source
        version_data = source.get_version_data()
        __version__ = version_data["version"]
    return __version__


print(get_version(Path.cwd()))

Use a hatchling subprocess:

import subprocess
from pathlib import Path


def get_version(root: Path) -> str:
    result = subprocess.run(
        ["hatchling", "version"], cwd=root, capture_output=True, text=True
    )
    return result.stdout.strip()


print(get_version(Path.cwd()))

We should also figure out how to handle the case that hatch-vcs is not installed in the dev environment.

@dornech, what do you think?

@dornech
Copy link
Author

dornech commented Dec 12, 2024

Hi, I understood that setuptools_scm does the increment automatically and I tried to avoid this. I do not like that hatch version and setuptools_scm provide different version numbers.

Actually in the pyproject.toml I do have

[tool.hatch.version]
source = "vcs"
raw-options = { version_scheme = "no-guess-dev" }

which should pass the setting to no-guess-dev to the internal hatch call of setuptools_scm - but this does of course not happen when using your code. I think that is what you realized as well already.
I also tried to provide the setting directly in a separate pyproject.toml table but that did not work.

The idea and advantage of calling "hatch version" in the footgun workaround is that you need only one setting and have always the same call to setuptools_scm.
I was trying to do a subprocess call with output redirection before but didn't succeed. In principle a direct call to hatchling should work out as well because hatch calls it internally anyway. But I didn't make it and was not sure how much additional code this would require to find all parameters.

@maresb
Copy link
Owner

maresb commented Dec 12, 2024

I do not like that hatch version and setuptools_scm provide different version numbers.

Could you clarify exactly what you mean by this?

In principle a direct call to hatchling should work out as well because hatch calls it internally anyway.

My impression is that the hatchling code is a bit screwy in this area and should probably be refactored. It shouldn't be so complicated as my first example.

@maresb
Copy link
Owner

maresb commented Dec 12, 2024

Could you clarify exactly what you mean by this?

If you want to use setuptools-scm directly, perhaps you are missing this?

[tool.setuptools_scm]
version_scheme = "no-guess-dev"

I have doubts about if the raw-options parameter was good design, but design is hard.

@dornech
Copy link
Author

dornech commented Dec 12, 2024

I tried with the setuptools_scm setting as you described (this is what I meant with providing the setting directly in a separate pyproject.toml table) but as mentioned but it did not work for me. I also do not really like to have that kind of double setting but on the other hand one would not change it to often ...

@dornech
Copy link
Author

dornech commented Dec 13, 2024

I am afraid your hatchling based solution suffers from one issue if executed from within a program (at least on Windows): when you start the subprocess, the Script-directory containing the hatchling.exe stub needs to be in the environment path which is not necessarily the case - at least not in my WinPython installation.

Actually I tried to do the same before with calling hatch but hatchling - and switched to the CliRunner approach exactly because of this problem. And i did not want to over-engineer this footgun code. This is also the reason why I did not want to add to much code to overcome the abovementioned problem with the parameter tables in pyproject.toml

@maresb
Copy link
Owner

maresb commented Dec 13, 2024

I tried with the setuptools_scm setting as you described (this is what I meant with providing the setting directly in a separate pyproject.toml table) but as mentioned but it did not work for me.

That sounds like a straight-up bug in setuptools-scm, since the feature is documented here.

I am afraid your hatchling based solution suffers from one issue...

Ya, the subprocess is far from ideal. I think we should refactor hatchling so that the data is accessible programatically.

@dornech
Copy link
Author

dornech commented Dec 13, 2024

I am afraid I am the wrong one to talk to about refactoring hatchling :-)

@maresb
Copy link
Owner

maresb commented Dec 14, 2024

After going fairly deeply into Hatchling, I got it down to:

from pathlib import Path

from hatchling.metadata.core import ProjectMetadata
from hatchling.plugin.manager import PluginManager


def get_version(root: Path) -> str:
    metadata = ProjectMetadata(str(root), PluginManager())
    # Version can be either statically set in pyproject.toml or computed dynamically:
    return metadata.core.version or metadata.hatch.version.cached

Seems pretty reasonable to me. What do you think?

Then we wrap this in something that detects whether hatchling is installed, and otherwise fall back to importlib.

@dornech
Copy link
Author

dornech commented Dec 16, 2024

Well, to be honest: I was use hatch and so I like the shorter solution via hatch (wich can be compacted to a one-liner) not careing about internal structure :-)

@maresb
Copy link
Owner

maresb commented Dec 16, 2024

Makes sense. Thanks a lot for all the feedback @dornech!

@maresb
Copy link
Owner

maresb commented Dec 27, 2024

I submitted pypa/hatch#1870, and that increases my confidence that my final solution above is correct.

@dornech
Copy link
Author

dornech commented Dec 28, 2024

I am sure you doing a great job but honestly - I am afraid to be quite far away from being familiar with the Hatchling internals to understand the details. What I guess to understand is you mainly switch from property to cached_property - what easens code (avoid if clauses in the hatchling code because then hidden in the decorator)
Just for my understanding: It does not (yet) mean that your PR would make the footgun code obsolete, right?

@maresb
Copy link
Owner

maresb commented Dec 28, 2024

Right, that PR is a small incremental step towards clearing up the versioning code. It is already much bigger than PRs should be, so I will attempt to get it merged before doing more work.

For the moment, that PR is is a pure refactor, so it is not relevant for simplifying this functionality. It's just that now I think I understand the hatchling internals enough that I believe this is probably correct. But we will see if there are any corrections to my PR.

@maresb
Copy link
Owner

maresb commented Jan 1, 2025

Should probably submit a docs PR to hatch-vcs / setuptools_scm about making version_scheme more prominent...

@maresb
Copy link
Owner

maresb commented Jan 3, 2025

Done for setuptools-scm in pypa/setuptools-scm#1094.
TODO: hatch-vcs...

@maresb
Copy link
Owner

maresb commented Jan 4, 2025

Done for hatch-vcs in ofek/hatch-vcs#85.

@dornech, I just tested switching over to setuptools-scm and including the following in my pyproject.toml and it worked for me:

[tool.setuptools_scm]
version_scheme = "no-guess-dev"

I think I've now done everything there is to do regarding this, so I'll close it out. Please feel free to open a new issue if you run into any more issues, or if you have any comments on my recent changes, and thanks a lot for the feedback!

@maresb maresb closed this as completed Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants