-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix test panics for submodule of book is not updated #113965
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
r? @clubby789 (rustbot has picked a reviewer for you, use r? to override) |
This will not give the error(which it should give) if submodule was fetched but directory was corrupted/removed. Checking if submodule was fetched or not could be better option here. |
1c42446
to
b711c84
Compare
updated the code to check the directory and exit(1) if checking failed. |
b711c84
to
ca9a20b
Compare
This comment has been minimized.
This comment has been minimized.
ca9a20b
to
d46804c
Compare
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.
Thanks for the fix! Looks good to me.
Only 1 suggestion you may want to consider following it. r=me with/without it since it's not a blocker.
let absolute_path = builder.src.join(&relative_path); | ||
let redirect_path = absolute_path.join("redirects"); | ||
if !absolute_path.exists() | ||
|| !redirect_path.exists() | ||
|| dir_is_empty(&absolute_path) | ||
|| dir_is_empty(&redirect_path) | ||
{ |
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.
We can create function something like fn is_submodule_checked_out(module_path: &str) -> bool
that uses git submodule foreach
command inside.
@bors r=ozkanonur |
☀️ Test successful - checks-actions |
Finished benchmarking commit (d9d80e2): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 652.001s -> 651.923s (-0.01%) |
Perf: this is noise that is corrected in the next run. @rustbot label: +perf-regression-triaged |
Fixes #113963