[RFC] [PoC] [WIP] Implement dynamic (SCM) version loader #672

webknjaz · 2018-11-25T15:33:54Z

This PR provides a sketch of how the developed dist version hardcoding could be avoided.
It is not a final implementation, but rather a request for comments.

I'd like to identify:

Where is the right place to place this architecturally
What should be the public API to this feature.
Currently it's a setup.cfg-like "attr: " tag referencing to an importable callable, which gives users a lot of flexibility while preserving poetry's built-in validation mechanisms
Should it rely on one version getter implementation or be flexibe?
Should it vendor some implementations or just have external/optional dependencies?
What should be user experience when they use this feature?
How does it influence the metadata?

Pull Request Check List

Added tests for changed code.
Updated documentation for changed code.

Resolves python-poetry#140 Resolves python-poetry#185 Resolves python-poetry#284

webknjaz · 2018-11-25T15:36:35Z

cc @kevinkjt2000 @madig @escaped @RonnyPfannschmidt @glehmann @seglberg @whoiswes @psafont ^^

RonnyPfannschmidt · 2018-11-25T16:10:49Z

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "poetry"
-version = "0.12.8"
+version = "attr: setuptools_scm.get_version"


would there be a way to enable setuptools_scm to provide the version more directly?

Currently, version arg is read unconditionally + I didn't notice any hooks in poetry

Of course calling setuptools_scm could be hardcoded in python source, but that's a question about flexibility

What is meant by this question? It looks to me like the setuptools_scm library is directly responsible for providing the version in this example.

@kevinkjt2000 This example only shows that users can refer to any callable they want. setuptools_scm has been chosen as a common example but any function will work. That's flexibility.
OTOH @RonnyPfannschmidt seems to want smth like use_version scm = True to work without having to refer to the function explicitly.

@kevinkjt2000 for example in the setuptools integration we trigger the integration and the file finder via the setuptools plugin system

for the poetry integration something similar could be thinkable

I'm a mediocre dev at best and can't intelligently comment, but I would prefer to pass a callable to the config versus offering any integrations. That seems to offer the most flexibility and least complexity in my book.

I may have a lack of understanding here, but to trigger the integration and the file finder via the setuptools plugin system is that not what the get_version function does?

@kevinkjt2000 setuptools has entry_point-based hook system. Other packages can specify their entry points with those name to "register" their hook implementations and setuptools then automatically knows it can use them. Example: https://github.com/pypa/setuptools_scm/blob/master/setup.py#L69

kevinkjt2000

I like it. Accepting a callable function should allow users to do what they want 😄
Still needs tests, of course.

kevinkjt2000 · 2018-11-25T16:21:40Z

poetry/packages/project_package.py

@@ -8,6 +8,23 @@

 class ProjectPackage(Package):
    def __init__(self, name, version, pretty_version=None):
+        if isinstance(version, str) and version.startswith('attr:'):
+            version_pkg, _, version_attr = (
+                version[6:].strip().rpartition('.')


Should "attr:function_name" be allowed, without a space after the colon?

Why not? It's harmless and it's how it works in setuptools.

kevinkjt2000 · 2018-11-25T16:23:23Z

poetry/packages/project_package.py

@@ -8,6 +8,23 @@

 class ProjectPackage(Package):
    def __init__(self, name, version, pretty_version=None):
+        if isinstance(version, str) and version.startswith('attr:'):


The init code would look a little cleaner by moving this code into a new function.

Sure, it's just a PoC.

kevinkjt2000 · 2018-11-25T16:25:34Z

poetry/packages/project_package.py

+            if version is None:
+                raise RuntimeError(
+                    'The version getter callable must return a string '
+                    'or a Version instance'


The message here does not match what the conditional is checking.

Yes, this should be fixed.

kevinkjt2000 · 2018-11-25T16:46:04Z

poetry/packages/project_package.py

+                __import__(version_pkg, fromlist=(version_pkg, )),
+                version_attr,
+            )
+            version = version_getter()


People may want to import a variable instead of a function. The setuptools_scm documentation shows an example in the programmatic usage section https://github.com/pypa/setuptools_scm#programmatic-usage
Checking if the attribute is callable would allow for this. https://docs.python.org/3/library/functions.html#callable

That's different. Also the callable may as well return that constant.

Constant declaration is already supported by setting it in pyproject.toml. There should be only one way to do it.

The variable is dynamically determined; I do not think this is already supported by pyproject.toml. I think adding a small function wrapper that returns a variable is an acceptable thing. But a small if check for whether or not it is callable would support both. Tiny quality of life thing that saves writing a tiny function for people who copy/paste from setuptools_scm docs.

You're suggesting to put import-time evaluation in place, which is exactly what doomed setuptools. And now because of backwards compat they cannot fix this.
That's why I'm -1 here.
If one wants to access the installed version info from their code we could provide them a helper function to do so but let's not do import-time version detection. After all, app needs the version retrieved from pkg_resources in installed mode, it shouldn't invoke version-guessing flows in this case.

What is the difference in what is accomplished between a variable that holds the version, and a function that evaluates to a variable that holds the version? I was suggesting this:

if callable(version_getter): version = version_getter() else: version = version_getter

The difference is that it encourages users to have

version = blah_blah()

called on during the module import, which is bad.

glehmann · 2018-11-25T20:37:51Z

@warner, the python-versioneer author, might also be interested in this discussion :)

webknjaz · 2018-11-25T21:02:34Z

@RonnyPfannschmidt would it make sense to reuse setuptools' entry points to retrieve the version getter from integrations? Do you think this'll make existing ones "just work"?

RonnyPfannschmidt · 2018-11-25T21:43:05Z

@webknjaz it would be highly problematic to try and replicate any of the hook systems of setuptools with sanity - it would never amount to more than a really ugly hack - i strongly advise towards having own integration points that match the design of poetry

webknjaz · 2018-11-25T21:44:12Z

Good point. Let's see what @sdispater thinks...

webknjaz · 2018-11-26T01:35:13Z

It looks like poetry has a similar plugin system: https://poetry.eustace.io/docs/pyproject/#plugins

webknjaz · 2018-11-30T15:16:48Z

@sdispater what do you think?

RonnyPfannschmidt · 2018-11-30T15:20:27Z

@webknjaz as far as i understood, what you linked is how poetry declares entry-points - thats not yet an actual extension system, just metadata that pkg_resources or the entrypoints package may consume

webknjaz · 2018-11-30T15:23:25Z

@RonnyPfannschmidt it seems you're right, but it's the closest I've managed to google

psafont · 2018-11-30T17:05:10Z

I'd also change the behaviour of poetry version to avoid conflicts with this feature.

Consider showing a message on how to tag the commit with the scm when a function is detected on the version field.

webknjaz · 2018-11-30T17:07:11Z

Good catch!

webknjaz · 2018-11-30T17:08:03Z

I won't make any changes to PR until I hear from @sdispater to avoid having to remake everything multiple times :)

sdispater · 2018-11-30T22:49:27Z

Thanks for taking the time to make this PR.

I will hold off this feature for now though. This is until Poetry can provide a proper plugin system so that people can opt in this behavior by installing the correct plugin.

I can't give you an ETA since implementing a plugin system might take some time.

webknjaz · 2018-11-30T22:52:19Z

Sad to hear. Can we help with this somehow? This is one of the features which is holding me from switching to Poetry completely.

webknjaz · 2018-12-01T18:49:22Z

Is there any architectural vision on plugins at all, currently?

Ref #658

webknjaz · 2018-12-01T20:05:03Z

I thought I'd raise an actual issue about the plugin system: #693.

zeebonk · 2019-01-06T10:27:39Z

@webjnjaz thanks for putting the time in this. This is also holding me back from using poetry.

webknjaz · 2019-01-06T14:47:52Z

@zeebonk it looks like it's in progress by @sdispater already: https://github.com/sdispater/poetry/projects/1. And it's been added to https://github.com/sdispater/poetry/milestone/1

zeebonk · 2019-01-07T10:16:38Z

@webknjaz that's great news! Subscribed :)

sdispater · 2019-03-06T21:26:53Z

Closing this for now. I'll ping you when the plugin system is implemented.

webknjaz · 2019-03-06T21:29:14Z

Okay, thanks

github-actions · 2024-03-01T16:08:50Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

webknjaz added 3 commits November 25, 2018 16:23

Add setuptools-scm(-git-archive) as extra deps

d603188

Get version using an arbitraty executable

856e326

Use SCM-based version in the Poetry itself

daf93d6

Resolves python-poetry#140 Resolves python-poetry#185 Resolves python-poetry#284

webknjaz mentioned this pull request Nov 25, 2018

Support automatic versioning setuptools_scm-style? #140

Closed

RonnyPfannschmidt reviewed Nov 25, 2018

View reviewed changes

kevinkjt2000 reviewed Nov 25, 2018

View reviewed changes

webknjaz changed the title ~~[PoC] [WIP] Implement dynamic (SCM) version loader~~ [RFC] [PoC] [WIP] Implement dynamic (SCM) version loader Nov 25, 2018

This was referenced Dec 1, 2018

automatic version à la python-versioneer #284

Closed

Integrate setuptools-scm #185

Closed

[Discussion] Poetry to provide a decent hook-based plugin system #693

Closed

sdispater closed this Mar 6, 2019

aequitas mentioned this pull request Apr 13, 2019

[WIP] Allow to manage project version with git tags #1034

Closed

2 tasks

asodeur mentioned this pull request Jul 4, 2019

Add ability to append build number to version identifier at build time #1208

Closed

2 tasks

baldurmen mentioned this pull request Jul 25, 2023

[FEATURE] Implement dynamic versioning for releases lidatong/dataclasses-json#446

Closed

github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024

[RFC] [PoC] [WIP] Implement dynamic (SCM) version loader #672

[RFC] [PoC] [WIP] Implement dynamic (SCM) version loader #672

Conversation

webknjaz commented Nov 25, 2018 • edited Loading

Pull Request Check List

webknjaz commented Nov 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz Nov 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkjt2000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glehmann commented Nov 25, 2018

webknjaz commented Nov 25, 2018

RonnyPfannschmidt commented Nov 25, 2018

webknjaz commented Nov 25, 2018

webknjaz commented Nov 26, 2018

webknjaz commented Nov 30, 2018

RonnyPfannschmidt commented Nov 30, 2018

webknjaz commented Nov 30, 2018 • edited Loading

psafont commented Nov 30, 2018

webknjaz commented Nov 30, 2018

webknjaz commented Nov 30, 2018

sdispater commented Nov 30, 2018

webknjaz commented Nov 30, 2018 • edited Loading

webknjaz commented Dec 1, 2018

webknjaz commented Dec 1, 2018

zeebonk commented Jan 6, 2019

webknjaz commented Jan 6, 2019

zeebonk commented Jan 7, 2019

sdispater commented Mar 6, 2019

webknjaz commented Mar 6, 2019

github-actions bot commented Mar 1, 2024

webknjaz commented Nov 25, 2018 •

edited

Loading

webknjaz Nov 25, 2018 •

edited

Loading

webknjaz commented Nov 30, 2018 •

edited

Loading

webknjaz commented Nov 30, 2018 •

edited

Loading