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.

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

second take at setuptools_scm #1834

Merged

Conversation

RonnyPfannschmidt
Copy link
Member

since setuptools 18.6 fixes the issues with develop installs

https://github.com/pypa/setuptools/blob/master/CHANGES.rst#186

pypa/setuptools#439

@RonnyPfannschmidt RonnyPfannschmidt added the type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature label Aug 21, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 93.0% when pulling de0f0cf on RonnyPfannschmidt:setuptools-scm-take-2 into abe8f5e on pytest-dev:master.

HOWTORELEASE.rst Outdated
@@ -3,11 +3,9 @@ How to release pytest

Note: this assumes you have already registered on pypi.

1. Bump version numbers in ``_pytest/__init__.py`` (``setup.py`` reads it).
1. Check and finalize ``CHANGELOG.rst``.
Copy link
Member

Choose a reason for hiding this comment

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

We probably want to mention to generate the tag locally right before creating the package, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

im considering a setuptools_scm feature where the branch name would be considered as well

@nicoddemus
Copy link
Member

Besides my comment on HOWTORELEASE, linting has fail due to the fact that the numbers in the sections don't increase sequentially anymore.

@nicoddemus
Copy link
Member

My previous comment is still valid, I think:

We probably want to mention to generate the tag locally right before creating the package, correct?

😁

@RonnyPfannschmidt
Copy link
Member Author

actually i have a pending setuptools_scm update for solving those details more nice - will show later today

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 93.008% when pulling d6e1021 on RonnyPfannschmidt:setuptools-scm-take-2 into e75078e on pytest-dev:master.

@RonnyPfannschmidt RonnyPfannschmidt changed the title second take at setuptools_scm [wip] second take at setuptools_scm Aug 29, 2016
@nicoddemus
Copy link
Member

Did you give up on this, or will try on another branch?

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus i'll pick this up when either setuptools_scm supports branches or we decide to get rid of the features branch

@nicoddemus
Copy link
Member

nicoddemus commented Apr 19, 2017

I might be missing something, but I don't really see why. Doesn't setuptools_scm determine the version based on the nearest tag? So if I tag a commit 2.1.1 the version will be 2.1.1, and if I decide to create a new feature release I can tag a commit 2.2.0 and it will just work, right?

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus yes, but its a bit of a pain when a feature branch for example has a "wrong" version attached for quite a while

@nicoddemus
Copy link
Member

Can you elaborate a bit what do you mean by "its a bit of a pain when a feature branch for example has a "wrong" version attached for quite a while"?

I insist because I would love to use setuptools_scm on pytest, and would like to understand the reason completely if we have to ditch it because of having a features branch.

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus setuptools_scm currently works based only on tags, so its not aware on how branches should bump the last version, and while it is supported to put certain tags in to help, its not exactly well received to litter the history with extra tags to keep setuptools_scm happy

@nicoddemus
Copy link
Member

Sorry, I still don't quite get what the issue is, bear with me... from my POV the version is important basically only when making a release, and at that point we will have a tag at the exact location we need it to be. Or am I I missing something?

@RonnyPfannschmidt
Copy link
Member Author

if we are fine with such a limitation, we can just switch over right now

@nicoddemus
Copy link
Member

I'm 👍 on it, unless I'm missing a situation where the development version is important.

@RonnyPfannschmidt RonnyPfannschmidt changed the base branch from master to features April 19, 2017 14:07
@RonnyPfannschmidt
Copy link
Member Author

will rebase after pass

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 92.614% when pulling 6a691b3 on RonnyPfannschmidt:setuptools-scm-take-2 into 80cabca on pytest-dev:features.

@nicoddemus
Copy link
Member

👍

Linting broke because of some bad interaction between setuptools-scm and check-manifest...

@RonnyPfannschmidt RonnyPfannschmidt changed the title [wip] second take at setuptools_scm second take at setuptools_scm Apr 19, 2017
@nicoddemus
Copy link
Member

can we include egg-info into frozen pytest

Unfortunately no. It think all freeze tools don't support including the packaging metadata (see marcelotduarte/cx_Freeze#216) and I suspect even after including package metadata pkg_resources would still need to learn how to deal with that metadata embedded in zipfiles.

note that using unknown as version would break expectations as well

I understand, but None breaks pytest completely... "unknown" as version would break some expectations, but I think it preferable than the former.

approach breaks check-manifest

I would prefer if we found a solution to that, given that check-manifest is a tool we use internally, as opposed to freezing which is something normal users use.

Just to add some context, we freeze pytest extensively together with our applications at work, so we can run the tests using the frozen executable. This is invaluable to catch packaging errors.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 92.614% when pulling 4242bf6 on RonnyPfannschmidt:setuptools-scm-take-2 into 80cabca on pytest-dev:features.

@@ -47,6 +47,9 @@ commands=

[testenv:linting]
basepython = python2.7
# needed to keep check-manifest working
setenv =
SETUPTOOLS_SCM_PRETEND_VERSION=2.0.1
Copy link
Member

Choose a reason for hiding this comment

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

So it seems this fixes the check-manifest issue. 😁

Can we switch to the write_to option now?

@RonnyPfannschmidt
Copy link
Member Author

Nppe, since write_to breaks check manifest

@nicoddemus
Copy link
Member

Nppe, since write_to breaks check manifest

I'm a little confused then, why do you need to set SETUPTOOLS_SCM_PRETEND_VERSION during linting?

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus pretend_version is for the breakage that is incurred from not having version data in whatever check-manifest copies around, the other breakage is incurred because check-manifest does nit allow for generated files in a sdist that aren't in the scm

@nicoddemus
Copy link
Member

nicoddemus commented Apr 22, 2017

the other breakage is incurred because check-manifest does nit allow for generated files in a sdist that aren't in the scm

Perhaps we can use the ignore option to ignore the generated version file?

[check-manifest]
ignore =
    .travis.yml

https://pypi.python.org/pypi/check-manifest/

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus thanks for that hint

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus unknown breaks hypothesis - im going to go back to None

@nicoddemus
Copy link
Member

Hmm wait, but this was working before with hypthosis right? I mean, hypothesis should be seeing the correct version obtained from _version.py.

But if we do find a reasonable explanation for the above, how about using 0.0.0 then? That's a valid version string but it is clear something is incorrect with the installation. None is not better because pytest itself breaks if version is None.

@RonnyPfannschmidt
Copy link
Member Author

pytest i can directly fix, hypothesis i cant

@nicoddemus
Copy link
Member

Why you don't like my suggestion of "0.0.0"? It is as good as "unknown" or None I think.

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus unknown doesnt break on bad version checks due to lexical order, 0.0.0 does in any case
i'd rather have a correct failure than trying to keep stuff working by means of pretense

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 92.505% when pulling a280e43 on RonnyPfannschmidt:setuptools-scm-take-2 into 80cabca on pytest-dev:features.

@nicoddemus
Copy link
Member

i'd rather have a correct failure than trying to keep stuff working by means of pretense

Fair enough. 👍

@RonnyPfannschmidt
Copy link
Member Author

RonnyPfannschmidt commented Apr 26, 2017

@nicoddemus as far as i can tell introducing setuptools_scm will warrant a major release due to the cahnge to pytest.__version__ behaviour

@The-Compiler
Copy link
Member

What changes about it?

@RonnyPfannschmidt
Copy link
Member Author

it could be none, and it an include local version components,
as such it will break naive version parsers

@nicoddemus
Copy link
Member

it could be none, and it an include local version components,
as such it will break naive version parsers

But that doesn't affect releases, which will always have a __version__ in the same format as always. It doesn't make sense to me to increase the major version because of a change that doesn't affect the releases at all. In other words, from a user's POV a release 3.1.0 with or without this change is indistinguishable from each other.

Do you agree?

@nicoddemus
Copy link
Member

Merging as this now looks great, thanks @RonnyPfannschmidt!

@The-Compiler
Copy link
Member

This breaks hypothesis, and maybe other plugins doing pytest version checking: HypothesisWorks/hypothesis#611

I still maintain my opinion that the benefits of saving some 10s of work for bumping the version number (which can trivially be automated, too) is by no means worth the breakage this causes everywhere...

@nicoddemus
Copy link
Member

is by no means worth the breakage this causes everywhere...

Definitely I agree, but that was not supposed to happen. Can you provide more details on how it was possible to install pytest and somehow setuptools_scm failing to generate a _version.py file? Puzzling...

@The-Compiler
Copy link
Member

The-Compiler commented May 10, 2017

Ah, doing another pip install -e run (after pulling the setuptools-scm change) fixes it - sorry for not trying this before. This was a develop install done before setuptools-scm was introduced.

I wanted to run a pytest --help to check something before going to bed and then got this 😆

Now I'm curious though - what's this, and why 1.6.0?

edit: Oh. (pytest-mock) version, not pytest (mock-version). 😆

$ cat .venv/lib/python3.6/site-packages/_pytest_mock_version.py
# coding: utf-8
# file generated by setuptools_scm
# don't change, don't track in version control
version = '1.6.0'

@nicoddemus
Copy link
Member

Ah, doing another pip install -e run (after pulling the setuptools-scm change) fixes it - sorry for not trying this before. This was a develop install done before setuptools-scm was introduced.

I figured, no worries. This will only happen if one has an old installation and didn't execute a pip install -e. Released packages will always have a _version.py file so this should not be an issue. 😉

@RonnyPfannschmidt RonnyPfannschmidt deleted the setuptools-scm-take-2 branch June 11, 2017 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants