-
Notifications
You must be signed in to change notification settings - Fork 39
feat: Add bootstrap-parallel command #683
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
dhellmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I like the fact that it reuses the existing commands.
Is there anything we can do to automatically ensure that if we add options to build-parallel they are included here (via a linter or something)? Maybe it's not a big deal, but it seems like a potential source of UI drift.
eda605b to
ee708c8
Compare
|
Nice! Looks good to me! |
|
This PR contains the commit from PR #687 |
ee708c8 to
09982ac
Compare
The new `bootstrap-parallel` command combines `bootstrap --sdist-only` and `build-parallel` in a single, convenient command. Signed-off-by: Christian Heimes <cheimes@redhat.com>
The `build`, `build-sequence`, and `build-parallel` jobs now clean up sdist root directory and build env directory. The `build-parallel` command reuses the build env directory from the bootstrap phase in the build-paralel phase. This speed up package builds for packages that depend on large dependencies like Torch. Signed-off-by: Christian Heimes <cheimes@redhat.com>
09982ac to
a4ccdc5
Compare
I have added a simple check that compares options names. It's not going to detect different defaults, but at least it will tell us when an option is added or removed. |
dhellmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it. Let's release and see how the integration goes.
| # statistics | ||
| wheels = sorted(f.name for f in wkctx.wheels_downloads.glob("*.whl")) | ||
| sdists = sorted(f.name for f in wkctx.sdists_downloads.glob("*.tar.gz")) | ||
| logger.debug("wheels: %s", ", ".join(wheels)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These log lines could end up being pretty massive for our full index builds. Let's see how it goes, but I anticipate coming back and either listing a few packages per line or removing the log calls entirely.
The new
bootstrap-parallelcommand combinesbootstrap --sdist-onlyandbuild-parallelin a single, convenient command.