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

Allow duplicate -arch in ARCHFLAGS #531

Closed

Conversation

matthew-brett
Copy link

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.

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

Better fix in #532

@dnicolodi dnicolodi closed this Nov 11, 2023
@rgommers rgommers added the enhancement New feature or request label Nov 22, 2023
@rgommers
Copy link
Contributor

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
Copy link
Author

Thanks - that's very helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants