gh-93584: Make all install+tests targets depends on all by tiran · Pull Request #93589 · python/cpython

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

Conversation

tiran

Copy link

Member

@tiran tiran commented Jun 7, 2022

All install targets use the "all" target as synchronization point to
prevent race conditions with PGO builds. PGO builds use recursive make,
which can lead to two parallel ./python setup.py build processes that
step on each others toes.

"test" targets now correctly compile PGO build in a clean repo.

All install targets use the "all" target as synchronization point to
prevent race conditions with PGO builds. PGO builds use recursive make,
which can lead to two parallel `./python setup.py build` processes that
step on each others toes.

"test" targets now correctly compile PGO build in a clean repo.

@tiran tiran marked this pull request as draft

Jun 7, 2022

@nascheme

This looks like a good cleanup to me. Perhaps a minor improvement is that we could introduce another name for what all these things depend on, something other than "all". Typically "all" is a dummy target that just builds all typical things. It would be a bit better to be more explicit what these other targets require. Could be something like:

all: python_runtime

python_runtime: @DEF_MAKE_RULE@

test: python_runtime

testall: python_runtime

.PHONY: python_runtime

Maybe that's over complication.

@pablogsal

I could not reproduce the error in #93586 anymore with this 🚀

@pablogsal

@tiran When is ready for review can you unmark it as a draft?

@tiran tiran marked this pull request as ready for review

Jun 8, 2022

@tiran tiran mentioned this pull request

Jun 8, 2022

@pablogsal

LGTM

I will leave it to you if you want to implement @nascheme's idea

pablogsal

@tiran

Copy link

Member Author

@tiran tiran commented Jun 8, 2022

I will leave it to you if you want to implement @nascheme's idea

I added a comment to the ticket. I like @nascheme idea, but would prefer to implement it in a separate PR.

@miss-islington

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington

Sorry @tiran, I had trouble checking out the 3.11 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 243ed5439c32e8517aa745bc2ca9774d99c99d0f 3.11

@tiran tiran deleted the gh-93584-make-sync-all branch

Jun 8, 2022

tiran added a commit to tiran/cpython that referenced this issue

Jun 8, 2022
…ythonGH-93589)

All install targets use the "all" target as synchronization point to
prevent race conditions with PGO builds. PGO builds use recursive make,
which can lead to two parallel `./python setup.py build` processes that
step on each others toes.

"test" targets now correctly compile PGO build in a clean repo.
(cherry picked from commit 243ed54)

Co-authored-by: Christian Heimes <christian@python.org>

@bedevere-bot

tiran added a commit that referenced this issue

Jun 8, 2022
) (GH-93603)

All install targets use the "all" target as synchronization point to
prevent race conditions with PGO builds. PGO builds use recursive make,
which can lead to two parallel `./python setup.py build` processes that
step on each others toes.

"test" targets now correctly compile PGO build in a clean repo.
(cherry picked from commit 243ed54)

Co-authored-by: Christian Heimes <christian@python.org>