pyproject-transition #922
Reference in New Issue
Block a user
Delete Branch "kaliko/borgmatic:pyproject-transition"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Moving to pyproject.toml
806d544fcato9e29ce788fI'm not sure how PyCQA is handled within borgmatic project. The doc suggest tox is the main entrypoint.
Then I think pycqa config could be gathered within
tox.iniconf file whenpyproject.tomlis not supported.Here are the last 2 tools I did not merge in
pyproject.toml:flake8
No support for pyproject.toml, but there are currently two locations with flake8 directives within the project:
setup.cfg.flake8Why not merge the conf in
.flake8ortox.ini?pycodestyle
No support for pyproject.toml. Keep config in
setup.cfgor move it totox.ini?c7cefd5922tob029d1cb67Thank you so much for taking the time to do all this! I've played around with this a little and left my thoughts/questions in the comments.
@@ -4,0 +58,4 @@skip = ".tox"[tool.codespell]skip = ".git,.tox,build"This
pyproject.tomllooks good to me. But it seems to result in an unimportableborgmaticmodule, I'm guessing becausesetup.pyis gone and something is still relying on it to considerborgmatica module.For instance, when I run tests, a particular test that tries to run the
borgmaticexecutable fails:And I can reproduce this by manually running borgmatic in the virtualenv:
So I'm not sure what's missing here.
Fixed with
601e393ec7(pyproject/build failed to ship submodules).@@ -34,4 +34,2 @@# Build borgmatic and publish to pypi.rm -fr distpython3 setup.py bdist_wheelpython3 setup.py sdistSomething is missing from the result tarball. Here's what I'm seeing on my machine:
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.
Cool, yeah, the tarball contents look much better now!
@@ -35,3 +35,2 @@rm -fr distpython3 setup.py bdist_wheelpython3 setup.py sdistpython3 -m buildWhen I try this inside a Python 3 virtualenv, the command doesn't appear to work:
Looks like it might need to be added to
test_requirements.txt? And/or to[build-system]inpyproject.toml?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
buildmodule is only a build frontend supportingpyproject.tomlto call thebuild-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
buildmodule totest_requirements.txtIMHO.But I fixed
scripts/releaseto use it insteadsetup.py.Then I guess we should add
builddependency somewhere in the release process.Could be
test_requirements.txtif 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.
Thanks for the explanation. I typically release borgmatic manually by running
scripts/releaseon 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 abuilddependency, 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 installbuildaccordingly.@@ -37,4 +0,0 @@'setuptools',),extras_require={"Apprise": ["apprise"]},include_package_data=True,Looks like the
pyproject.tomlequivalent of this line is missing? I think the main reason this exists is so that the configuration schema file gets installed. (It's atborgmatic/config/schema.yamlin 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.inand by default, include-package-data is true in pyproject.toml.I can see these files in the resulting source dist tar ball.
Awesome, thanks!
@@ -27,4 +0,0 @@'validate-borgmatic-config = borgmatic.commands.validate_config:main',]},obsoletes=['atticmatic'],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.tomlwith an extra conf :I really don't feel strongly either way. Just thought I'd mention it.
@@ -19,4 +0,0 @@'Topic :: Security :: Cryptography','Topic :: System :: Archiving :: Backup',],packages=find_packages(exclude=['tests*']),I don't know if something like this is necessary in
pyproject.tomlso 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.findthen I believe anything outside./borgmaticfolder in not covered anyway.Please not that
tool.setuptools.packages.findalso setnamespaces = false, from the doc “This will prevent any folder without an init.py file from being scanned.“ src.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.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.
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.
That's correct.
I'm not familar with PyCQA. What is its config? It looks like Tox does support
pyproject.toml(via legacy INI format or even native TOML).. or its config can be kept in the legacytox.ini.Either of those approaches sound fine to me.
Also either approach is fine with me. Note however the comment above about potentially moving Tox config to
pyproject.toml, which is maybe a backdoor way to get both flake8 and pycodestyle config intopyproject.toml!d5afdeebdato601e393ec7Sorry I was just referring to PyCQA as the collection of tools used in borgmatic for QA (tox, black, etc…).
Anyway, I did see the
legacy_tox_iniintegration of ini tox config inpyproject.tomlbut it looks ugly (I know it's highly subjective :D).Personally I prefer to merge everything in
tox.ini, but the decision is yours as the main developer.Are you taking over from there? Or do you want me to make some more changes?
Cheers,
k
You're the one who instigated this work item, so while I certainly have opinions, if you wanna leave stuff in
tox.ini... that's totally fine with me! I will mention though that there's also a "native" TOML format for stuffingtox.inicontents intopyproject.toml. Maybe you were referring to that as ugly as well though. 😄Anyway, to answer your question: If you're feeling up to continuing this, it would be great if you could!
WIP: pyproject-transitionto pyproject-transitionI rebased the branch on latest changes from main to control and it still looks fine (I did not pushed rebased code though).
These change are good to merge now.
Looks great! Thank you so much for taking on this work.. It really needed doing.
Released in borgmatic 1.9.0!
FYI: #932.