-
Notifications
You must be signed in to change notification settings - Fork 766
fix building from source on macos #2593
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
Conversation
|
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 |
|
Time for a macOS CI builder? Also, this is weird, because the file generated is |
So the sdist is correctly cased as |
|
Sounds like a bug in hatchling... |
|
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 |
Yes, probably.
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 I'm pretty surprised that we're the first project to hit this (or maybe it was introduced recently in hatchling?). |
Are they still unproportionally slow? If yes, I'd vote to only use it selectively before release. |
|
We could configure one to only run on "tagged" builds. I think GitHub allows for that. |
|
@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. |
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...
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 What doesn't work is (much more rare) things like |
|
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 |
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. |
+1 on both points. |
Add a workaround for pypa/hatch#1054 (cherry picked from commit c178ba3)
|
Ok. Getting a 2.17.2 out now. |
|
Something is broken. I'm building the branch (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 |
|
Let me look into it.
|
|
Thanks. I added some more information. If I had to guess, the packages statement makes it only include the |
|
@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. |
|
Ok. I'll cherry-pick that as well. |
|
2.17.2 is out. Thank you @Anteru! @branchvincent Could you confirm it worked for Homebrew? |
|
can confirm it works for us, thanks everyone! |
|
Sorry for the trouble everyone! Here's an update: pypa/hatch#1054 (comment) |
|
This is now fixed in hatchling, thank you @ofek! |
Since #2573 switched to
hatchling, macOS users cannot build from source:This is because the wheel gets packaged as
Pygments(prob due to macOS's case-insensitive filesystem). This explicitly tellshatchlingthat the package ispygments.