-
-
Notifications
You must be signed in to change notification settings - Fork 523
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.
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
start using setuptools_scm #483
start using setuptools_scm #483
Conversation
setuptools_scm removes the need to maintain versions numbers in the source code. This commit removes all code that was needed due to manually handling version numbers. * remove _getdoctarget.py which was a helper to fetch the version for sphinx * remove SITETARGET using that script as it is very likely obsolete asI can't find any usage or references to this * fetch version from pkg_resources instead
o.k. this needs some improvements still - I guess. Will get back to that later then. |
@obestwalter seems like homegrown version parsing is the cause, it should suffice to use the packaging strict version lib there |
@RonnyPfannschmidt thanks, will do that. |
…an digest I feel uncomfortable to introduce a dependency on packaging just do some string splitting. It is always available inside virtualenvs, but not outside.
About the aditional code: At the moment the vendored in _verlib.py is used and a version number like I just had my first journey into packaging (the module - not the activity) and it seems to be an implicit dependency in all virtualenvs but is not part of the distribution, so I used the same method that packaging uses to get the public version for the tox version which renders the exact same format that was in use for dev versions before. |
btw, please rebase instead of merging from master |
That merge from master was done by the new (at least saw it for the first time today) web conflict resolving "tool". I wanted to try it out. It can only merge. Will not use it anymore then. |
…into 474-start-using-setuptools-scm
o.k. I definitely have to improve my git foo. This is quite different from the workflow I am used to and the results are pretty ugly. But this will all go away when I squash the commits on merge anyway :) |
oh, i see, that one is quite a mess ^^ |
tox/__init__.py
Outdated
|
||
from .hookspecs import hookspec, hookimpl # noqa | ||
|
||
_version = get_distribution('tox').version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should use the "file generation feature" instead (example here)? I really like that solution due to its simplicity and avoiding having a runtime dependency with pkg_resources
, which is widely known to be really slow to import, see pypa/setuptools#926. It also poses problems to frozen applications (testing-cabal/mock#385), although I don't think people freeze tox
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, thanks @nicoddemus did not see that option yet. I will update the PR accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tox uses entrypoints for plugins, so its no loss
I wonder if using the write_to approach could make the lifes of packagers harder. @RonnyPfannschmidt and @warsaw - might I ask your opinion first, before I introduce any changes there? What is the best way to do this? |
I don't see how, but let's hear other's opinion first. |
Better safe than sorry. I don't want to introduce unnecessary pain down the line :) |
@obestwalter Sorry, but can you explain what the "write_to approach" is? |
@warsaw with write_to setuptools_scm writes the version string to a file that can be obtained by import or reading, thus is way more inexpensive than asking pkg_ressources for small tools |
my personal suggestion is, if you use entrypoints in a major code-path, just use pkg_resources |
@warsaw the reason why I included you is because I read this here and I smell trouble for you, when you are packaging tox:
http://www.danielstender.com/blog/setuptools-scm/ If you haven't dealt with this yet, this might be a surprise then. |
@RonnyPfannschmidt I don't quite get what you mean with this. Do you suggest we leave it like it is for tox then (no I could also imagine to still keep the version number in a file that actually ends up in the distribution to prevent packaging surprises. My main aim here is to have one and exactly one place where the version number needs to be bumped manually and this ideally is the git tag. |
Yes, it will be painful for Debian if the version string is not included in the tarball. That blog post describes some workarounds which sound ugly, but could be doable. I haven't yet tried the This is a fairly common problem that I have to deal with in my own packages and applications. I know the temptation to use git tags is very compelling, but tbh I have my own low tech approach. It still means I have to bump a text file version number on release, but I only have to do it in one place, so it's not that bad. I'd certainly prefer not to have to implement workarounds in the Debian packaging, so including the version in the tarball would be best. But if you have a branch that generates a tarball representative of what you want to release, I can do a test update and see what happens. |
@warsaw i do wonder if debian could start to adpat a scheme where there is just a fork of the main repo with debian version tags following the normal development |
@RonnyPfannschmidt There are definitely people who want to build new packages off of upstream git repos, but for now PyPI tarballs are the only acceptable workflow for Debian packaging within the Python packaging team. So much of our tooling is based on that, and it'll be fairly disruptive to change. I could move tox out of the team, but I really don't want to do that (team maintenance is a good thing). There are plenty of packages that don't use git for example. (I've only encountered one that doesn't release on PyPI, but only on GitHub, and that does break some tooling.) If more packages don't release on PyPI, it'll be something that we'll have to look at, but I still hope that doesn't happen. ;) |
@warsaw i see, true - i jsut saw something like that happening for example for borgbackup/borg a while back and was thinking that it perhaps was a new way forward |
The version file generated by
Ahh that's a good point. I say let's keep this PR as it is then, we are already paying the price of importing |
On Mar 13, 2017, at 08:48 AM, Bruno Oliveira wrote:
The version file generated by `write_to` *should* be included in the
generated tarball as it is supposed to be part of the distribution, that's
why I couldn't imagine how that would be a problem for other packagers. 😄
If that's the case, then I can't either! :)
|
oh dear ... good that we talked about this. Back to square one then 😁 As you said @nicoddemus we import @warsaw - if you like I can create a tarball for you to check if the new versioning hotness breaks your workflow, although I am pretty sure that it won't cause problems in the way we have it now. One fine day someone can maybe explain this "entrypoints in a major code-path"-thing, if not here I will bug you at the next sprint if the penny hasn't dropped until then. |
@obestwalter what i meant is that tox already uses pkg_resources to iterate its installed plugins using entry-points, so its always being used and not an extra cost |
I just upoaded a 2.6.1 version built from this branch, if anyone wanst to check with their packaging workflow. |
@nicoddemus I am just using your cloud testing hack to test that package ... sweeeet :D https://travis-ci.org/obestwalter/devpi-cloud-tester/builds/210661806 |
Hehehehe 😄 |
@obestwalter I did a test build with your tar.gz and after adjusting the Debian packaging for the upstream changes since 2.5.0, everything seemed to pass. The package built and passed its tests. I haven't actually tried to use it, but since that's not the focus of the setuptools-scm change, I think you're good to go with this PR. |
Great! Thank you @warsaw. |
Are there any objections to merge this directly after the release of 2.7.0 then? |
fixes #474
setuptools_scm removes the need to maintain versions numbers in the source code. This PR adds the setuptools_scm functioniality and removes all code that was needed due to manually handling version numbers.