Implement excluding a build-step via --exclude #48105
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shepmaster
added
the
S-waiting-on-review
|
This will need a refactor of how we make multi-path default steps work. Waiting-on-author... |
Mark-Simulacrum
added
S-waiting-on-author
Previously, a Step would be able to tell on its own when it was invoked
"by-default" (that is, `./x.py test` was called instead of `./x.py test
some/path`). This commit replaces that functionality, invoking each Step
with each of the paths it has specified as "should be invoked by."
For example, if a step calls `path("src/tools/cargo")` and
`path("src/doc/cargo")` then it's make_run will be called twice, with
"src/tools/cargo" and "src/doc/cargo." This makes it so that default
handling logic is in builder, instead of spread across various Steps.
However, this meant that some Step specifications needed to be updated,
since for example `rustdoc` can be built by `./x.py build
src/librustdoc` or `./x.py build src/tools/rustdoc`. A `PathSet`
abstraction is added that handles this: now, each Step can not only list
`path(...)` but also `paths(&[a, b, ...])` which will make it so that we
don't invoke it with each of the individual paths, instead invoking it
with the first path in the list (though this shouldn't be depended on).
Future work likely consists of implementing a better/easier way for a
given Step to work with "any" crate in-tree, especially those that want
to run tests, build, or check crates in the std, test, or rustc crate
trees. Currently this is rather painful to do as most of the logic is
duplicated across should_run and make_run. It seems likely this can be
abstracted away into builder somehow.
Mark-Simulacrum
force-pushed
the
exclude-paths
branch
from
e66deb4 to
f104b12
Compare
|
@Mark-Simulacrum was this ready to go or did you want to queue up some more changes? (it looked ready!) |
|
I think with the latest commits this is ready for review. I have another branch with more work but that's separate and not quite ready. |
|
@bors: r+ |
|
📌 Commit f104b12 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit f104b12 with merge b8134c7b4df103f484d790d40be51404fe7d1491... |
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
Many failures, e.g. |
Some Steps are by-default run but don't have any paths associated with them. We need to have at least one PathSet per each Step, though, so we add an empty one on calls to `never()`.
|
@bors r=alexcrichton |
|
📌 Commit a64575c has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit a64575c with merge 4928171c911aca5e7538a49352f8815c9ce8d96f... |
bors
added
the
S-waiting-on-bors
|
⌛ Testing commit a64575c with merge 3e13f0e5cb7f97c681bb1e2a18d1b998cb5c3aac... |
|
💔 Test failed - status-appveyor |
bors
added
S-waiting-on-review
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Feb 14, 2018
Implement excluding a build-step via --exclude First step to fixing #47911. This doesn't change any CI configuration, but implements what I believe necessary to make that feasible in rustbuild. In theory this should be sufficient to allow someone to open a PR against .travis.yml and appveyor.yml which splits the Windows 32-bit tests and maybe the OS X tests into multiple builders (depending on what our cost-concerns are) to reduce runtimes. r? @alexcrichton cc @kennytm
|
💔 Test failed - status-appveyor |
bors
added
S-waiting-on-review
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Feb 15, 2018
Implement excluding a build-step via --exclude First step to fixing #47911. This doesn't change any CI configuration, but implements what I believe necessary to make that feasible in rustbuild. In theory this should be sufficient to allow someone to open a PR against .travis.yml and appveyor.yml which splits the Windows 32-bit tests and maybe the OS X tests into multiple builders (depending on what our cost-concerns are) to reduce runtimes. r? @alexcrichton cc @kennytm
|
☀️ Test successful - status-appveyor, status-travis |
Merged
This was referenced Mar 16, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First step to fixing #47911. This doesn't change any CI configuration, but implements what I believe necessary to make that feasible in rustbuild.
In theory this should be sufficient to allow someone to open a PR against .travis.yml and appveyor.yml which splits the Windows 32-bit tests and maybe the OS X tests into multiple builders (depending on what our cost-concerns are) to reduce runtimes.
r? @alexcrichton
cc @kennytm