pyproject-transition #922

Merged
witten merged 7 commits from kaliko/borgmatic:pyproject-transition into main 2024-10-26 23:38:35 +00:00
6 changed files with 67 additions and 71 deletions

View File

@ -1 +0,0 @@
select = Q0

View File

@ -1,3 +1,62 @@
[project]
name = "borgmatic"
version = "1.9.0.dev0"
authors = [
{ name="Dan Helfman", email="witten@torsion.org" },
]
description = "Simple, configuration-driven backup software for servers and workstations"
readme = "README.md"
requires-python = ">=3.8"
classifiers=[
"Development Status :: 5 - Production/Stable",
"Environment :: Console",
"Intended Audience :: System Administrators",
"License :: OSI Approved :: GNU General Public License v3 (GPLv3)",
"Programming Language :: Python",
"Topic :: Security :: Cryptography",
"Topic :: System :: Archiving :: Backup",
]
dependencies = [
"colorama>=0.4.1,<0.5",
"jsonschema",
"packaging",
"requests",
"ruamel.yaml>0.15.0",
]
[project.scripts]
borgmatic = "borgmatic.commands.borgmatic:main"
generate-borgmatic-config = "borgmatic.commands.generate_config:main"
validate-borgmatic-config = "borgmatic.commands.validate_config:main"
[project.optional-dependencies]
Apprise = ["apprise"]
[project.urls]
Homepage = "https://torsion.org/borgmatic"
[build-system]
requires = ["setuptools>=61.0"]
# allow looking for conf in setup.{cfg,py}
#build-backend = "setuptools.build_meta"
[tool.setuptools.packages.find]
include = ["borgmatic*"]
namespaces = false
[tool.black]
line-length = 100
skip-string-normalization = true
[tool.pytest.ini_options]
testpaths = "tests"
addopts = "--cov-report term-missing:skip-covered --cov=borgmatic --ignore=tests/end-to-end"
[tool.isort]
profile = "black"
known_first_party = "borgmatic"
line_length = 100
skip = ".tox"
[tool.codespell]
witten marked this conversation as resolved
Review

This pyproject.toml looks good to me. But it seems to result in an unimportable borgmatic module, I'm guessing because setup.py is gone and something is still relying on it to consider borgmatic a module.

For instance, when I run tests, a particular test that tries to run the borgmatic executable fails:

$ tox
...
E               subprocess.CalledProcessError: Command '('borgmatic', '--version')' returned non-zero exit status 1.

/usr/lib/python3.12/subprocess.py:571: CalledProcessError
-------------------------------------------------------------- Captured stderr call --------------------------------------------------------------
Traceback (most recent call last):
  File "/home/witten/Downloads/borgmatic/.tox/py312/bin/borgmatic", line 5, in <module>
    from borgmatic.commands.borgmatic import main
ModuleNotFoundError: No module named 'borgmatic.commands'
...

And I can reproduce this by manually running borgmatic in the virtualenv:

$ . .tox/py312/bin/activate
$ borgmatic --version
Traceback (most recent call last):
  File "/home/witten/Downloads/borgmatic/.tox/py312/bin/borgmatic", line 5, in <module>
    from borgmatic.commands.borgmatic import main
ModuleNotFoundError: No module named 'borgmatic.commands'

So I'm not sure what's missing here.

This `pyproject.toml` looks good to me. But it seems to result in an unimportable `borgmatic` module, I'm guessing because `setup.py` is gone and something is still relying on it to consider `borgmatic` a module. For instance, when I run tests, a particular test that tries to run the `borgmatic` executable fails: ```bash $ tox ... E subprocess.CalledProcessError: Command '('borgmatic', '--version')' returned non-zero exit status 1. /usr/lib/python3.12/subprocess.py:571: CalledProcessError -------------------------------------------------------------- Captured stderr call -------------------------------------------------------------- Traceback (most recent call last): File "/home/witten/Downloads/borgmatic/.tox/py312/bin/borgmatic", line 5, in <module> from borgmatic.commands.borgmatic import main ModuleNotFoundError: No module named 'borgmatic.commands' ... ``` And I can reproduce this by manually running borgmatic in the virtualenv: ```bash $ . .tox/py312/bin/activate $ borgmatic --version Traceback (most recent call last): File "/home/witten/Downloads/borgmatic/.tox/py312/bin/borgmatic", line 5, in <module> from borgmatic.commands.borgmatic import main ModuleNotFoundError: No module named 'borgmatic.commands' ``` So I'm not sure what's missing here.
Review

Fixed with 601e393ec7 (pyproject/build failed to ship submodules).

Fixed with 601e393ec7 (pyproject/build failed to ship submodules).
skip = ".git,.tox,build"

View File

@ -33,8 +33,7 @@ git push github $version
# Build borgmatic and publish to pypi.
rm -fr dist
python3 setup.py bdist_wheel
python3 setup.py sdist
witten marked this conversation as resolved Outdated

Something is missing from the result tarball. Here's what I'm seeing on my machine:

$ tar tfz dist/borgmatic-1.9.0.dev0.tar.gz
borgmatic-1.9.0.dev0/
borgmatic-1.9.0.dev0/AUTHORS
borgmatic-1.9.0.dev0/LICENSE
borgmatic-1.9.0.dev0/MANIFEST.in
borgmatic-1.9.0.dev0/PKG-INFO
borgmatic-1.9.0.dev0/README.md
borgmatic-1.9.0.dev0/borgmatic/
borgmatic-1.9.0.dev0/borgmatic/__init__.py
borgmatic-1.9.0.dev0/borgmatic/config/
borgmatic-1.9.0.dev0/borgmatic/config/schema.yaml
borgmatic-1.9.0.dev0/borgmatic/execute.py
borgmatic-1.9.0.dev0/borgmatic/logger.py
borgmatic-1.9.0.dev0/borgmatic/signals.py
borgmatic-1.9.0.dev0/borgmatic/verbosity.py
borgmatic-1.9.0.dev0/borgmatic.egg-info/
borgmatic-1.9.0.dev0/borgmatic.egg-info/PKG-INFO
borgmatic-1.9.0.dev0/borgmatic.egg-info/SOURCES.txt
borgmatic-1.9.0.dev0/borgmatic.egg-info/dependency_links.txt
borgmatic-1.9.0.dev0/borgmatic.egg-info/entry_points.txt
borgmatic-1.9.0.dev0/borgmatic.egg-info/requires.txt
borgmatic-1.9.0.dev0/borgmatic.egg-info/top_level.txt
borgmatic-1.9.0.dev0/pyproject.toml
borgmatic-1.9.0.dev0/sample/
borgmatic-1.9.0.dev0/sample/systemd/
borgmatic-1.9.0.dev0/sample/systemd/borgmatic-user.service
borgmatic-1.9.0.dev0/sample/systemd/borgmatic-user.timer
borgmatic-1.9.0.dev0/sample/systemd/borgmatic.service
borgmatic-1.9.0.dev0/sample/systemd/borgmatic.timer
borgmatic-1.9.0.dev0/setup.cfg

Notice how most of the source subdirectories are missing: borgmatic/actions/, borgmatic/borg/, etc.

Something is missing from the result tarball. Here's what I'm seeing on my machine: ``` $ tar tfz dist/borgmatic-1.9.0.dev0.tar.gz borgmatic-1.9.0.dev0/ borgmatic-1.9.0.dev0/AUTHORS borgmatic-1.9.0.dev0/LICENSE borgmatic-1.9.0.dev0/MANIFEST.in borgmatic-1.9.0.dev0/PKG-INFO borgmatic-1.9.0.dev0/README.md borgmatic-1.9.0.dev0/borgmatic/ borgmatic-1.9.0.dev0/borgmatic/__init__.py borgmatic-1.9.0.dev0/borgmatic/config/ borgmatic-1.9.0.dev0/borgmatic/config/schema.yaml borgmatic-1.9.0.dev0/borgmatic/execute.py borgmatic-1.9.0.dev0/borgmatic/logger.py borgmatic-1.9.0.dev0/borgmatic/signals.py borgmatic-1.9.0.dev0/borgmatic/verbosity.py borgmatic-1.9.0.dev0/borgmatic.egg-info/ borgmatic-1.9.0.dev0/borgmatic.egg-info/PKG-INFO borgmatic-1.9.0.dev0/borgmatic.egg-info/SOURCES.txt borgmatic-1.9.0.dev0/borgmatic.egg-info/dependency_links.txt borgmatic-1.9.0.dev0/borgmatic.egg-info/entry_points.txt borgmatic-1.9.0.dev0/borgmatic.egg-info/requires.txt borgmatic-1.9.0.dev0/borgmatic.egg-info/top_level.txt borgmatic-1.9.0.dev0/pyproject.toml borgmatic-1.9.0.dev0/sample/ borgmatic-1.9.0.dev0/sample/systemd/ borgmatic-1.9.0.dev0/sample/systemd/borgmatic-user.service borgmatic-1.9.0.dev0/sample/systemd/borgmatic-user.timer borgmatic-1.9.0.dev0/sample/systemd/borgmatic.service borgmatic-1.9.0.dev0/sample/systemd/borgmatic.timer borgmatic-1.9.0.dev0/setup.cfg ``` Notice how most of the source subdirectories are missing: `borgmatic/actions/`, `borgmatic/borg/`, etc.

Package discovery was broken indeed and somehow my test failed to raise the issue (some leftovers interfered I guess). Anyway I fixed this in 601e393ec7.

I'm hoping to have more time next week to work on your comments,
Thanks for the review.

Package discovery was broken indeed and somehow my test failed to raise the issue (some leftovers interfered I guess). Anyway I fixed this in 601e393ec7. I'm hoping to have more time next week to work on your comments, Thanks for the review.

Cool, yeah, the tarball contents look much better now!

Cool, yeah, the tarball contents look much better now!
python3 -m build
witten marked this conversation as resolved
Review

When I try this inside a Python 3 virtualenv, the command doesn't appear to work:

$ python3 -m build
/home/witten/Downloads/borgmatic/.tox/py312/bin/python3: No module named build

Looks like it might need to be added to test_requirements.txt? And/or to [build-system] in pyproject.toml?

When I try this inside a Python 3 virtualenv, the command doesn't appear to work: ``` $ python3 -m build /home/witten/Downloads/borgmatic/.tox/py312/bin/python3: No module named build ``` Looks like [it might need to be added]( https://build.pypa.io/en/stable/installation.html) to `test_requirements.txt`? And/or to `[build-system]` in `pyproject.toml`?
Review

I believe there are two different issues here. One regarding tox and another one concerning the release script.

My understanding of recent python packaging is that build module is only a build frontend supporting pyproject.toml to call the build-system (cf. doc here).

I believe we don't' need to tell tox to use it, and apparently tox can build the package without it, probably using plain pip (an alternative build frontend ).

Then there is no need to add build module to test_requirements.txt IMHO.

But I fixed scripts/release to use it instead setup.py.
Then I guess we should add build dependency somewhere in the release process.
Could be test_requirements.txt if you run the script within a tox venv, but I believe it's better to release from a pristine environment to avoid side effects of pre-populated venv with tons of deps for test/coverage/lint/etc….

But this is something I let you decide, I don't know how you release borgmatic.

I believe there are two different issues here. One regarding **tox** and another one concerning the **release script**. My understanding of recent python packaging is that ``build`` module is only a build frontend supporting ``pyproject.toml`` to call the ``build-system`` (cf. [doc here](https://packaging.python.org/en/latest/tutorials/packaging-projects/#choosing-a-build-backend)). I believe we don't' need to tell tox to use it, and apparently tox can build the package without it, probably using plain ``pip`` (an alternative build frontend ). Then there is no need to add ``build`` module to ``test_requirements.txt`` IMHO. But I fixed ``scripts/release`` to use it instead ``setup.py``. Then I guess we should add ``build`` dependency somewhere in the release process. Could be ``test_requirements.txt`` if you run the script within a tox venv, but I believe it's better to release from a pristine environment to avoid side effects of pre-populated venv with tons of deps for test/coverage/lint/etc…. But this is something I let you decide, I don't know how you release borgmatic.
Review

Thanks for the explanation. I typically release borgmatic manually by running scripts/release on my machine (not from a virtualenv), although I'm open to changing the process if it would help. But if the release script is the only place that requires a build dependency, I'm fine just manually installing it on my machine for now. And then if/when the release script moves to, say, CI, that automated process can install build accordingly.

Thanks for the explanation. I typically release borgmatic manually by running `scripts/release` on my machine (not from a virtualenv), although I'm open to changing the process if it would help. But if the release script is the only place that requires a `build` dependency, I'm fine just manually installing it on my machine for now. And then if/when the release script moves to, say, CI, that automated process can install `build` accordingly.
twine upload -r pypi --username __token__ dist/borgmatic-*.tar.gz
twine upload -r pypi --username __token__ dist/borgmatic-*-py3-none-any.whl

View File

@ -1,26 +0,0 @@
[metadata]
description_file=README.md
[tool:pytest]
testpaths = tests
addopts = --cov-report term-missing:skip-covered --cov=borgmatic --ignore=tests/end-to-end
[flake8]
max-line-length = 100
extend-ignore = E203,E501,W503
exclude = *.*/*
multiline-quotes = '''
docstring-quotes = '''
[tool:isort]
profile=black
known_first_party = borgmatic
line_length = 100
skip = .tox
[codespell]
skip = .git,.tox,build
[pycodestyle]
ignore = E203
max_line_length = 100

View File

@ -1,42 +0,0 @@
from setuptools import find_packages, setup
VERSION = '1.9.0.dev0'
setup(
name='borgmatic',
version=VERSION,
description='Simple, configuration-driven backup software for servers and workstations',
author='Dan Helfman',
author_email='witten@torsion.org',
url='https://torsion.org/borgmatic',
classifiers=[
'Development Status :: 5 - Production/Stable',
'Environment :: Console',
'Intended Audience :: System Administrators',
'License :: OSI Approved :: GNU General Public License v3 (GPLv3)',
'Programming Language :: Python',
'Topic :: Security :: Cryptography',
'Topic :: System :: Archiving :: Backup',
],
packages=find_packages(exclude=['tests*']),
witten marked this conversation as resolved Outdated

I don't know if something like this is necessary in pyproject.toml so tests don't get installed into the final package? Or is that done automatically? I didn't see tests end up in the resulting tarball, so maybe it's fine as is.

I don't know if something like this is necessary in `pyproject.toml` so tests don't get installed into the final package? Or is that done automatically? I didn't see tests end up in the resulting tarball, so maybe it's fine as is.

It's done automatically indeed.

And the resulting tar ball does not contain any test files.

But to make sure to package all modules in borgmatic package I used tool.setuptools.packages.find then I believe anything outside ./borgmatic folder in not covered anyway.

Please not that tool.setuptools.packages.find also set namespaces = false, from the doc “This will prevent any folder without an init.py file from being scanned.“ src.

It's [done automatically indeed](https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#flat-layout). And the resulting tar ball does not contain any test files. But to make sure to package all modules in borgmatic package I used `tool.setuptools.packages.find` then I believe anything outside `./borgmatic` folder in not covered anyway. Please not that `tool.setuptools.packages.find` also set `namespaces = false`, from the doc “This will prevent any folder without an __init__.py file from being scanned.“ [src](https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#finding-simple-packages).

That makes sense. The only thing potentially missing that I could see of being use is the sample/ directory, as I'm vaguely recalling that some Linux distribution packages of borgmatic programmatically consume those files.

That makes sense. The only thing potentially missing that I could see of being use is the `sample/` directory, as I'm vaguely recalling that some Linux distribution packages of borgmatic [programmatically consume those files](https://gitlab.archlinux.org/archlinux/packaging/packages/borgmatic/-/blob/main/PKGBUILD?ref_type=heads#L46).
Review

We discussed it already :)

These files are in the source distribution. The MANIFEST.in is declaring it and setuptools includes them by default, no need to explicitly declare

We used systemd unit file in Debian as well, but we got the source from the tagged version in github, then we get the whole source tree. I'm not familiar with PKGBUILD but I think they use the git source as well, not the pypi source distribution. Anyway thanks for shipping these, it's helpful for packaging.

We discussed it already :) [These files are in the source distribution. The MANIFEST.in is declaring it and setuptools includes them by default, no need to explicitly declare ](https://projects.torsion.org/borgmatic-collective/borgmatic/pulls/922/files#issuecomment-8753) We used systemd unit file in Debian as well, but we got the source from the tagged version in github, then we get the whole source tree. I'm not familiar with PKGBUILD but I think they use the git source as well, not the pypi source distribution. Anyway thanks for shipping these, it's helpful for packaging.
Review

Okay, just making sure since you quoted: "This will prevent any folder without an __init.py__ file from being scanned." But if the MANIFEST.in still takes care of sample/, great.

Getting the tagged version from GitHub should be fine, although note that projects.torsion.org is the canonical source for the source and GitHub is "only" a mirror.

Okay, just making sure since you quoted: "This will prevent any folder without an \_\_init.py\_\_ file from being scanned." But if the MANIFEST.in still takes care of `sample/`, great. Getting the tagged version from GitHub should be fine, although note that projects.torsion.org is the canonical source for the source and GitHub is "only" a mirror.
entry_points={
'console_scripts': [
'borgmatic = borgmatic.commands.borgmatic:main',
'generate-borgmatic-config = borgmatic.commands.generate_config:main',
'validate-borgmatic-config = borgmatic.commands.validate_config:main',
]
},
obsoletes=['atticmatic'],
witten marked this conversation as resolved Outdated

This didn't get carried forward to the pyproject.toml, but that's probably okay given how much it's ancient history.

This didn't get carried forward to the `pyproject.toml`, but that's probably okay given how much it's ancient history.

Right, it can still be ported to pyproject.toml with an extra conf :

[tool.setuptools]
obsoletes = ['atticmatic']
Right, it can still be ported to `pyproject.toml` with an extra conf : ```ini [tool.setuptools] obsoletes = ['atticmatic'] ```

I really don't feel strongly either way. Just thought I'd mention it.

I really don't feel strongly either way. Just thought I'd mention it.
install_requires=(
'colorama>=0.4.1,<0.5',
'jsonschema',
'packaging',
'requests',
'ruamel.yaml>0.15.0',
'setuptools',
),
extras_require={"Apprise": ["apprise"]},
include_package_data=True,
witten marked this conversation as resolved Outdated

Looks like the pyproject.toml equivalent of this line is missing? I think the main reason this exists is so that the configuration schema file gets installed. (It's at borgmatic/config/schema.yaml in the source.) But also the systemd and other sample files (sample/) probably get included too.

Looks like the `pyproject.toml` equivalent of this line is missing? I think the main reason this exists is so that the configuration schema file gets installed. (It's at `borgmatic/config/schema.yaml` in the source.) But also the systemd and other sample files (`sample/`) probably get included too.

These files (systemd units and schema) are declared in MANIFEST.in and by default, include-package-data is true in pyproject.toml.

I can see these files in the resulting source dist tar ball.

These files (systemd units and schema) are declared in ``MANIFEST.in`` and by default, [include-package-data is true in pyproject.toml](https://setuptools.pypa.io/en/latest/userguide/datafiles.html#configuration-options). I can see these files in the resulting source dist tar ball.

Awesome, thanks!

Awesome, thanks!
python_requires='>=3.8',
)

View File

@ -45,3 +45,10 @@ commands =
deps = {[testenv]deps}
commands =
codespell --write-changes
[flake8]
max-line-length = 100
extend-ignore = E203,E501,W503
exclude = *.*/*
multiline-quotes = '''
docstring-quotes = '''