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

WheelFile moved location between 0.31.1 and 0.32.0, breaking user code #255

Closed
matthew-brett opened this issue Sep 29, 2018 · 23 comments
Closed

Comments

@matthew-brett
Copy link

I have, for a long time, been using WheelFile in:

Both of these are fairly widely used. At some point between 0.31.2 and 0.32.0, WheelFile appears to have moved from the install module to the wheelfile module, thus breaking imports for both bits of code. Of course, I can work round this with some conditional imports, and new releases of these packages, but it would help if you would consider deprecation warnings or similar before moving stuff around in the public API.

See: https://github.com/matthew-brett/multibuild/issues/202

matthew-brett added a commit to multi-build/multibuild that referenced this issue Sep 29, 2018
WheelFile moved for this release.

See: pypa/wheel#255
@benoit-pierre
Copy link
Member

AFAIK, there's no supported public API, as noted in the README:

It should be noted that wheel is not intended to be used as a library, and as such there is no stable, public API.

@matthew-brett
Copy link
Author

I thought you'd say that :)

Pip took the same approach - it certainly reduces maintenance burden, for wheel, but the consequence is that people have to start copying code out of wheel into their own packages, meaning an increasing mess of out of date and incompatible code copies. The same situation is already starting to cause pain for pip.

@agronholm
Copy link
Contributor

Why is this a problem for wheel?

@hugovk
Copy link
Contributor

hugovk commented Oct 1, 2018

This also affects auditwheel:

pypa/auditwheel#102

@matthew-brett
Copy link
Author

We (package maintainers generally) have to choose how to do the most good, which involves some trade-off between features and maintenance time.

In the particular case of Wheel, the maximum benefit to us packagers, would be both of:

  • a stable working CLI
  • a stable maintained API

The second is of benefit to us, the rest of the Python packaging system, because y'all have code that works, that implements the standards. You keep up to date with the changes in the standards. That's why we are using your code, when you told us not too - it was just too useful to ignore, and too substantial a maintenance burden for us to copy. Otherwise we have to repeat the work that you already have to do, which is to keep up with the standards, as they evolve.

The stable API is some maintenance burden. I think the compromise most people go for, is keeping at least some of the API somewhat stable, on the basis that is not too much work, and making the rest of it private.

You've gone to the extreme on this spectrum, in that you have what appears to be a public API (no underscores) but you don't support it at all. That will make your life simpler, as a maintainer of the CLI (I'd love not to support the API for my packages), but at a significant cost for the rest of us.

Pip recently went for making their whole API private, explicitly. This was designed to minimize maintenance, and if possible, persuade people to refactor the API parts that they wanted into separate libraries, but that didn't work, and what happened was that we, the users of Pip's API, just copied Pip's code into our own codebases. Pipenv, for example, carries a complete copy of older PIp. In the discussion on the pypa mailing list, I think it's becoming clear that some better compromise is needed.

Of course you can say "I don't care about the rest of the packaging ecosystem, we warned you not to use our API, you did, now it's broken, that's your fault". But, if your desire is the maximum benefit to the pip / wheel ecosystem, I think you will find, that some minimal effort to maintain a stable API, will help the rest of us a great deal, with small cost to you.

@agronholm
Copy link
Contributor

It is not a public API if it's not documented. There used to be a (poor) API documentation but IIRC people have been using functionality even outside that. My intention with this release was to make that abundantly clear that it was not intended to be used from outside. Changing it to use underscores would not have helped at this point. I did not write the original code so it having no underscores is not my fault.

That said, a compromise would be possible but I'd need to know which parts of the library people want me to expose as a public API. Only then can I make an informed decision as for what to commit into maintaining.

@matthew-brett
Copy link
Author

Thanks for considering.

I think most people are like me, and a) don't document their own APIs very well and therefore b) do not take documentation as evidence of the public API and therefore c) assume a Python API is public if there are no underscores. That's probably why there are so many bits of code using the Wheel API. I'm sure that's why Pip moved their entire API to an underscore prefix.

No need for finding fault, the only needful thing is to work out what to do about:

  • the immediate breakage of what looks like a lot of code out there, using the wheel API and
  • the policy for future releases.

I can tell you what I am using Wheel code for:

  • rewriting a RECORD file to record new hashes for changed or added files
  • read_pkg_info, write_pkg_info
  • WheelFile, for getting the parsed filename dictionary, and the parsed wheel info, and the wheel info filename.
  • working out the tags that the wheel corresponds to. Until this release, I was using the tags property of WheelFile for that, now I've had to fall back to copying to the old Wheel code, and using WHEEL_INFO_RE to parse the wheel file name.

I think you'll find that we packagers are fine with API changes, as long as we have some time to deal with them before public stuff breaks. We're packagers, we know that maintenance is costly, and sometimes, you have to make breaking changes.

@matthew-brett
Copy link
Author

Incidentally, the biggest headache for my code was dealing with the zero-permissions RECORD file. If you could see your way through to reverting that change, I am sure that would help.

@agronholm
Copy link
Contributor

Incidentally, the biggest headache for my code was dealing with the zero-permissions RECORD file

I have no idea what you're talking about, but if you can point me to the right direction, I'll see what I can do.

jni added a commit to scikit-image/multibuild that referenced this issue Oct 1, 2018
0.32 broke the universe. See the following for more info:

pypa/auditwheel#102
pypa/wheel#255
python-pillow/pillow-wheels#102
@matthew-brett
Copy link
Author

Huh - sorry - now I investigate, I do not see the RECORD permissions problem I was having on my laptop. I'll get back to you if I can reproduce.

@dholth
Copy link
Member

dholth commented Oct 1, 2018

Some parts of wheel are more API-like than others, with wheelfile being the most so and bdist_wheel.py being the least so, some of the recent changes are bdist_wheel making better use of wheel's own API.
IMO underscores are meaningless. Everything is public, I blame Python. Sorry for the inconvenience.
But pip is so complex and poorly defined that it is a poor reference implementation for anything.

@matthew-brett
Copy link
Author

Yes, I must admit I was drawn to WheelFile as an API.

I know Python doesn't enforce privacy, but I also know that when I'm looking at someone else's library, and I find myself using an underscore method or function, I am conscious that this is at my own risk, much more so than for a non-underscore method.

I suppose the reason that people do tend to use Pip code, despite the warnings, is that a) at least it's a standard, rather than each author making their own and b) just because it's Pip, it defines behavior that people are used to.

@safiyat
Copy link

safiyat commented Oct 2, 2018

@agronholm I am able to reproduce the zero-permissions RECORD file problem as faced by @matthew-brett.

Pardon my use of unzip, but after unzipping the wheel, I see that the RECORD file has the permission bits set to 000. The output of ls -l listed below for your perusal.

total 44
-rw-rw---- 1 jenkins jenkins  2692 Oct  2 06:01 METADATA
---------- 1 jenkins jenkins 27811 Oct  2 06:01 RECORD
-rw-rw---- 1 jenkins jenkins    82 Oct  2 06:01 WHEEL
-rw-rw---- 1 jenkins jenkins    57 Oct  2 06:01 entry_points.txt
-rw-rw---- 1 jenkins jenkins    17 Oct  2 06:01 top_level.txt

I am using wheel 0.32.0 and Python 2.7.6.

@eliasbrange
Copy link

AFAIK, there's no supported public API, as noted in the README:

It should be noted that wheel is not intended to be used as a library, and as such there is no stable, public API.

Thing is. That entered the readme around 3 months ago. Around 15 months after the previous release...

@agronholm
Copy link
Contributor

Thing is. That entered the readme around 3 months ago. Around 15 months after the previous release...

And I removed all the API documentation one year ago already.

@agronholm
Copy link
Contributor

That said, I am willing to provide a public API if that is what the community wants, but care must be taken to make the API narrow enough to give the maintainers (me) some wiggle room regarding the implementation.

@agronholm
Copy link
Contributor

Pardon my use of unzip, but after unzipping the wheel, I see that the RECORD file has the permission bits set to 000. The output of ls -l listed below for your perusal.

Would you mind creating a new ticket about this? I'll fix it in the next patch release.

@matthew-brett
Copy link
Author

Regardless of what happened before, my question now is whether there is some part of the API that can be supported with minimal effort and maximal benefit. I really hope so, because code duplication just means more points of breakage, and a weaker packaging system overall.

@agronholm
Copy link
Contributor

agronholm commented Oct 2, 2018

I think that at least the WheelFile class could be exposed in the public API. I'll have to look around to see what else other projects use from wheel.

@agronholm
Copy link
Contributor

I'm closing this in favor of #262.

@dholth
Copy link
Member

dholth commented Oct 4, 2018 via email

@wimglenn
Copy link
Contributor

wimglenn commented Oct 4, 2018

@dholth I understand that, but it's also better to have a reference implementation of the spec with a public API don't you think? It's a source of problems when the various implementations have slightly differing behavior. btw I moved my comment over to the other issue which looked like a more relevant place for discussion. #262

@mcepl
Copy link

mcepl commented Oct 16, 2018

Just to say that pbr has broken test suite as well. Patch for it has been attached to https://bugs.launchpad.net/pbr/+bug/1798130

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

9 participants