refactor: attempt to modernise build #294

grawlinson · 2023-02-07T08:29:19Z

First attempt at fixing issue #290.

TomasTomecek

Perfect! Just one question about the build frontend and we're good to merge

also please rebase to get changes from #293

pyproject.toml

TomasTomecek · 2023-02-07T12:48:36Z

Also please suggest how to obtain current version, that's what the build error is:

2023-02-07 08:31:57.646 base_git.py       INFO   Using user-defined script for ActionName.get_current_version: [['python3', 'setup.py', '--version']]
2023-02-07 08:31:57.646 commands.py       DEBUG  Command: python3 setup.py --version
2023-02-07 08:31:57.660 logging.py        INFO   python3: can't open file '/tmp/tmpsarfl_k2/setup.py': [Errno 2] No such file or directory
2023-02-07 08:31:57.663 commands.py       ERROR  Command 'python3 setup.py --version' failed.
2023-02-07 08:31:57.663 commands.py       DEBUG  Command stderr: python3: can't open file '/tmp/tmpsarfl_k2/setup.py': [Errno 2] No such file or directory

I'm happy to fix that and push to your branch if you don't want to mess with RPM packaging 😁

grawlinson · 2023-02-07T19:13:18Z

Also please suggest how to obtain current version, that's what the build error is:

2023-02-07 08:31:57.646 base_git.py       INFO   Using user-defined script for ActionName.get_current_version: [['python3', 'setup.py', '--version']]
2023-02-07 08:31:57.646 commands.py       DEBUG  Command: python3 setup.py --version
2023-02-07 08:31:57.660 logging.py        INFO   python3: can't open file '/tmp/tmpsarfl_k2/setup.py': [Errno 2] No such file or directory
2023-02-07 08:31:57.663 commands.py       ERROR  Command 'python3 setup.py --version' failed.
2023-02-07 08:31:57.663 commands.py       DEBUG  Command stderr: python3: can't open file '/tmp/tmpsarfl_k2/setup.py': [Errno 2] No such file or directory

I'm happy to fix that and push to your branch if you don't want to mess with RPM packaging 😁

That would be great!

I’m off on holiday for a few days, so I won’t be able to do much until I get back.

First attempt at fixing issue #290.

TomasTomecek · 2023-02-08T12:16:47Z

@grawlinson LGTM from my side, enjoy your vacation!

once you're back, let me know if you'd like to do more changes here

TomasTomecek · 2023-02-09T11:31:41Z

pyproject.toml

+# setuptools: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
+# setuptools-scm: https://github.com/pypa/setuptools_scm/#pyprojecttoml-usage
+[build-system]
+requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2", "wheel"]


I just tested this PR locally in a fedora:36 container and it worked fine with python3-setuptools-59.6.0-3.fc36.noarch so I'm thinking that maybe we don't need to pin the version of setuptools

fascinating, even works with a very old setuptools - python3-setuptools-53.0.0-12.el9.noarch

I suspect it's because build creates a venv and installs new setuptools inside: sadly the output doesn't say which versions

Yes, build creates its own env but you can pass a CLI option to prevent it from managing the env.

Note that ancient setuptools wouldn't know how to read pyproject.toml.

.packit.yaml

webknjaz · 2023-02-09T13:32:58Z

.packit.yaml

  get-current-version:
-    - "python3 setup.py --version"
+    - "python3 -m setuptools_scm"


This may need piping through awk because it may output extra text.

yes, that's possible and it already happened: the build fails then and needs investigation

pyproject.toml

webknjaz · 2023-02-09T13:35:52Z

pyproject.toml

+# setuptools: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
+# setuptools-scm: https://github.com/pypa/setuptools_scm/#pyprojecttoml-usage
+[build-system]
+requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2", "wheel"]


Note that wheel is not needed here. Setuptools adds it for wheel builds automatically.

Suggested change

requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2", "wheel"]

requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2"]

webknjaz · 2023-02-09T13:36:58Z

pyproject.toml

+# setuptools: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
+# setuptools-scm: https://github.com/pypa/setuptools_scm/#pyprojecttoml-usage
+[build-system]
+requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2", "wheel"]


Yes, build creates its own env but you can pass a CLI option to prevent it from managing the env.

webknjaz · 2023-02-09T13:37:29Z

pyproject.toml

+# setuptools: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
+# setuptools-scm: https://github.com/pypa/setuptools_scm/#pyprojecttoml-usage
+[build-system]
+requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2", "wheel"]


Note that ancient setuptools wouldn't know how to read pyproject.toml.

webknjaz

Oh, I forgot another important bit: for setuptools-scm to work properly with downloads from GitHub, you must update .git_archival.txt. See their readme for details, it's pretty straightforward.

* use setuptools_scm directly to get version * use python3-build to create dists Signed-off-by: Tomas Tomecek <[email protected]>

docs: https://src.fedoraproject.org/rpms/pyproject-rpm-macros/ good luck

TomasTomecek · 2023-02-13T11:46:44Z

Oh, I forgot another important bit: for setuptools-scm to work properly with downloads from GitHub, you must update .git_archival.txt. See their readme for details, it's pretty straightforward.

just checked and we already have that set up :)

webknjaz · 2023-02-13T12:42:47Z

just checked and we already have that set up :)

@TomasTomecek the file exists but the contents are outdated — there's a newer "match" bit still missing.

TomasTomecek · 2023-02-13T15:09:52Z

@webknjaz you're correct. I just checked what the option does exactly - luckily it's easy to play with:

The usual tags we have here:

$ git describe --tags --dirty --long --match="*[0-9]*"
0.10.1-6-g8166839

$ git describe --tags --dirty --long
0.10.1-6-g8166839

Let's do an "ugly" tag:

$ git tag foo-bar

$ git describe --tags --dirty --long
foo-bar-0-g8166839

$ git describe --tags --dirty --long --match="*[0-9]*"
0.10.1-6-g8166839

So the match option prevents weird versions from weird tags.

I don't think we need that option but agreed it could prevent odd behaviour in the future. Let's apply it if no one objects.

webknjaz · 2023-02-13T15:42:38Z

Those updates will only work with modern Git, they'll allow installing from non-tagged commits and getting proper intermediate versions.

@webknjaz

new recomentation from setuptools_scm thanks @webknjaz Signed-off-by: Tomas Tomecek <[email protected]>

@webknjaz

@webknjaz says setuptools adds it automatically Signed-off-by: Tomas Tomecek <[email protected]>

Signed-off-by: Tomas Tomecek <[email protected]>

TomasTomecek · 2023-03-13T12:42:28Z

I implemented the last bits of feedback. Let's merge after all's green.

TomasTomecek · 2023-03-13T13:14:28Z

the failing test is a network DNS flake, merging

gotmax23 · 2023-03-13T14:32:51Z

This breaks compatibility with EL 9 and Fedora 36.

gotmax23 · 2023-03-13T14:37:54Z

I'd recommend implementing this in the same way we did for specfile. Remove the setup.py, keep the metadata in setup.cfg sans setup_requires, and add a minimal pyproject.toml.

TomasTomecek · 2023-03-13T14:44:00Z

I'm not concerned about EL9 since there are no EPEL9 builds: https://src.fedoraproject.org/rpms/ansible-bender

Folks installing from PyPI can upgrade their setuptools.

Similarly F36, gonna be EOL within 2 months.

It will take a long time before this gets released given my velocity of a sloth.

Nevertheless, I'm okay changing the python packaging as long as it works.

gotmax23 · 2023-03-13T16:29:34Z

I've submitted #296. @webknjaz and @grawlinson, feel free to take a look as well :).

TomasTomecek reviewed Feb 7, 2023

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

refactor: attempt to modernise build

54252c8

First attempt at fixing issue #290.

TomasTomecek reviewed Feb 9, 2023

View reviewed changes

webknjaz reviewed Feb 9, 2023

View reviewed changes

webknjaz suggested changes Feb 9, 2023

View reviewed changes

TomasTomecek added 2 commits February 13, 2023 12:41

packit: utilize the new python packaging

1364335

* use setuptools_scm directly to get version * use python3-build to create dists Signed-off-by: Tomas Tomecek <[email protected]>

rpm: utilize the new python packaging

8166839

docs: https://src.fedoraproject.org/rpms/pyproject-rpm-macros/ good luck

TomasTomecek added 3 commits March 13, 2023 13:40

git_archival: add match

e2660e4

new recomentation from setuptools_scm thanks @webknjaz Signed-off-by: Tomas Tomecek <[email protected]>

pyproject.toml: drop wheel

ee2e085

@webknjaz says setuptools adds it automatically Signed-off-by: Tomas Tomecek <[email protected]>

packit.yaml: consistent "-" indent

592c88e

Signed-off-by: Tomas Tomecek <[email protected]>

TomasTomecek merged commit 692618c into ansible-community:master Mar 13, 2023

gotmax23 mentioned this pull request Mar 13, 2023

Restore compatability with older setuptools versions and cleanup specfile #296

Merged

gotmax23 mentioned this pull request Mar 14, 2023

Miscellaneous RPM packaging improvements #297

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: attempt to modernise build #294

refactor: attempt to modernise build #294

grawlinson commented Feb 7, 2023

TomasTomecek left a comment

TomasTomecek commented Feb 7, 2023

grawlinson commented Feb 7, 2023

TomasTomecek commented Feb 8, 2023

TomasTomecek Feb 9, 2023

TomasTomecek Feb 9, 2023

webknjaz Feb 9, 2023

webknjaz Feb 9, 2023

webknjaz Feb 9, 2023

TomasTomecek Feb 9, 2023

webknjaz Feb 9, 2023

webknjaz Feb 9, 2023

webknjaz Feb 9, 2023

webknjaz left a comment

TomasTomecek commented Feb 13, 2023

webknjaz commented Feb 13, 2023

TomasTomecek commented Feb 13, 2023

webknjaz commented Feb 13, 2023

TomasTomecek commented Mar 13, 2023

TomasTomecek commented Mar 13, 2023

gotmax23 commented Mar 13, 2023

gotmax23 commented Mar 13, 2023

TomasTomecek commented Mar 13, 2023

gotmax23 commented Mar 13, 2023

	requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2", "wheel"]
	requires = ["setuptools>=61", "setuptools-scm[toml]>=6.2"]

refactor: attempt to modernise build #294

refactor: attempt to modernise build #294

Conversation

grawlinson commented Feb 7, 2023

TomasTomecek left a comment

Choose a reason for hiding this comment

TomasTomecek commented Feb 7, 2023

grawlinson commented Feb 7, 2023

TomasTomecek commented Feb 8, 2023

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

webknjaz left a comment

Choose a reason for hiding this comment

TomasTomecek commented Feb 13, 2023

webknjaz commented Feb 13, 2023

TomasTomecek commented Feb 13, 2023

webknjaz commented Feb 13, 2023

TomasTomecek commented Mar 13, 2023

TomasTomecek commented Mar 13, 2023

gotmax23 commented Mar 13, 2023

gotmax23 commented Mar 13, 2023

TomasTomecek commented Mar 13, 2023

gotmax23 commented Mar 13, 2023