search

mesonbuild / meson-python

Meson PEP 517 Python build backend
https://mesonbuild.com/meson-python/
MIT License
118 stars 59 forks source link

Allow duplicate -arch in ARCHFLAGS #531

Closed matthew-brett closed 7 months ago

matthew-brett commented 7 months ago

We came across a situation where the build added duplicate archs in ARCHFLAGS, e.g. -arch arm64 -arch arm64. These usually pass through compilers etc without problem, but mesonpy was raising in that case, accusing the build of being multiarch. Fix and test.

dnicolodi commented 7 months ago

I am not convinced that we should fix this. ARCHFLAGS is a setuptools thing that we support for compatibility, the compiler or other parts of the toolchain do not know anything about it. IIRC setuptools allows you to pass any flag accepted by the compiler via ARCHFLAGS, meson-python does not allow this. I think it is much easier to fix your build scripts.

matthew-brett commented 7 months ago

So - is there a specific reason that an error is desirable here? I can't see any situation where the error would be anything but spurious. And if that is so, it seems to me unhelpful not to fix it, and so to force all your users to (independently) discover the problem, debug it, and find their own workarounds.

dnicolodi commented 7 months ago

Why would anyone set ARCHFLAGS='-arch arm64 -arch arm64'? It is much easier to fix the build script that generates this environment variable than to fix meson-python to accept it and wait till the next meson-python release to be able to run your builds.

As I explained, setuptools allows to pass anything via ARCHFLAGS but meson-python does not. Thus there are more sensible setting for the environment variable that are refused. I agree that the parsing of the environment variable and the error messages can be improved, but I don't think handling arbitrary flags to be passed via ARCHFLAGS is desirable.

matthew-brett commented 7 months ago

Could I ask you again - what is the purpose of the error? Who would be served by it? Just a personal opinion - but I think build systems really have to be careful not to trip people up for things that won't in fact cause a problem.

dnicolodi commented 7 months ago

Better fix in #532

rgommers commented 7 months ago

Thanks for the patch Matthew, and for the slightly cleaned fix Daniele. This should work in 0.16.0 now after gh-532. I agree this is a useful improvement, even though it's also true that cleaning up the duplicate flags would be better for the user. But it seems like we can support this without opening the gates to allowing more env var usage, so why not.

matthew-brett commented 7 months ago

Thanks - that's very helpful.