Remap Cargo dependencies to /rust/deps #115872

pietroalbini · 2023-09-15T14:41:11Z

⚠️ This doesn't affect user-compiled programs, it only affects building the Rust compiler itself. ⚠️

Right now, rust.remap-debuginfo = true doesn't completely remap all paths: while LLVM and rustc sources are properly remapped (respectively to /rust/llvm and /rust/$commit), Cargo dependencies still use absolute paths from the Cargo home.

This never affected builds from CI much, because CARGO_HOME=/cargo in CI, so users see paths like this included in the precompiled binaries and libraries:

/cargo/registry/src/index.crates.io-6f17d22bba15001f/gimli-0.26.2/src/read/line.rs

Builds outside CI don't have remapping though, and it's confusing that the config flag doesn't fully do what it advertises.

This PR fixes it by adding remapping for dependencies too. All registries's source directory are remapped to /rust/deps, to account for multiple registries being able to contain crates.io crates (sparse index vs git, and source replacement mirrors). This results in paths like this being included:

/rust/deps/gimli-0.26.2/src/read/line.rs

rustbot · 2023-09-15T14:41:18Z

r? @clubby789

(rustbot has picked a reviewer for you, use r? to override)

pietroalbini · 2023-09-15T14:41:33Z

Note that tests will fail until #115869 is merged.

bjorn3 · 2023-09-15T16:35:12Z

All registries's source directory are remapped to /rust/deps, to account for multiple registries being able to contain crates.io crates (sparse index vs git, and source replacement mirrors).

Why not use the registry url as present in Cargo.lock? For crates.io this is fixed independent of if the git or sparse registry is used. Multiple registries may contain the same crate name and version with different sources. Cargo will consider them distinct crates. Omitting the registry will not allow distinguishing between them.

pietroalbini · 2023-09-18T07:44:48Z

Why not use the registry url as present in Cargo.lock? For crates.io this is fixed independent of if the git or sparse registry is used. Multiple registries may contain the same crate name and version with different sources. Cargo will consider them distinct crates. Omitting the registry will not allow distinguishing between them.

Hmm, how would you implement that? This is implemented without any kind of support on the Cargo side right now, and I'd rather avoid reimplementing trim_paths in bootstrap. I expect this will be reverted once trim_paths is implemented in Cargo, and I'm not aware of anyone using private registries for rustc itself.

bjorn3 · 2023-09-19T09:14:57Z

Maybe just hard code git.luolix.top-1ecc6299db9ec823 and index.crates.io-6f17d22bba15001f as mapping to /rust/deps/git.luolix.top-1ecc6299db9ec823 or whatever path will be used by cargo in the future for trim-paths?

bors · 2023-10-14T21:16:04Z

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

pietroalbini · 2023-10-24T10:25:33Z

Rebased on top of master.

Maybe just hard code git.luolix.top-1ecc6299db9ec823 and index.crates.io-6f17d22bba15001f as mapping to /rust/deps/git.luolix.top-1ecc6299db9ec823 or whatever path will be used by cargo in the future for trim-paths?

I'm not sure which path Cargo will use for trim-paths, but I don't have problems with rustc using its own prefix (like it does for /rust/$sha and /rust/llvm).

clubby789 · 2023-10-25T13:32:32Z

@bors r+

bors · 2023-10-25T13:32:35Z

📌 Commit 85c0ce2 has been approved by clubby789

It is now in the queue for this repository.

bors · 2023-10-26T00:12:21Z

⌛ Testing commit 85c0ce2 with merge 278eaf5...

bors · 2023-10-26T02:06:59Z

☀️ Test successful - checks-actions
Approved by: clubby789
Pushing 278eaf5 to master...

rust-timer · 2023-10-26T03:28:11Z

Finished benchmarking commit (278eaf5): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

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.

	mean	range	count
Regressions ❌ (primary)	1.6%	[1.5%, 1.7%]	2
Regressions ❌ (secondary)	1.2%	[1.2%, 1.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-3.1%, -2.6%]	2
All ❌✅ (primary)	1.6%	[1.5%, 1.7%]	2

Cycles

Results

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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.5%	[2.2%, 2.7%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: missing data
Artifact size: 304.48 MiB -> 304.41 MiB (-0.02%)

weihanglo · 2024-01-02T22:15:56Z

FWIW, here are proposed remap rules in Cargo: rust-lang/cargo#13171

…esleywiser bootstrap: handle vendored sources when remapping crate paths rust-lang#115872 introduced a feature to add path remapping for crate dependencies, but only when they came from Cargo's registry cache, not a vendor directory. This caused builds that used remapped debuginfo and vendor directories to fail with: ``` std::fs::read_dir(registry_src) failed with No such file or directory (os error 2) ``` or (if the `registry/src` directory exists but is empty) ``` error: --remap-path-prefix must contain '=' between FROM and TO ``` Fixes rust-lang#117885 by explicitly supporting the `vendor` directory and adding it to `RUSTC_CARGO_REGISTRY_SRC_TO_REMAP`. Note that `bootstrap.py` already assumes that `./vendor` within the rust repo is the only supported vendoring location. r? `@pietroalbini`

Rollup merge of rust-lang#119582 - arlosi:bootstrap-vendor-remap, r=wesleywiser bootstrap: handle vendored sources when remapping crate paths rust-lang#115872 introduced a feature to add path remapping for crate dependencies, but only when they came from Cargo's registry cache, not a vendor directory. This caused builds that used remapped debuginfo and vendor directories to fail with: ``` std::fs::read_dir(registry_src) failed with No such file or directory (os error 2) ``` or (if the `registry/src` directory exists but is empty) ``` error: --remap-path-prefix must contain '=' between FROM and TO ``` Fixes rust-lang#117885 by explicitly supporting the `vendor` directory and adding it to `RUSTC_CARGO_REGISTRY_SRC_TO_REMAP`. Note that `bootstrap.py` already assumes that `./vendor` within the rust repo is the only supported vendoring location. r? `@pietroalbini`

rustbot assigned clubby789 Sep 15, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Sep 15, 2023

remap cargo dependencies to /rust/deps

85c0ce2

pietroalbini force-pushed the pa-remap-cargo-home branch from ae022d3 to 85c0ce2 Compare October 24, 2023 10:24

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 Oct 25, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 26, 2023

bors merged commit 278eaf5 into rust-lang:master Oct 26, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 26, 2023

pietroalbini deleted the pa-remap-cargo-home branch October 26, 2023 08:35

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

jyn514 mentioned this pull request Nov 12, 2023

Distinguish crates with the same name in type errors #117609

Closed

stefson mentioned this pull request Nov 13, 2023

bootstrap is broken in rust-1.75.0-beta with: thread 'main' panicked at src/core/builder.rs:1795:26: std::fs::read_dir(registry_src) failed with No such file or directory (os error 2) #117885

Closed

arlosi mentioned this pull request Jan 4, 2024

bootstrap: handle vendored sources when remapping crate paths #119582

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remap Cargo dependencies to /rust/deps #115872

Remap Cargo dependencies to /rust/deps #115872

pietroalbini commented Sep 15, 2023

rustbot commented Sep 15, 2023

pietroalbini commented Sep 15, 2023

bjorn3 commented Sep 15, 2023

pietroalbini commented Sep 18, 2023

bjorn3 commented Sep 19, 2023

bors commented Oct 14, 2023

pietroalbini commented Oct 24, 2023

clubby789 commented Oct 25, 2023

bors commented Oct 25, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-timer commented Oct 26, 2023

weihanglo commented Jan 2, 2024

Remap Cargo dependencies to /rust/deps #115872

Remap Cargo dependencies to /rust/deps #115872

Conversation

pietroalbini commented Sep 15, 2023

rustbot commented Sep 15, 2023

pietroalbini commented Sep 15, 2023

bjorn3 commented Sep 15, 2023

pietroalbini commented Sep 18, 2023

bjorn3 commented Sep 19, 2023

bors commented Oct 14, 2023

pietroalbini commented Oct 24, 2023

clubby789 commented Oct 25, 2023

bors commented Oct 25, 2023

bors commented Oct 26, 2023

bors commented Oct 26, 2023

rust-timer commented Oct 26, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

weihanglo commented Jan 2, 2024