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

Broken with Pipenv due to inexact version pin on black #21

Closed
bbenne10 opened this issue May 18, 2020 · 8 comments
Closed

Broken with Pipenv due to inexact version pin on black #21

bbenne10 opened this issue May 18, 2020 · 8 comments

Comments

@bbenne10
Copy link

bbenne10 commented May 18, 2020

3bfa2df was introduced to specify a minimum version of black upon which flake8-black depends. Unfortunately, this commit also breaks usage with pipenv, as we can no longer specify an exact version pin in our Pipfile.

It seems that pipenv simply takes the union of all version strings specified by the Pipfile and its dependency chain and applies logic to that union. Therefore, if I specify the version to be EXACTLY black==19.10b0, I end up with a union which is black==19.10b0,>19.3b0. Logically, this resolves to 19.10b0, but this is not what pipenv sees. Because black is still pre-release, pipenv must have a single version pin (or multiple of the same statement) that resolves to a single version. black==19.10b0 would work, black==19.10b0,black==19.10b0 would work, but (as mentioned) black==19.10b0,>=19.3b0 does not. I do not know but also believe that black==19.10b0,>=19.10b0 would fail.

I don't necessarily expect action in regards to code within this repository, but a simple mention of the incompatibility in the README (despite this problem being 100% NOT flake8-black's fault) might've saved me 30 or so minutes this afternoon.

Feel free to close it or address this issue as you see fit. The existence of this ticket will at least serve to document the incompatibility in the future (though more aggressive action - either changing the pin or updating the readme) would help more.

Either way - thank you for the time reading over this :)

@peterjc
Copy link
Owner

peterjc commented May 18, 2020

Looks like a duplicate of #18, sadly pypa/pipenv#3928 strikes again. Commenting there might help in terms of signalling to the pipenv team that issue does need some attention.

Are you sure that putting black==19.10b0 would actually work? Reading over the older issue that was not my impression - but I never actually tried a release with an exact pin (this seems very likely to cause more problems than it solves).

I fear you will have to wait for either a pipenv fix, or a non-beta black release :(

Other than that, the only viable option I can see is that I drop the explicit version requirement in my dependencies, and instead give a run time failure if the copy of black installed is too old. Not ideal either, but perhaps the lesser evil? After all, black 19.3b0 has been out over a year and the audience are extremely likely to be using that or black 19.10b0 by now.

@bbenne10
Copy link
Author

I apologize for re-visiting the issue then. I missed the original reporting!

Pre-release installs work two ways in pipenv:

  • global (with allow_prerelease=true in Pipfile): This allows pre-releases for all packages. This is a bad idea, generally.
  • per-package, but requiring a single version pin: This means that any ambiguity will cause the behavior that I described above.

I do not have a way to test presently, but I do suspect that an exact version pin would work as expected. There's still only one version to install (in this case 19.10b0) and pipenv should deal with that properly. ("should" being operative here)

As mentioned, this isn't your fault and I don't expect code-level changes here. I posted this expecting that, at best, I'd convince you to simply update the readme to include an incompatibility notice.

However, seeing as how 19.10b0 has been out since Oct, it may be that the release schedule is slow enough that you can keep up in flake8-black with a matching release whenever they drop. If that's okay with you, I am happy to do the legwork and open a PR revising the pinned version and bumping the version here (probably 0.2?).

In light of PEP517, I would love to drop pipenv for a better tool. It's dependency resolution is flaky at best and I very much dislike having to specify my dependencies in both setup.py and my Pipfile. Unfortunately, it's just not in the cards right now.

@peterjc
Copy link
Owner

peterjc commented May 18, 2020

I understand - closed issues do almost vanish on GitHub, you have to actively look for them.

Doing a version bump and dropping the minimum version requirement is the easy bit. Testing the behaviour with an older black is more effort, but since https://gitlab.com/pycqa/flake8/-/issues/559 remains open there is no elegant way to signal this back to flake8 gracefully anyway.

@bbenne10
Copy link
Author

Were I in your shoes, the route I'd go would be an exact version pin on black at the latest available version and a corresponding documentation update.

Alternatively, a simple block in the readme might help to explain the problem and the incompatibility.

However, I'm not in your shoes and they're probably not my size anyway 😛 Feel free to resolve this however you would like. I'm available for testing, if that's a thing you need.

peterjc added a commit that referenced this issue May 18, 2020
Should close issues #18 / #21 with the least
bad workaround. I consider it unlikely that
many people using black and flake8-black would
be using anything older than black 19.3b0 which
is now over a year old.
peterjc added a commit that referenced this issue May 18, 2020
Should close issues #18 / #21 with the least
bad workaround. I consider it unlikely that
many people using black and flake8-black would
be using anything older than black 19.3b0 which
is now over a year old.
@peterjc
Copy link
Owner

peterjc commented May 18, 2020

Pull request coming up for you to comment on - Did you have strong reason for suggesting bumping the version of v0.2 rather than v0.1.2?

@peterjc
Copy link
Owner

peterjc commented May 18, 2020

Ah, looks like flake8 v3.8 broke a unit test - I'll have to deal with that first, likely due to changes in path normalisation which might resolve #13.

peterjc added a commit that referenced this issue May 18, 2020
Should close issues #18 / #21 with the least
bad workaround. I consider it unlikely that
many people using black and flake8-black would
be using anything older than black 19.3b0 which
is now over a year old.
peterjc added a commit that referenced this issue May 20, 2020
Should close issues #18 / #21 with the least
bad workaround. I consider it unlikely that
many people using black and flake8-black would
be using anything older than black 19.3b0 which
is now over a year old.
peterjc added a commit that referenced this issue May 20, 2020
Should close issues #18 / #21 with the least
bad workaround. I consider it unlikely that
many people using black and flake8-black would
be using anything older than black 19.3b0 which
is now over a year old.
peterjc added a commit that referenced this issue May 20, 2020
Should close issues #18 / #21 with the least
bad workaround. I consider it unlikely that
many people using black and flake8-black would
be using anything older than black 19.3b0 which
is now over a year old.
@peterjc
Copy link
Owner

peterjc commented May 20, 2020

Fixed via #22 (a good compromise now that the minimum version of black we need is over a year old).

@bbenne10 please reopen or file a new issue if this doesn't solve your pipenv problem. Thanks!

@peterjc peterjc closed this as completed May 20, 2020
@bbenne10
Copy link
Author

Thank you for such a quick turnaround! I appreciate the responsiveness :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants