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

Remove assertion that all paths in ShouldRun exist #96196

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Apr 19, 2022

This breaks on submodules (see #96188). Disable the assertion for now until I can think of a proper
fix.

This doesn't revert any of the changes in Steps themselves, only what
ShouldRun::paths does.

This breaks on submodules (see rust-lang#96188). Disable the assertion for now until I can think of a proper
fix.

This doesn't revert any of the changes in `Step`s themselves, only what
`ShouldRun::paths` does.
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 19, 2022
@Mark-Simulacrum
Copy link
Member

A reasonable idea is probably to just hardcode a list of submodules and check if the paths coincide - we don't update submodule lists often enough for that to matter.

(A test asserting that list might work for CI; it won't work in non-git checkouts though, so we should be careful. A test might not be entirely necessary.)

@bors r+ to unbreak things for now.

@bors
Copy link
Contributor

bors commented Apr 19, 2022

📌 Commit b4ca065 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2022
@Mark-Simulacrum
Copy link
Member

@bors p=1 to fix perf

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#90630 (Create real parser for search queries)
 - rust-lang#96193 ([fuchsia] Add implementation for `current_exe`)
 - rust-lang#96196 (Remove assertion that all paths in `ShouldRun` exist)
 - rust-lang#96228 (Fix locations for intrinsics impls and change to links)
 - rust-lang#96236 (Add an explicit `Span` field to `OutlivesConstraint`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Apr 21, 2022

⌛ Testing commit b4ca065 with merge 7be1da0...

@bors bors merged commit 0179aac into rust-lang:master Apr 21, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 21, 2022
@jyn514 jyn514 deleted the no-assertion branch April 21, 2022 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants