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

Ensure a clean copy of configure arguments is used #484

Merged
merged 1 commit into from
Apr 21, 2024

Conversation

freakboy3742
Copy link
Contributor

This is a variation on the change introduced by #473.

host_configure_args is defined as a class-level variable; however, there are build steps that assign this class-level instance directly to configure_cmd, and then concatenate build-specific commands. As a result, configure command line arguments can be duplicated (sometimes in contradictory ways).

I noticed this on the iOS builds; see L3 defining the args in this build - there are essentially three copies of the same command line arguments (--with-pydebug et al is defined 3 times).

However, it also impacts on any build that uses the UnixCrossBuild base class. AFAICT, the only example of a builder affected by this that is currently in use is the (now deprecated) WASI32 builders; those end up with artefacts like the --with-pydebug --without-pydebug arguments in this build log. This doesn't appear to be causing problems at present, but it is at the very least confusing.

This PR takes a copy of the class-level host_configure_args variable before modifying it, which should prevent different instance setups tainting each other. This is a pattern that is also followed by the handling of other args (e.g., defaultTestOpts and testFlags).

@vstinner vstinner merged commit 677b108 into python:main Apr 21, 2024
2 checks passed
@vstinner
Copy link
Member

LGTM, merged.

@freakboy3742 freakboy3742 deleted the clean_configure_env branch April 21, 2024 22:28
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

Successfully merging this pull request may close these issues.

2 participants