pkginfo deleted

[henryiii]

pkginfo seems to have been deleted in current main (no PR, but commit was 4c87d58). This is being used by @matthew-brett's delocate:

https://github.com/matthew-brett/delocate/blob/9c2c6a42c96b02602ce8d2059423a77e257a1a98/delocate/wheeltools.py#L18

Latest main does not therefore work with delocate, which breaks my ability to test #422 in the wild (on macOS). :'(

[agronholm]

The wheel README states that it has no public API. No package should therefore try to import wheel.

[agronholm]

Just what is the expectation here? That I put an unused module back in the repository?

[henryiii]

Let's see what @matthew-brett wants to do. This will break all older delocate's when the next version is released, so having some sort of shim for a bit would likely be nice. Delocate needs to stop using these, but wheel could be nice to delocate too. Was there really any reason to inline those functions? Easier to unit test the way they were.

[agronholm]

I can delay the next wheel release until @matthew-brett fixes the problem on their end, assuming that happens in a reasonable timeframe.

Was there really any reason to inline those functions? Easier to unit test the way they were.

Less clutter. Are you suggesting that testing is somehow harder now? I at least have not noticed that.

[henryiii]

tests/test_pkginfo.py was deleted and not replaced. It was possible to test this in isolation before, now it's not, as far as I can tell.

[agronholm]

This should not change the coverage of tests.

[henryiii]

Unit tests are better than integration tests - when something goes wrong, it's easier to see what goes wrong. Anyway, I'm just saying you are risking breaking a massive number of packages (basically all binary package distribution on macOS); adding this back in for a while with a FutureWarning would be friendlier and more Pythonic, even if it's delocate's fault.

[HexDecimal]

Delocate maintainer here.

The removed functions are very small and would be easy to rewrite in Delocate, and all independently written functions should be stable because the file format is stable. I'd be okay with doing this.

No public API means the upstream developers are not obligated to keep backwards compatibility for other projects. It doesn't mean that other projects won't or shouldn't use that API (especially in Python where nothing is really private.) Any assumption that the functions in wheel.pkginfo were only used in one place have been revealed to be false, but you're free to remove them if you think that's what's best. I don't want to slow down the development of wheel over this.

Removing tests/test_pkginfo.py was extreme. This change would have hit a lot of people by surprise if it wasn't for henryiii. It also reveals a bad assumption in Delocate since it isn't testing with the latest revisions of wheel.

[HexDecimal]

For those interested, this is a refactored version of the module that I plan on going with:

"""Tools for reading and writing PKG-INFO / METADATA without caring
about the encoding.

This is based on a copy of the wheel.pkginfo module.
"""
from email.generator import Generator
from email.message import Message
from email.parser import Parser
from os import PathLike
from typing import Union


def read_pkg_info_bytes(bytestr: Union[bytes, str]) -> Message:
    """Parse a PKG-INFO or METADATA data string."""
    if isinstance(bytestr, bytes):
        bytestr = bytestr.decode("utf-8")
    return Parser().parsestr(bytestr)


def read_pkg_info(path: Union[bytes, str, PathLike]) -> Message:
    """Read a PKG-INFO or METADATA file."""
    with open(path, encoding="utf-8") as headers:
        return Parser().parse(headers)


def write_pkg_info(path: Union[bytes, str, PathLike], message: Message) -> None:
    """Write to a PKG-INFO or METADATA file."""
    with open(path, "w", encoding="utf-8") as out:
        Generator(out, mangle_from_=False, maxheaderlen=0).flatten(message)

[henryiii]

I definitely think long-term the solution is for delocate to have it's own copy of these functions. I'm thinking that the "nice" thing for wheel to do is to provide them (with a FutureWarning) for a release or two; as much as I hate version capping, it does happen so some users are going to be broken if it's a hard cutoff. Delocate is a major user at least all the macOS-supporting packages here, and many, many more. This happened recently with packaging using a private API from pyparsing, and pyparsing restored the removed private API for a while to keep older packaging from breaking - they didn't have to, but it helps. Like packaging, delocate is in the wrong here (though it doesn't have the nice underscore, and not everybody reads README's!), but I think it would be nice for wheel to be nice here.

I would recommend wheel to put it's API behind an underscore, like pytest and pip do, if it really is serious about not having any public API, FWIW.

[HexDecimal]

In this case it was simple to replicate the module. I'm not really sure what to do in the long term. I assume that more complex functions are safe from being removed simply because it'd be harder to refactor them out. Delocate has already made updates for breaking changes in wheel before.

There isn't really a public equivalent to wheel that I know of. Maybe revisit issue #262?

[agronholm]

The longer term plan is to have a public API in wheel, in which case the private modules would be behind an underscore. This is something I have been (slowly) working towards, with the help of people like Pradyun Gedam. Until then I hoped that simply having a strong statement about it in the (short) README / Github front page would be enough.

[HexDecimal]

Is the progress on that tracked anywhere. Or do you mean the 1.0.0 milestone?

[agronholm]

What exactly are you importing wheel for? Is there something that packaging doesn't already do?

[agronholm]

I just want to reiterate that until there is a public API for wheel, importing it is not safe. No refactoring is off the table as far as internal code organization goes.

[HexDecimal]

With pkginfo removed it looks like Decloate still uses the following from wheel:

from wheel.util import native, urlsafe_b64encode
from wheel.wheelfile import WheelFile

I think the uses of wheel.util can be removed easily, but the dependency on WheelFile will be harder to take out.

[agronholm]

What is it doing with WheelFile? This class was going to be refactored into WheelReader and WheelWriter.

[agronholm]

In cases like these, vendoring wheel is the only "correct" solution until there is a release containing the public API.

[HexDecimal]

Looking into it more. Delocate uses WheelFile in one function where it's constructed and then the WheelFile.dist_info_path and WheelFile.parsed_filename attributes are read.

Maybe the packaging module has equivalents to these?

[agronholm]

Packaging contains parse_wheel_filename(). This is something wheel itself is going to use in the next release (I have a local branch for this).

[pfmoore]

And dist_info_path is simply {name}-{version}.dist-info, which is mandated by the specification, so doesn't really need a library function (except as a convenience).

Edit: The version in wheelfile is possibly wrong, in fact, as it doesn't seem to normalise the project name (which IIRC it should). Please don't take this as definite, though - I haven't had time to check properly.

[matthew-brett]

Thanks to all the swift work by HexDecimal - I believe Delocate will work with upcoming versions of Wheel - that's not a significant problem. The problem is the one that @henryiii has been talking about further up this thread- that new versions of Wheel will break old versions of Delocate, and this will cause some pain and confusion for macOS maintainers for a while. So here again is a plea to keep these APIs in for a while - say a year - with FutureWarnings attached to them, so there is some time for people to upgrade their Delocate versions, without first breaking against current Wheel - as a kindness to the community out there.

[agronholm]

Do you mean they would upgrade wheel without also upgrading delocate?

[matthew-brett]

Yes, that's right - that is how the problems could arise. And I'm sure that that exact thing will happen, with some degree of regularity. So this is just a plea on behalf of the packaging ecosystem, to make that transition more gentle.

[henryiii]

Adding limits causes the same issues, too - if someone installs a limited or pinned delocate, for example, they will suddenly start failing due to getting a new wheel and old delocate. If there was a deprecation period where warnings were issued, we could get an idea of how common this was before things broke. (Not as familiar with versions in delocate, but I have seen auditwheel<4, so ppl might limit delocate too).