fix building from source on macos by branchv · Pull Request #2593 · pygments/pygments
Since #2573 switched to hatchling, macOS users cannot build from source:
$ python3 -m venv .venv $ .venv/bin/pip install --no-binary=pygments pygments $ .venv/bin/python3 -c 'import pygments' Traceback (most recent call last): File "<string>", line 1, in <module> ModuleNotFoundError: No module named 'pygments'
This is because the wheel gets packaged as Pygments (prob due to macOS's case-insensitive filesystem). This explicitly tells hatchling that the package is pygments.
I find it hard to imagine that installing hatchling-built packages would be broken for everyone on macOS so I suspect there is something else going on. Let me investigate.
It's not broken for everyone, just macOS users building from source (i.e. using --no-binary, which is very few people but does include Homebrew. See Homebrew/homebrew-core#154759, and our failing CI here).
Time for a macOS CI builder? Also, this is weird, because the file generated is pygments now, no longer Pygments (at least on Linux) and I noticed the case change. Why would macOS hatchling suddenly produce an upper-case Pygments?
Why would macOS hatchling suddenly produce an upper-case Pygments?
So the sdist is correctly cased as pygments, but when hatchling builds a wheel from that sdist it infers the package name by first checking if Pygments/__init__.py is a file and then later pygments/__init__.py here. That works on Linux but on a case-insensitive filesystem like macOS, the former is "found" and thus hatchling uses it
If you ask me, any case-insensitive file system is a bug 😉 More seriously, yes, it does, I've just filed it as pypa/hatch#1054
After SSHing into a macOS VM that I have access to for another project, I can confirm that this fixes the problem. The wheel contents seem correct, including the RECORD and the capitalized name in METADATA. Merging, thank you! And it looks like we're going to need another bugfix release :(
Time for a macOS CI builder?
Yes, probably.
Also, this is weird, because the file generated is pygments now, no longer Pygments (at least on Linux) and I noticed the case change. Why would macOS hatchling suddenly produce an upper-case Pygments?
Just to be sure we're on the same page: it's not about the wheel file name, but the contents of the wheel.
In a wheel, you need to trim everything that's not needed: docs, tests, ... In our case, basically just keep everything in pygments/. This is hatchling's default behavior because it detects the very common case of having an import package that matches the project name. However, it tries the unnormalized project name (Pygments) first and then the normalized name (pygments). On macOS, a Pygments/ directory "exists", so it thinks that there is a Pygments/ package, and it adds all the files under a Pygments/ directory in the ZIP archive.
I'm pretty surprised that we're the first project to hit this (or maybe it was introduced recently in hatchling?).
Time for a macOS CI builder?
Yes, probably.
Are they still unproportionally slow? If yes, I'd vote to only use it selectively before release.
@jeanas Do I get it right we need a 2.17.2 to fix the packaing issue on macOS as currently there's nothing working on macOS? Can't they use the package from PyPI? I don't have a mac so I don't understand the extent of the issue. Not being able to build Pygments on macOS doesn't seem to warrant a hotfix if they can still use it.
Are they still unproportionally slow? If yes, I'd vote to only use it selectively before release.
We can also default to testing macOS with just one Python version instead of all supported ones, to reduce the matrix? Should be enough for the CI on each PR, though we may want more before a release...
@jeanas Do I get it right we need a 2.17.2 to fix the packaing issue on macOS as currently there's nothing working on macOS? Can't they use the package from PyPI? I don't have a mac so I don't understand the extent of the issue. Not being able to build Pygments on macOS doesn't seem to warrant a hotfix if they can still use it.
On PyPI, there is an sdist and a wheel. I'll refer you to this recent not yet merged PR if you're not sure what the usefulness of each is (I opened it because I used to be pretty confused about it...).
pip defaults to selecting wheels, so pip install pygments does work for most macOS users right now, because the wheel is the one you built on your Linux machine while doing the release, which is in good shape.
What doesn't work is (much more rare) things like pip install --no-binary :all: where pip is forced to build from the sdist. A few people do that sort of thing, e.g., because of extension modules that they want to build from source against Homebrew-provided libraries (for example). Or, because they want to audit everything and many builds aren't reproducible (I'm a student who's never worked in a corporate environment, so I can't tell you how common that actually is, but I've heard it exists).
IMO this kind of bug is an annoyance that should be addressed by a release, especially since it's a regression caused by us switching the build system.
That said, I have no objections to just picking the single commit onto 2.17.1 for the release, instead of releasing from current master.
That said, I have no objections to just picking the single commit onto 2.17.1 for the release, instead of releasing from current master.
I think this is a good idea. Some of the recent changes are quite huge diffs, let's not fold them into a point release.
I do think it should be fixed in the next release, I'm arguing if this warrants 2.17.2 on its own if PyPI users can use Pygments using the recommended installation method. If it's "much more rare" -- is it rare enough to meet whatever arbitrary bar we have for a point release, or is it too rare to warrant a fix now? I understand it's a regression, but it seems a fairly minor one.
If we do agree that 2.17.2 it is, then yes, I'll branch it off 2.17.1, and we'll cherry-pick that commit over. I would argue though that we should have a CI test for this to make sure we're actually not breaking anything while doing so. I have no access to a mac machine to try myself, and I feel somewhat uncomfortable about this. Sure, I can merge this PR and believe everyone it's working, but having a way to actually verify this would be much better.
Can I get a quick vote on 2.17.2 yay/nay for this one fix only? My vote is no, fold into 2.18, but we strategically decided to have three maintainers so you can overrule me.
I vote for the 2.17.2. If it's such a chore to do a hotfix then we really need to change the process.
I vote for the 2.17.2. If it's such a chore to do a hotfix then we really need to change the process.
+1 on both points.
Something is broken. I'm building the branch release/2.17.x, the wheel size looks fine, but the tar.gz is now 983 KiB instead of the expected 4.8 MiB :( Seems to be missing all tests and everything. Is this another hatchling problem? @jeanas , did this remove all tests from the sdist as a drive-by?
(pygments) anteru@vm:~/pygments$ python -m build * Creating virtualenv isolated environment... * Installing packages in isolated environment... (hatchling) * Getting build dependencies for sdist... * Building sdist... * Building wheel from sdist * Creating virtualenv isolated environment... * Installing packages in isolated environment... (hatchling) * Getting build dependencies for wheel... * Building wheel... Successfully built pygments-2.17.2.tar.gz and pygments-2.17.2-py3-none-any.whl (pygments) anteru@vm:~/pygments$ ls -lah dist/ total 2.1M drwxr-xr-x 2 anteru anteru 4.0K Nov 21 21:13 . drwxr-xr-x 10 anteru anteru 4.0K Nov 21 21:13 .. -rw------- 1 anteru anteru 1.2M Nov 21 21:13 pygments-2.17.2-py3-none-any.whl -rw------- 1 anteru anteru 983K Nov 21 21:13 pygments-2.17.2.tar.gz (pygments) anteru@vm:~/pygments$ cd dist/ (pygments) anteru@vm:~/pygments/dist$ tar xzf pygments-2.17.2.tar.gz (pygments) anteru@vm:~/pygments/dist$ ls pygments-2.17.2 pygments-2.17.2-py3-none-any.whl pygments-2.17.2.tar.gz (pygments) anteru@vm:~/pygments/dist$ cd pygments-2.17.2/ (pygments) anteru@vm:~/pygments/dist/pygments-2.17.2$ ls AUTHORS description.rst LICENSE PKG-INFO pygments pyproject.toml
Thanks. I added some more information. If I had to guess, the packages statement makes it only include the pygments folder.
@birkenfeld Not a chore per se, more like a wild ride with no safety belt and on a rusty looping designed by someone playing Rollercoaster Tycoon.
Oh, I see. I've pushed 2972a1c to master.
2.17.2 is out. Thank you @Anteru! @branchvincent Could you confirm it worked for Homebrew?
This is now fixed in hatchling, thank you @ofek!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters