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

Remove build specific environment variables from the test environment #1863

Open
mayeut opened this issue Jun 9, 2024 · 3 comments
Open

Comments

@mayeut
Copy link
Member

mayeut commented Jun 9, 2024

Description

macOS builds are using multiple environment variables specific to the build environment:
MACOSX_DEPLOYMENT_TARGET, _PYTHON_HOST_PLATFORM, ARCHFLAGS, SDKROOT

Those are propagated to the test environments. They should probably be removed from those.

see #1856 (comment) and the 2 following comments for more context.

Build log

No response

CI config

No response

@henryiii
Copy link
Contributor

While MACOSX_DEPLOYMENT_TARGET makes sense, the others might cause it to be harder to build wheels in the test step if wheels are missing from test dependencies. Not the best idea to build here, in general, but it may currently work because the correct variables are set for building.

@mayeut
Copy link
Member Author

mayeut commented Jun 10, 2024

Maybe we should adapt them all then ?

If we're thinking that we might break some edge case that requires building from sources by removing any of those environment variable then IMHO we should update them all:

  • MACOSX_DEPLOYMENT_TARGET to the current running OS (this one is already done but was meant to be temporary, required by uv for now)
  • _PYTHON_HOST_PLATFORM/ARCHFLAGS to the current running test architecture as it's more likely to fail an universal2 build than a specific one. In fact, that was the issue when building matplotlib from sources in feat: build[uv] #1856 (comment), it might have failed later on of course.

@joerick
Copy link
Contributor

joerick commented Oct 27, 2024

I think removing these variables for the test step makes sense in general.

Probably the right approach would be to start again from os.environ rather than copying env here:

virtualenv_env = env.copy()

And equivalents for the other platforms.

  • MACOSX_DEPLOYMENT_TARGET to the current running OS (this one is already done but was meant to be temporary, required by uv for now)

I don't follow this - normally on a user's machine MACOSX_DEPLOYMENT_TARGET isn't set and installing with uv works.

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

No branches or pull requests

3 participants