Fix some bootstrap papercuts #139823

WaffleLapkin · 2025-04-14T18:41:48Z

... related to jj and my ./build symlink setup¹.

I'm not sure if these are good solutions, but they seem to work. See commits for a bit more info.

r? @jieyouxu

see use realpath in bootstrap.py when creating build-dir #139804 ↩

rustbot · 2025-04-14T18:41:55Z

Some changes occurred in src/tools/compiletest

cc @jieyouxu

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

jieyouxu · 2025-04-15T03:48:19Z

src/bootstrap/src/core/config/config.rs

+        // NOTE: This check is required because `jj git clone` doesn't create directories for
+        // submodules, they are completely ignored. The code below assumes this directory exists,
+        // so create it here.
+        if !absolute_path.exists() {
+            t!(fs::create_dir_all(&absolute_path));
+        }


Remark: Path::exists will traverse symlinks to look for existence of the destination file (i.e. not necessarily the "shallow" $checkout_root/$usual_submodule_location), but I guess if the user has a symlink in the place of the usual submodules in checkout...

jieyouxu · 2025-04-15T03:50:20Z

src/tools/compiletest/src/runtest.rs

+        // Canonicalize test build directory path.
+        // Without this some tests fail if build directory is a symlink.
+        let output_base_dir = fs::canonicalize(self.output_base_dir()).unwrap();


Suggestion: I just switched compiletest over to use camino to enforce UTF-8 paths, so this should instead be

let output_base_dir = self.output_base_dir().canonicalize_utf8().unwrap();

Discussion: that being said, I'm not 100% sure this is sufficient in itself, since rustc itself might have inconsistencies when build/ is a symlink to elsewhere...

(note that there are more fs::canonicalize in the file, not sure if they are intended)

The tests seem to pass for me, so this is probably fine. I'm more concerned about a case where the directory doesn't exist, or, windows where canonicalize might return weird paths:

On Windows, this converts the path to use extended length path syntax, which allows your program to use longer path names, but means you can only join backslash-delimited paths to it, and it may be incompatible with other applications (if passed to the application on the command-line, or written to a file another application may read).

Yeah, some of these runtest bits need further auditing, which is beyond the scope of this PR.

jieyouxu · 2025-04-15T04:08:52Z

@rustbot author

rustbot · 2025-04-15T04:08:56Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Fix fixes failures of the following tests when build directory is a symlink: - `tests/ui/error-codes/E{0464,0523}.rs` - `tests/ui/crate-loading/crateresolve{1,2}.rs` (those are the same tests)

jieyouxu

Thanks

jieyouxu · 2025-04-15T07:52:41Z

@bors r+ rollup

bors · 2025-04-15T07:52:44Z

📌 Commit f038953 has been approved by jieyouxu

It is now in the queue for this repository.

WaffleLapkin · 2025-04-15T08:03:56Z

I ran more tests and some are broken (potentially as a result of this PR), so @bors r-

Apparently there are tests that print canonical paths *and* tests which print non-canonical paths. An example of the latter is `tests/ui/type_length_limit.rs`.

WaffleLapkin · 2025-04-15T22:11:19Z

@rustbot review
(see new commit)

jieyouxu

Huh... I guess I'm not really surprised that this isn't super consistent, lol. Thanks

jieyouxu · 2025-04-16T04:40:16Z

@bors r+ rollup

bors · 2025-04-16T04:40:19Z

📌 Commit 89b4eba has been approved by jieyouxu

It is now in the queue for this repository.

Fix some bootstrap papercuts ... related to jj and my `./build` symlink setup[^1]. I'm not sure if these are good solutions, but they seem to work. See commits for a bit more info. r? `@jieyouxu` [^1]: see rust-lang#139804

Rollup of 10 pull requests Successful merges: - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options) - rust-lang#139823 (Fix some bootstrap papercuts) - rust-lang#139853 (Disable combining LLD with external llvm-config) - rust-lang#139867 (Fix some tidy paper cuts) - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block) - rust-lang#139876 (Make CodeStats' type_sizes public) - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT) - rust-lang#139884 (Update books) - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks) - rust-lang#139893 (Add test for issue 125668) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options) - rust-lang#139823 (Fix some bootstrap papercuts) - rust-lang#139867 (Fix some tidy paper cuts) - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block) - rust-lang#139876 (Make CodeStats' type_sizes public) - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT) - rust-lang#139884 (Update books) - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks) - rust-lang#139893 (Add test for issue 125668) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139823 - WaffleLapkin:bootpaper, r=jieyouxu Fix some bootstrap papercuts ... related to jj and my `./build` symlink setup[^1]. I'm not sure if these are good solutions, but they seem to work. See commits for a bit more info. r? ``@jieyouxu`` [^1]: see rust-lang#139804

Don't canonicalize crate paths When printing paths in diagnostic we should favour printing the paths that were passed in rather than resolving all symlinks. This PR changes the form of the crate path but it should only really affect diagnostics as filesystem functions won't care which path is used. The uncanonicalized path was already used as a fallback for when canonicalization failed. This is a partial alternative to rust-lang#139823.

Rollup merge of rust-lang#139834 - ChrisDenton:spf, r=WaffleLapkin Don't canonicalize crate paths When printing paths in diagnostic we should favour printing the paths that were passed in rather than resolving all symlinks. This PR changes the form of the crate path but it should only really affect diagnostics as filesystem functions won't care which path is used. The uncanonicalized path was already used as a fallback for when canonicalization failed. This is a partial alternative to rust-lang#139823.

rustbot assigned jieyouxu Apr 14, 2025

This comment has been minimized.

Sign in to view

ChrisDenton mentioned this pull request Apr 15, 2025

Don't canonicalize crate paths #139834

Merged

jieyouxu reviewed Apr 15, 2025

View reviewed changes

rustbot 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 Apr 15, 2025

WaffleLapkin added 2 commits April 15, 2025 09:42

update submodules if the directory doesn't exist

fc8df06

fix typo

1397dab

WaffleLapkin force-pushed the bootpaper branch from 1076c7e to f038953 Compare April 15, 2025 07:44

canonicalize test build dir before normalizing it

6c441cc

Fix fixes failures of the following tests when build directory is a symlink: - `tests/ui/error-codes/E{0464,0523}.rs` - `tests/ui/crate-loading/crateresolve{1,2}.rs` (those are the same tests)

jieyouxu approved these changes Apr 15, 2025

View reviewed changes

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 15, 2025

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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 15, 2025

normalize canonical and non-canonical paths in compiletest

89b4eba

Apparently there are tests that print canonical paths *and* tests which print non-canonical paths. An example of the latter is `tests/ui/type_length_limit.rs`.

WaffleLapkin force-pushed the bootpaper branch from f038953 to 89b4eba Compare April 15, 2025 19:55

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 Apr 15, 2025

jieyouxu approved these changes Apr 16, 2025

View reviewed changes

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 16, 2025

Zalathar mentioned this pull request Apr 16, 2025

Rollup of 10 pull requests #139895

Closed

matthiaskrgr mentioned this pull request Apr 16, 2025

Rollup of 9 pull requests #139912

Merged

bors merged commit f7b6f15 into rust-lang:master Apr 16, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 16, 2025

WaffleLapkin deleted the bootpaper branch April 17, 2025 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix some bootstrap papercuts #139823

Fix some bootstrap papercuts #139823

WaffleLapkin commented Apr 14, 2025

rustbot commented Apr 14, 2025

This comment has been minimized.

jieyouxu Apr 15, 2025

jieyouxu Apr 15, 2025

jieyouxu Apr 15, 2025

WaffleLapkin Apr 15, 2025

jieyouxu Apr 15, 2025

jieyouxu commented Apr 15, 2025

rustbot commented Apr 15, 2025

jieyouxu left a comment

jieyouxu commented Apr 15, 2025

bors commented Apr 15, 2025

WaffleLapkin commented Apr 15, 2025

WaffleLapkin commented Apr 15, 2025

jieyouxu left a comment

jieyouxu commented Apr 16, 2025

bors commented Apr 16, 2025

Fix some bootstrap papercuts #139823

Fix some bootstrap papercuts #139823

Conversation

WaffleLapkin commented Apr 14, 2025

Footnotes

rustbot commented Apr 14, 2025

This comment has been minimized.

jieyouxu Apr 15, 2025

Choose a reason for hiding this comment

jieyouxu Apr 15, 2025

Choose a reason for hiding this comment

jieyouxu Apr 15, 2025

Choose a reason for hiding this comment

WaffleLapkin Apr 15, 2025

Choose a reason for hiding this comment

jieyouxu Apr 15, 2025

Choose a reason for hiding this comment

jieyouxu commented Apr 15, 2025

rustbot commented Apr 15, 2025

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Apr 15, 2025

bors commented Apr 15, 2025

WaffleLapkin commented Apr 15, 2025

WaffleLapkin commented Apr 15, 2025

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Apr 16, 2025

bors commented Apr 16, 2025