-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Migrate run-make/rustdoc-io-error
to rmake.rs
#124807
Migrate run-make/rustdoc-io-error
to rmake.rs
#124807
Conversation
Some changes occurred in run-make tests. cc @jieyouxu |
7a43b72
to
182fbf7
Compare
182fbf7
to
74475c9
Compare
permissions.set_readonly(false); | ||
fs::set_permissions(&out_dir, permissions).unwrap(); | ||
|
||
#[cfg(not(windows))] |
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.
Is there a reason assert_eq!(output.status.code().unwrap(), 1);
is non-Windows? Basic r/w permissions are available on Windows too right? Does rustdoc behave differently on Windows for a non-modifiable output directory?
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.
Because I couldn't ensure the exact error code value on Windows (I don't have windows) so for now I only check that it fails whatever the OS, and if it's unix, then I check the error code is 1.
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.
You could try editing PR CI to enable some Windows runners in PR CI and see if the error code is the same on Windows
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.
The original test was running fine on Windows so I suppose it's fine to just check the error code whatever the platform.
74475c9
to
6a614ad
Compare
Removed the |
Actually, the original test never ran on Windows 😄
|
Note for myself: should reduce coding while attending conferences. ^^' Let's make a trybuild to ensure it's ok, and otherwise let's see what windows actually return. @bors try |
…ror, r=<try> Migrate `run-make/rustdoc-io-error` to `rmake.rs` Part of rust-lang#121876. r? `@jieyouxu`
☀️ Try build successful - checks-actions |
Seems like windows is happy so let's go! :D @bors r=jieyouxu rollup |
☔ The latest upstream changes (presumably #124916) made this pull request unmergeable. Please resolve the merge conflicts. |
6a614ad
to
ab1a67e
Compare
Rebased. @bors r=jieyouxu rollup |
…error, r=jieyouxu Migrate `run-make/rustdoc-io-error` to `rmake.rs` Part of rust-lang#121876. r? `@jieyouxu`
c5bb1ac
to
1ba5c98
Compare
Updated! |
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.
Looks good! r=me after CI is green :3
@bors r+ |
@bors rollup=iffy (downgrading because no longer running on windows) |
…error, r=jieyouxu Migrate `run-make/rustdoc-io-error` to `rmake.rs` Part of rust-lang#121876. r? `@jieyouxu` try-job: armhf-gnu
Rollup of 7 pull requests Successful merges: - rust-lang#124807 (Migrate `run-make/rustdoc-io-error` to `rmake.rs`) - rust-lang#126095 (Migrate `link-args-order`, `ls-metadata` and `lto-readonly-lib` `run-make` tests to `rmake`) - rust-lang#126308 (Ban `ArrayToPointer` and `MutToConstPointer` from runtime MIR) - rust-lang#126620 (Actually taint InferCtxt when a fulfillment error is emitted) - rust-lang#126629 (Migrate `run-make/compressed-debuginfo` to `rmake.rs`) - rust-lang#126644 (Rewrite `extern-flag-rename-transitive`. `debugger-visualizer-dep-info`, `metadata-flag-frobs-symbols`, `extern-overrides-distribution` and `forced-unwind-terminate-pof` `run-make` tests to rmake) - rust-lang#126650 (Rename a bunch of things in the new solver and `rustc_type_ir`) r? `@ghost` `@rustbot` modify labels: rollup
@bors r- (failed in #126710 (comment)) |
Hecc... I don't understand how this can pass on |
Judging from
armhf-gnu fs perms is likely kinda weird... so I would be okay with slapping a //@ ignore-arm on this test with a "weird file perms on armhf-gnu" reason.
|
Ignored as well. Really dark magic... |
@bors r+ rollup=iffy |
☀️ Test successful - checks-actions |
Finished benchmarking commit (cb8a7ea): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (secondary -2.4%)This 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: 692.671s -> 691.131s (-0.22%) |
Part of #121876.
r? @jieyouxu
try-job: armhf-gnu