Migrate `incr-add-rust-src-component` and `issue-84395-lto-embed-bitcode` `run-make` tests to rmake #128562

Oneirical · 2024-08-02T17:43:36Z

Part of #121876 and the associated Google Summer of Code project.

try-job: x86_64-gnu-debug
try-job: test-various
try-job: dist-various-1
try-job: armhf-gnu
try-job: aarch64-gnu
try-job: aarch64-apple
try-job: x86_64-msvc
try-job: x86_64-mingw
try-job: i686-msvc
try-job: i686-mingw

rustbot · 2024-08-02T17:43:43Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-08-02T17:43:45Z

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

Kobzol · 2024-08-02T19:22:37Z

@bors try

Migrate `incr-add-rust-src-component` and `issue-84395-lto-embed-bitcode` `run-make` tests to rmake Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). In `incr-add-rust-src-component`, I directly modify the sysroot, unlike the original test which used a symlink. This is probably a very bad idea, but I want to see what happens. try-job: x86_64-gnu-debug

bors · 2024-08-02T19:23:49Z

⌛ Trying commit 25380bc with merge 5de06a2...

bors · 2024-08-02T20:56:13Z

💔 Test failed - checks-actions

bors · 2024-08-02T21:49:52Z

☔ The latest upstream changes (presumably #128361) made this pull request unmergeable. Please resolve the merge conflicts.

Oneirical · 2024-08-05T17:36:34Z

@bors try

(the incr-add-rust-src-component is still broken, but the try job should only run issue-84395-lto-embed-bitcode)

@rustbot review

About the sysroot problem: the original test creates a symbolic link to the sysroot and starts editing it - how does this not just follow the link to the real sysroot and break the test?

The syslink feature in run_make_support seems much more limited. One clear example is when I tried to port the test libs-through-symlinks, which has this line: ln -nsf outdir/$(NAME) $(TMPDIR).

This means it's supposed to make the current working directory a link to another place while simultaneously keeping all its contents. This is some quantum physics wormhole stuff.

bors · 2024-08-05T17:36:37Z

@Oneirical: 🔑 Insufficient privileges: not in try users

Kobzol · 2024-08-05T18:04:04Z

@bors try

bors · 2024-08-05T18:10:29Z

⌛ Trying commit 76a7e72 with merge 67d4129...

Migrate `incr-add-rust-src-component` and `issue-84395-lto-embed-bitcode` `run-make` tests to rmake Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). In `incr-add-rust-src-component`, I directly modify the sysroot, unlike the original test which used a symlink. This is probably a very bad idea, but I want to see what happens. try-job: x86_64-gnu-debug

jieyouxu · 2024-08-17T05:22:10Z

It turns out the compiletest fix is going to be blocked on a bootstrap fix for similar problems with symlink on windows.

…nks, r=<try> bootstrap: fix clean's remove_dir_all implementation It turns out bootstrap's `clean.rs`'s hand-rolled `rm_rf` (which probably comes before `std::fs::remove_dir_all` was stable) is very broken on native Windows around both read-only files/directories and especially symbolic links. So instead of rolling our own, just use `std::fs::remove_dir_all`. Because I wasn't super sure about `std::fs::remove_dir_all`'s behavior and to catch `std::fs::remove_dir_all`'s behavioral changes here on forward, I added a collection of tests that checks if our expectation of the behavior of `std::fs::remove_dir_all` and its underlying Unix syscalls and Win32 APIs matches with its actual behavior. This is a blocker for compiletest's own `rm_rf` implementation rust-lang#129155 which happens to be also buggy, which in turn is a blocker for the rmake.rs test port rust-lang#128562 that heavily exercises symlinks (I was reviewing rust-lang#128562 and testing it on native Windows which is how I found out). I also left a FIXME for `detect_src_and_out` due to a failing assertion on native Windows: ``` ---- core::config::tests::detect_src_and_out stdout ---- thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:72:13: assertion `left == right` failed left: "E:\\tmp" right: "C:\\tmp" ``` Fixes rust-lang#112544 (because now we handle Windows symlinks properly). try-job: x86_64-msvc try-job: i686-mingw try-job: test-various try-job: armhf-gnu try-job: aarch64-apple try-job: aarch64-gnu

…nks, r=<try> bootstrap: fix clean's remove_dir_all implementation It turns out bootstrap's `clean.rs`'s hand-rolled `rm_rf` (which probably comes before `std::fs::remove_dir_all` was stable) is very broken on native Windows around both read-only files/directories and especially symbolic links. So instead of rolling our own, just use `std::fs::remove_dir_all`. This is a blocker for compiletest's own `rm_rf` implementation rust-lang#129155 which happens to be also buggy, which in turn is a blocker for the rmake.rs test port rust-lang#128562 that heavily exercises symlinks (I was reviewing rust-lang#128562 and testing it on native Windows which is how I found out). I also left a FIXME for `detect_src_and_out` due to a failing assertion on native Windows (opened rust-lang#129188): ``` ---- core::config::tests::detect_src_and_out stdout ---- thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:72:13: assertion `left == right` failed left: "E:\\tmp" right: "C:\\tmp" ``` Fixes rust-lang#112544 (because now we handle Windows symlinks properly). try-job: x86_64-msvc try-job: i686-mingw try-job: test-various try-job: armhf-gnu try-job: aarch64-apple try-job: aarch64-gnu

…links, r=Kobzol bootstrap: fix clean's remove_dir_all implementation It turns out bootstrap's `clean.rs`'s hand-rolled `rm_rf` (which probably comes before `std::fs::remove_dir_all` was stable) is very broken on native Windows around both read-only files/directories and especially symbolic links. So instead of rolling our own, just use `std::fs::remove_dir_all`. This is a blocker for compiletest's own `rm_rf` implementation rust-lang#129155 which happens to be also buggy, which in turn is a blocker for the rmake.rs test port rust-lang#128562 that heavily exercises symlinks (I was reviewing rust-lang#128562 and testing it on native Windows which is how I found out). I also left a FIXME for `detect_src_and_out` due to a failing assertion on native Windows (opened rust-lang#129188): ``` ---- core::config::tests::detect_src_and_out stdout ---- thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:72:13: assertion `left == right` failed left: "E:\\tmp" right: "C:\\tmp" ``` Fixes rust-lang#112544 (because now we handle Windows symlinks properly). try-job: x86_64-msvc try-job: i686-mingw try-job: test-various try-job: armhf-gnu try-job: aarch64-apple try-job: aarch64-gnu

…iler-errors compiletest: use `std::fs::remove_dir_all` now that it is available It turns out `aggressive_rm_rf` is not sufficiently aggressive (RAGEY) on Windows and obviously handles Windows symlinks incorrectly. Instead of rolling our own version, let's use `std::fs::remove_dir_all` now that it's available (well, it's been available for a good while, but probably wasn't available when this helper was written). cc rust-lang#129187 since basically this is failing due to similar problems. Blocker for rust-lang#128562. Fixes rust-lang#129155. Fixes rust-lang#126334.

…links, r=Kobzol bootstrap: fix clean's remove_dir_all implementation It turns out bootstrap's `clean.rs`'s hand-rolled `rm_rf` (which probably comes before `std::fs::remove_dir_all` was stable) is very broken on native Windows around both read-only files/directories and especially symbolic links. So instead of rolling our own, just use `std::fs::remove_dir_all`. This is a blocker for compiletest's own `rm_rf` implementation rust-lang#129155 which happens to be also buggy, which in turn is a blocker for the rmake.rs test port rust-lang#128562 that heavily exercises symlinks (I was reviewing rust-lang#128562 and testing it on native Windows which is how I found out). I also left a FIXME for `detect_src_and_out` due to a failing assertion on native Windows (opened rust-lang#129188): ``` ---- core::config::tests::detect_src_and_out stdout ---- thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:72:13: assertion `left == right` failed left: "E:\\tmp" right: "C:\\tmp" ``` Fixes rust-lang#112544 (because now we handle Windows symlinks properly). try-job: x86_64-msvc try-job: i686-mingw try-job: test-various try-job: armhf-gnu try-job: aarch64-apple try-job: aarch64-gnu

…iler-errors compiletest: use `std::fs::remove_dir_all` now that it is available It turns out `aggressive_rm_rf` is not sufficiently aggressive (RAGEY) on Windows and obviously handles Windows symlinks incorrectly. Instead of rolling our own version, let's use `std::fs::remove_dir_all` now that it's available (well, it's been available for a good while, but probably wasn't available when this helper was written). cc rust-lang#129187 since basically this is failing due to similar problems. Blocker for rust-lang#128562. Fixes rust-lang#129155. Fixes rust-lang#126334.

Rollup merge of rust-lang#129302 - jieyouxu:compiletest-RAGEY, r=compiler-errors compiletest: use `std::fs::remove_dir_all` now that it is available It turns out `aggressive_rm_rf` is not sufficiently aggressive (RAGEY) on Windows and obviously handles Windows symlinks incorrectly. Instead of rolling our own version, let's use `std::fs::remove_dir_all` now that it's available (well, it's been available for a good while, but probably wasn't available when this helper was written). cc rust-lang#129187 since basically this is failing due to similar problems. Blocker for rust-lang#128562. Fixes rust-lang#129155. Fixes rust-lang#126334.

Rollup merge of rust-lang#129187 - jieyouxu:squeaky-clean-windows-symlinks, r=Kobzol bootstrap: fix clean's remove_dir_all implementation It turns out bootstrap's `clean.rs`'s hand-rolled `rm_rf` (which probably comes before `std::fs::remove_dir_all` was stable) is very broken on native Windows around both read-only files/directories and especially symbolic links. So instead of rolling our own, just use `std::fs::remove_dir_all`. This is a blocker for compiletest's own `rm_rf` implementation rust-lang#129155 which happens to be also buggy, which in turn is a blocker for the rmake.rs test port rust-lang#128562 that heavily exercises symlinks (I was reviewing rust-lang#128562 and testing it on native Windows which is how I found out). I also left a FIXME for `detect_src_and_out` due to a failing assertion on native Windows (opened rust-lang#129188): ``` ---- core::config::tests::detect_src_and_out stdout ---- thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:72:13: assertion `left == right` failed left: "E:\\tmp" right: "C:\\tmp" ``` Fixes rust-lang#112544 (because now we handle Windows symlinks properly). try-job: x86_64-msvc try-job: i686-mingw try-job: test-various try-job: armhf-gnu try-job: aarch64-apple try-job: aarch64-gnu

jieyouxu · 2024-09-27T13:49:18Z

The symlink troubles observed previously should be related to admin ownership of artifacts when running in non-admin mode, which is not a blocker for this PR.

bors · 2024-10-25T21:27:31Z

☔ The latest upstream changes (presumably #131917) made this pull request unmergeable. Please resolve the merge conflicts.

jieyouxu · 2024-12-23T14:24:11Z

Opened #134656 for the incr-add-rust-src-component one.

jieyouxu · 2024-12-25T11:04:50Z

Ok it turns out I copied over and fixed up the lto bitcode test in #131917 already.

jieyouxu · 2024-12-25T11:05:22Z

Closing this in favor of #134656 and #131917 for each of the two tests respectively. Thanks!

…-component, r=wesleywiser Migrate `incr-add-rust-src-component` to rmake This PR partially supersedes rust-lang#128562, and ports the Makefile-based `tests/run-make/incr-add-rust-src-component` to use rmake.rs infra. Part of rust-lang#121876. This run-make test is a regression test for rust-lang#70924. It (tries to) checks that if we add the `rust-src` component in between two incremental compiles, that the compiler doesn't ICE on the second invocation. - Original issue:rust-lang#70924 - Fix PR: rust-lang#72767 - PR adding this regression test: rust-lang#72952 However, the Makefile version of this used `$SYSROOT/lib/rustlib/src/rust/src/libstd/lib.rs`, but that actually got moved around and reorganized over the years. As of Dec 2024, the `rust-src` component is more like (specific for our purposes): ``` $SYSROOT/lib/rustlib/src/rust/ library/std/src/lib.rs src/ ``` However, this run-make test is ancient and it exercises incr-comp system logic. I'm not sure if this test would actually catch the original regression. This PR was co-authored with `@Oneirical.` r? incremental try-job: i686-msvc try-job: x86_64-mingw-1 try-job: x86_64-msvc try-job: aarch64-apple

Rollup merge of rust-lang#134656 - jieyouxu:migrate-incr-add-rust-src-component, r=wesleywiser Migrate `incr-add-rust-src-component` to rmake This PR partially supersedes rust-lang#128562, and ports the Makefile-based `tests/run-make/incr-add-rust-src-component` to use rmake.rs infra. Part of rust-lang#121876. This run-make test is a regression test for rust-lang#70924. It (tries to) checks that if we add the `rust-src` component in between two incremental compiles, that the compiler doesn't ICE on the second invocation. - Original issue:rust-lang#70924 - Fix PR: rust-lang#72767 - PR adding this regression test: rust-lang#72952 However, the Makefile version of this used `$SYSROOT/lib/rustlib/src/rust/src/libstd/lib.rs`, but that actually got moved around and reorganized over the years. As of Dec 2024, the `rust-src` component is more like (specific for our purposes): ``` $SYSROOT/lib/rustlib/src/rust/ library/std/src/lib.rs src/ ``` However, this run-make test is ancient and it exercises incr-comp system logic. I'm not sure if this test would actually catch the original regression. This PR was co-authored with `@Oneirical.` r? incremental try-job: i686-msvc try-job: x86_64-mingw-1 try-job: x86_64-msvc try-job: aarch64-apple

rustbot assigned jieyouxu Aug 2, 2024

Oneirical force-pushed the no-contest-expected branch from 68eaebb to 84d0649 Compare August 2, 2024 17:44

This comment has been minimized.

Sign in to view

Oneirical force-pushed the no-contest-expected branch from 84d0649 to 25380bc Compare August 2, 2024 18:41

Kobzol mentioned this pull request Aug 2, 2024

Tracking Issue for porting run-make tests to use Rust #121876

Open

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2024

Oneirical force-pushed the no-contest-expected branch from 25380bc to 76a7e72 Compare August 5, 2024 17:35

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 5, 2024

This comment has been minimized.

Sign in to view

jieyouxu mentioned this pull request Aug 17, 2024

bootstrap: fix clean's remove_dir_all implementation #129187

Merged

jieyouxu mentioned this pull request Aug 20, 2024

compiletest: use std::fs::remove_dir_all now that it is available #129302

Merged

jieyouxu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 27, 2024

jieyouxu mentioned this pull request Dec 23, 2024

Migrate incr-add-rust-src-component to rmake #134656

Merged

jieyouxu closed this Dec 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate `incr-add-rust-src-component` and `issue-84395-lto-embed-bitcode` `run-make` tests to rmake #128562

Migrate `incr-add-rust-src-component` and `issue-84395-lto-embed-bitcode` `run-make` tests to rmake #128562

Oneirical commented Aug 2, 2024 •

edited

Loading

rustbot commented Aug 2, 2024

rustbot commented Aug 2, 2024

This comment has been minimized.

Kobzol commented Aug 2, 2024

bors commented Aug 2, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Aug 2, 2024

bors commented Aug 2, 2024

Oneirical commented Aug 5, 2024 •

edited

Loading

bors commented Aug 5, 2024

Kobzol commented Aug 5, 2024

bors commented Aug 5, 2024

This comment has been minimized.

This comment has been minimized.

jieyouxu commented Aug 17, 2024

jieyouxu commented Sep 27, 2024

bors commented Oct 25, 2024

jieyouxu commented Dec 23, 2024

jieyouxu commented Dec 25, 2024

jieyouxu commented Dec 25, 2024

Migrate incr-add-rust-src-component and issue-84395-lto-embed-bitcode run-make tests to rmake #128562

Migrate incr-add-rust-src-component and issue-84395-lto-embed-bitcode run-make tests to rmake #128562

Conversation

Oneirical commented Aug 2, 2024 • edited Loading

rustbot commented Aug 2, 2024

rustbot commented Aug 2, 2024

This comment has been minimized.

Kobzol commented Aug 2, 2024

bors commented Aug 2, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Aug 2, 2024

bors commented Aug 2, 2024

Oneirical commented Aug 5, 2024 • edited Loading

bors commented Aug 5, 2024

Kobzol commented Aug 5, 2024

bors commented Aug 5, 2024

This comment has been minimized.

This comment has been minimized.

jieyouxu commented Aug 17, 2024

jieyouxu commented Sep 27, 2024

bors commented Oct 25, 2024

jieyouxu commented Dec 23, 2024

jieyouxu commented Dec 25, 2024

jieyouxu commented Dec 25, 2024

Migrate `incr-add-rust-src-component` and `issue-84395-lto-embed-bitcode` `run-make` tests to rmake #128562

Migrate `incr-add-rust-src-component` and `issue-84395-lto-embed-bitcode` `run-make` tests to rmake #128562

Oneirical commented Aug 2, 2024 •

edited

Loading

Oneirical commented Aug 5, 2024 •

edited

Loading