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.

Sign up for GitHub

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

Jump to bottom

Restore compatability with older setuptools versions and cleanup specfile #296

Merged
merged 2 commits into from
Mar 24, 2023
Merged

Restore compatability with older setuptools versions and cleanup specfile #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd4914c
Revert "refactor: attempt to modernise build"
gotmax23 Mar 13, 2023
44d20d5
setup.cfg: don't mark wheel as universal
gotmax23 Mar 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 1 addition & 56 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,58 +1,3 @@
[project]
name = "ansible-bender"
authors = [
{name = "Tomas Tomecek", email = "tomas@tomecek.net"},
]
description = "A tool which builds container images using Ansible playbooks"
readme = "README.md"
requires-python = ">= 3.6"
keywords = [
"ansible",
"containers",
"linux",
"buildah",
]
license = {text = "MIT"}
classifiers = [
"Development Status :: 5 - Production/Stable",
"Environment :: Console",
"Intended Audience :: Developers",
"License :: OSI Approved :: MIT License",
"Operating System :: POSIX :: Linux",
"Programming Language :: Python",
"Programming Language :: Python :: 3.6",
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Topic :: Software Development",
"Topic :: Utilities",
]
dependencies = [
"PyYAML",
"tabulate",
"jsonschema",
"setuptools",
# https://peps.python.org/pep-0631/
"importlib_metadata; python_version < '3.8'",
]
dynamic = ["version"]

[project.urls]
homepage = "https://github.com/ansible-community/ansible-bender"
documentation = "https://ansible-community.github.io/ansible-bender/build/html/index.html"

[project.optional-dependencies]
testing = [
"pytest",
"flexmock",
"pytest-cov",
]

[project.scripts]
ansible-bender = "ansible_bender.cli:main"

# reference for usage:
# https://github.com/pypa/setuptools_scm/#pyprojecttoml-usage
[tool.setuptools_scm]
Expand All @@ -61,5 +6,5 @@ ansible-bender = "ansible_bender.cli:main"
# 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"]
requires = ["setuptools", "setuptools-scm[toml]"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's usually a good idea to declare minimum versions that have the required features...
While pypa/build would normally install the latest compatible versions, somebody may want to go for reproducibile builds and pin the build dep versions via PIP_CONSTRAINT, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to arbitrarily pin versions. If there's a feature that the project uses that is known to require a certain minimum version of setuptools(_scm), let me know. We could pin setuptools to whenever build_meta was added, but I don't think that's common practice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinning is a separate topic. It's not something you'd put in pyproject.toml, of course.
But the minimum versions are usually a good idea — it is a metadata bit that shows what build dep versions have the features used.

FWIW, I think that the [toml] extra was added in setuptools-scm v6 and the git archive plugin was absorbed in v7.
As for setuptools, v40.9.0 added the support for setup.py-less projects, though I only documented it in the upstream docs in v59.0.0.

So realistically, this is what's minimal:

Suggested change
requires = ["setuptools", "setuptools-scm[toml]"]
requires = [
"setuptools >= 40.9.0", # required for `setup.py`-less builds
"setuptools-scm[toml] >= 7", # supports .git_archival.txt for non-tagged commits
]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could also be useful to have a smoke test in GHA with different PIP_CONSTRAINT setting the oldest supported build deps pins and something in the middle, as well as bleeding-edge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinning is a separate topic. It's not something you'd put in pyproject.toml, of course.

I often say pinning when I mean setting a minimum version.

FWIW, I think that the [toml] extra was added in setuptools-scm v6 and the git archive plugin was absorbed in v7.

I prefer to maintain compatibility and not pin to >= 7.

As for setuptools, v40.9.0 added the support for setup.py-less projects, though I only documented it in the upstream docs in v59.0.0.

The setuptools docs don't recommend this, but I can add that.

It could also be useful to have a smoke test in GHA with different PIP_CONSTRAINT setting the oldest supported build deps pins and something in the middle, as well as bleeding-edge.

That can be separate. PRs should have a clear/limitted scope.

Copy link
Member

@webknjaz webknjaz Mar 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to maintain compatibility and not pin to >= 7.

Then, specify a lower version. Note that prior to v7, the support for .git_archival.txt comes from a plugin.

Determine which old Python versions you want to work with setuptools-scm-git-archive and use env markers to make it a conditional dependency. And you'll need two conditional entries for setuptools-scm — the one for newer Pythons would require setuptools-scm v7+ and the old would have the same marker as the plugin and could be something like ~= 6.0 (earlier versions would not be able to pick up the config in pyproject.toml, I think). I'm almost sure you can't combine the plugin and the setuptools-scm version that incorporates the same features.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinning is a separate topic. It's not something you'd put in pyproject.toml, of course.

I often say pinning when I mean setting a minimum version.

FWIW, I think that the [toml] extra was added in setuptools-scm v6 and the git archive plugin was absorbed in v7.

I prefer to maintain compatibility and not pin to >= 7.

As for setuptools, v40.9.0 added the support for setup.py-less projects, though I only documented it in the upstream docs in v59.0.0.

The setuptools docs don't recommend this, but I can add that.

The docs are sometimes incomplete so we end up relying on the tribal knowledge. I sometimes add the docs or review the docs PRs myself.
The only reason setup.py used to be needed in the last ≈4 years, was for editable installs. But PEP 660 has been supported in pip and setuptools for a while now, making the legacy fallback completely unnecessary. This is the current standard that replaced the old mechanism that never had a standard.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That can be separate. PRs should have a clear/limitted scope.

Absolutely. The PRs should be atomic. I just wanted to include this information while on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to maintain compatibility and not pin to >= 7.

Then, specify a lower version. Note that prior to v7, the support for .git_archival.txt comes from a plugin.

Determine which old Python versions you want to work with setuptools-scm-git-archive and use env markers to make it a conditional dependency. And you'll need two conditional entries for setuptools-scm — the one for newer Pythons would require setuptools-scm v7+ and the old would have the same marker as the plugin and could be something like ~= 6.0 (earlier versions would not be able to pick up the config in pyproject.toml, I think). I'm almost sure you can't combine the plugin and the setuptools-scm version that incorporates the same features.

I'm fine with .git_archival support not always being available. It doesn't work with Github's generated archives anyways (try pip install https://github.com/pypa/setuptools_scm/archive/main.tar.gz and see what happens). In the unlikely event that someone manually generates a tarball with git archive and pip installs it, they should get the newest setuptools_scm version thanks to build isolation. Linux distribution packagers or regular pip users use the the PyPI sdist that doesn't need this at all.

The only reason setup.py used to be needed in the last ≈4 years, was for editable installs.

or for "stable enterprise" linux distributions :). The way that setuptools now handles editable installs still isn't great (you need --config-settings editable_mode=compat to prevent it from using the runtime import hook approach that breaks static analysis tools), but I digress...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try pip install https://github.com/pypa/setuptools_scm/archive/main.tar.gz and see what happens

Interesting... That's definitely a bug that should be reported upstream. But, for the projects using it, it actually works. Try pip install install https://github.com/python-attrs/attrs/archive/main.tar.gz and you'll see Successfully installed attrs-22.2.1.dev39, for example.

Linux distribution packagers or regular pip users use the the PyPI sdist that doesn't need this at all.

Most. But I've also seen some building from SCM.

or for "stable enterprise" linux distributions :)

There's no reason they can't echo 'import setuptools; setuptools.setup()' > setup.py which is effectively what setuptools does in-memory, anyway.

build-backend = "setuptools.build_meta"
49 changes: 49 additions & 0 deletions setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,57 @@
[metadata]
name = ansible-bender
url = https://github.com/ansible-community/ansible-bender
description = A tool which builds container images using Ansible playbooks
long_description = file: README.md
long_description_content_type = text/markdown
author = Tomas Tomecek
author_email = tomas@tomecek.net
license = MIT
license_file = LICENSE
classifiers =
Development Status :: 5 - Production/Stable
gotmax23 marked this conversation as resolved.
Show resolved Hide resolved
Environment :: Console
Intended Audience :: Developers
License :: OSI Approved :: MIT License
Operating System :: POSIX :: Linux
Programming Language :: Python
Programming Language :: Python :: 3.6
Programming Language :: Python :: 3.7
Programming Language :: Python :: 3.8
Programming Language :: Python :: 3.9
Programming Language :: Python :: 3.10
Programming Language :: Python :: 3.11
Topic :: Software Development
Topic :: Utilities
keywords =
ansible
containers
linux
buildah


[options]
python_requires = >=3.6
packages = find:
include_package_data = True

install_requires =
PyYAML
tabulate
jsonschema
importlib_metadata; python_version < '3.8'

[options.packages.find]
exclude =
tests
tests.*

[options.extras_require]
testing =
pytest
flexmock
pytest-cov

[options.entry_points]
console_scripts =
ansible-bender = ansible_bender.cli:main