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

0.9.0 release produces macOS wheel platform tag that cannot be installed on Homebrew Python #160

Closed
rgommers opened this issue Sep 30, 2022 · 14 comments · Fixed by #161
Closed
Labels
bug Something isn't working

Comments

@rgommers
Copy link
Contributor

@mkoeppe noticed a problem with macOS wheel tags with Homebrew Python on Sage, see https://github.com/FFY00/meson-python/issues/91#issuecomment-1263004679. The linked Sage issue (https://trac.sagemath.org/ticket/34081#comment:90) shows:

[scipy-1.9.1] Using pip 22.2.2 from /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/pip (python 3.10)
[scipy-1.9.1] Looking in links: /Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.10/var/lib/sage/wheels
[scipy-1.9.1] ERROR: scipy-1.9.1-cp310-cp310-macosx_12_6_x86_64.whl is not a supported wheel on this platform.

@mkoeppe can you confirm that you built this wheel on macOS 12.6? If so, the wheel tag should be correct. However, there is a problem of course if pip refuses to install it, which we may have to work around.

Also, can you determine when it is installable? If you rename the wheel from 12_6 to say 12_3, 12_0, 11_0, it should become installable.

@rgommers
Copy link
Contributor Author

So the issue is that packaging is deliberately setting the macOS minor version to zero: https://github.com/pypa/packaging/blob/b8b2f8a/packaging/tags.py#L394-L403

The accepted tags are:

>>> from packaging import tags
>>> list(tags.mac_platforms())
['macosx_12_0_arm64',
 'macosx_12_0_universal2',
 'macosx_11_0_arm64',
 'macosx_11_0_universal2',
 'macosx_10_16_universal2',
 'macosx_10_15_universal2',
 'macosx_10_14_universal2',
 'macosx_10_13_universal2',
 'macosx_10_12_universal2',
 'macosx_10_11_universal2',
 'macosx_10_10_universal2',
 'macosx_10_9_universal2',
 'macosx_10_8_universal2',
 'macosx_10_7_universal2',
 'macosx_10_6_universal2',
 'macosx_10_5_universal2',
 'macosx_10_4_universal2']

That code is then used by pip, and it refuses to install a 12_6 rather than 12_0. The workaround for us is to copy that code exactly, so we also name wheels as 12_0.

Resetting the minor version like that is only justified if it's 100% guaranteed that minor versions are forward and backwards compatible. It's still not a great thing for packaging to be rejecting valid tags in such a strange manner, forcing us to copy-paste unnecessary code to avoid tags that are complying with PEP 3149 just fine. However, the more important question is whether that binary compatibility is actually guaranteed. I have not yet found an authoritative source stating that.

@rgommers
Copy link
Contributor Author

The main discussion focused on minor versions at the end, and pypa/packaging#319 (comment) says that (as one would expect) this is indeed not guaranteed. So the packaging code is now both unnecessary and buggy.

@rgommers
Copy link
Contributor Author

More archeology, from pypa/wheel#385:

the numbering of macOS has changed from 10.major.minor to major.minor.minor2.

In order to actually remove the minor version we'd have to coordinate with wheel (and maybe create a PEP?) Instead we just pin the minor version to 0.

Please pin the minor version to 0 and treat MACOSX_DEPLOYMENT_TARGET=N as MACOSX_DEPLOYMENT_TARGET=N.0.

Now let's test if this is actually true. Write a little test file tmp.c:

#include<stdio.h>
int main()
{
printf("\nthis is main file");
}

Now let's build for different minor versions, and see if they are the same:

% clang -mmacosx-version-min=12.0 tmp.c -o 12-0.o
% clang -mmacosx-version-min=12.4 tmp.c -o 12-4.o
% clang tmp.c -o current-os.o  # my machine is on 12.4

% otool -l 12-0.o | grep minos                   
    minos 12.0
% otool -l 12-4.o | grep minos
    minos 12.4
% otool -l current-os.o | grep minos
    minos 12.0

So yes and no. You are allowed to set the deployment target to a minimum version, and that info is kept in the produced binary. And presumably that won't then run on an older minor version - so technically packaging is wrong here. However, if you don't use -mmacosx-version-min explicitly, it defaults to major.0 - so in practice things are going to be mostly fine by pinning to 11_0, 12_0, etc.

@rgommers rgommers added the bug Something isn't working label Sep 30, 2022
@rgommers
Copy link
Contributor Author

@FFY00 let's copy the relevant lines of the packaging code verbatim and release 0.9.1?

@rgommers
Copy link
Contributor Author

I did some more testing and verified that both Clang and GCC do honor MACOSX_DEPLOYMENT_TARGET, so Meson actually doesn't need to do anything here. Clang docs, and test:

% echo $MACOSX_DEPLOYMENT_TARGET
                   
% clang tmp.c -o current-os.o              
% otool -l current-os.o | grep minos             
    minos 12.0
% export MACOSX_DEPLOYMENT_TARGET=12.2
% clang tmp.c -o current-os.o         
% otool -l current-os.o | grep minos  
    minos 12.2

So it looks like there's nothing Meson needs to be doing. Not sure what the deal is with CMake explicitly handling that env var, its code is not very understandable.

tl;dr all we should be doing is forcing the minor version to zero for macOS >= 11. And documenting that if the env var is set, please do set the minor version to zero.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 8, 2022

I can confirm that this fixes the reported problem (tested with meson-python 0.10.0). Thanks all!

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 1, 2022

The problem appears to have returned, this time affecting macOS Ventura (13). Reported in https://trac.sagemath.org/ticket/34658#comment:17; I have not reproduced it yet.

@dimpase
Copy link

dimpase commented Nov 1, 2022

It seems that packaging is not up to date in regard of macOS 13.

@dimpase
Copy link

dimpase commented Nov 1, 2022

that's what I see on an intel macOS 13

% /usr/local/opt/python@3/bin/python3
Python 3.10.8 (main, Oct 13 2022, 10:17:43) [Clang 14.0.0 (clang-1400.0.29.102)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from packaging import tags
>>> list(filter(lambda t:  "x_13" in t, tags.mac_platforms()))
['macosx_13_0_x86_64', 'macosx_13_0_intel', 'macosx_13_0_fat64', 'macosx_13_0_fat32', 'macosx_13_0_universal2', 'macosx_13_0_universal']
>>> 

With the Python I see scipy build error, I get

['macosx_13_0_arm64', 'macosx_13_0_universal2']

So it seems that _0 piece is gone missing during the build.

@dimpase
Copy link

dimpase commented Nov 2, 2022

packaging says it's basically the same bug as with macOS 12, see pypa/packaging#578

@dnicolodi
Copy link
Member

#191 makes meson-python use the packaging module to generate the platform tags, thus it should solve the discrepancies reported here. I don't know whether the tags should or should not have the minor version, but at least they will be consistent.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 7, 2022

Also fails on macOS Catalina (10.15.7)
https://trac.sagemath.org/ticket/34081#comment:121

@rgommers
Copy link
Contributor Author

I think this was fixed again by gh-202. If not, it'd be better to open a new issue so we don't lose track of this.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 17, 2022

Thanks, I'll wait for the next release to test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants