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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear up that BEFORE_ALL runs outside of Python env #1147

Merged
merged 2 commits into from Jul 15, 2022

Conversation

abitrolly
Copy link
Contributor

No description provided.

@joerick
Copy link
Contributor

joerick commented Jun 25, 2022

I'm not totally sure I understand the problem here. What was your misunderstanding with the previous way it was worded?

@henryiii
Copy link
Contributor

How about keeping the old wording but adding after the parenthetical remark "before creating Python environments"?

@Czaki
Copy link
Contributor

Czaki commented Jun 26, 2022

I think that @abitrolly forget to link #1146 discussion.

docs/options.md Outdated
@@ -635,7 +635,7 @@ To specify more than one environment variable, separate the variable names by sp
### `CIBW_BEFORE_ALL` {: #before-all}
> Execute a shell command on the build system before any wheels are built.

Shell command to prepare a common part of the project (e.g. build or install libraries which does not depend on the specific version of Python).
Shell command that runs before creating Python environment, to build or install parts that do not depend on the specific version of Python.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Shell command that runs before creating Python environment, to build or install parts that do not depend on the specific version of Python.
Shell command to prepare a common part of the project (e.g. build or install libraries which does not depend on the specific version of Python). This runs before creating the Python environments for each build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a "common part of the project" doesn't include pieces that are shared between operating system environments, it is better to leave this scope escape off to avoid bloating the description beyond that is mere necessary.

@abitrolly
Copy link
Contributor Author

I'm not totally sure I understand the problem here.

The current doesn't contain necessary technical details to prevent people from shooting themselves in the foot as I did in #1146.

@joerick
Copy link
Contributor

joerick commented Jul 5, 2022

I've added a little more detail to this. I just wanted to make sure the point was made - 'before the Python environment is created' seemed a little hand-wavy to me (which Python environment - cibuildwheel creates many).

Look okay?

image

https://cibuildwheel--1147.org.readthedocs.build/en/1147/options/#before-all

@abitrolly
Copy link
Contributor Author

Looks good. Maybe too much text to my liking, but it solves the issue.

@joerick joerick merged commit 7c922f9 into pypa:main Jul 15, 2022
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.

None yet

4 participants