Skip to content
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

Tracking issue for -Z binary-dep-depinfo #63012

Open
2 tasks
ehuss opened this issue Jul 26, 2019 · 18 comments
Open
2 tasks

Tracking issue for -Z binary-dep-depinfo #63012

ehuss opened this issue Jul 26, 2019 · 18 comments
Labels
A-cli Area: Command line interface to the compiler. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. requires-nightly This issue requires a nightly compiler in some way. S-tracking-design-concerns Status: There are blocking ❌ design concerns. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Jul 26, 2019

This is a tracking issue for -Z binary-dep-depinfo added in #61727.
The cargo side is implemented in rust-lang/cargo#7137.

Blockers:

  • Canonicalized paths on Windows. The dep-info file includes a mix of dos-style and extended-length (\\?\) paths, and I think we want to use only one style (whatever is compatible with make and other tools). See the PR for details.
  • Codegen backends are not tracked.

cc @Mark-Simulacrum @alexcrichton

@ehuss ehuss added B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Jul 26, 2019
@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. requires-nightly This issue requires a nightly compiler in some way. labels Jul 26, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Aug 14, 2019
…xcrichton

Utilize -Zbinary-dep-depinfo in rustbuild

The last commit moves us over to using binary-dep-depinfo while the first two permit us to bootstrap from what will become future beta, to be released in the next week (it's the `cfg(bootstrap)` processing).

We no longer utilize stamp-file mtimes at all inside rustbuild, and a future PR may be able to entirely eliminate them by eagerly copying to the appropriate sysroot. The only mtime-based dependency tracking left is for documentation because we lie to Cargo about the rustdoc binary, so Cargo does not track changes to the real binary, and codegen-backends because binary-dep-depinfo does not emit that information into the depfiles.

Both of these are fixable in the longer term but this existing patch gives us the following benefits:
 * We no longer delete Cargo target directories manually within a stage. Cross-stage, changes to codegen backends will still clear out target directories. This means that incremental state persists across individual steps (e.g., rebuilding libstd does not clear out librustc incremental state). Fixes rust-lang#54712.
 * Dependency tracking across steps within a given stage is now fully precise. We will not clear out all codegen backend dependencies due to changes in librustc_driver, for example, only deleting the final librustc_codegen_llvm crate. Fixes rust-lang#54008, fixes rust-lang#50481.
 * We properly track codegen backends as a dependency (equivalent to rustc) across changes. Fixes rust-lang#53284, and fixes rust-lang#52719.
 * Cross-stage dependency tracking of crates is also much more accurate and reliable. Most likely fixes rust-lang#49979 (but no reproduction steps in that issue). Fixes rust-lang#59105.

cc rust-lang#63012
Centril added a commit to Centril/rust that referenced this issue Aug 15, 2019
…xcrichton

Utilize -Zbinary-dep-depinfo in rustbuild

The last commit moves us over to using binary-dep-depinfo while the first two permit us to bootstrap from what will become future beta, to be released in the next week (it's the `cfg(bootstrap)` processing).

We no longer utilize stamp-file mtimes at all inside rustbuild, and a future PR may be able to entirely eliminate them by eagerly copying to the appropriate sysroot. The only mtime-based dependency tracking left is for documentation because we lie to Cargo about the rustdoc binary, so Cargo does not track changes to the real binary, and codegen-backends because binary-dep-depinfo does not emit that information into the depfiles.

Both of these are fixable in the longer term but this existing patch gives us the following benefits:
 * We no longer delete Cargo target directories manually within a stage. Cross-stage, changes to codegen backends will still clear out target directories. This means that incremental state persists across individual steps (e.g., rebuilding libstd does not clear out librustc incremental state). Fixes rust-lang#54712.
 * Dependency tracking across steps within a given stage is now fully precise. We will not clear out all codegen backend dependencies due to changes in librustc_driver, for example, only deleting the final librustc_codegen_llvm crate. Fixes rust-lang#54008, fixes rust-lang#50481.
 * We properly track codegen backends as a dependency (equivalent to rustc) across changes. Fixes rust-lang#53284, and fixes rust-lang#52719.
 * Cross-stage dependency tracking of crates is also much more accurate and reliable. Most likely fixes rust-lang#49979 (but no reproduction steps in that issue). Fixes rust-lang#59105.

cc rust-lang#63012
Centril added a commit to Centril/rust that referenced this issue Aug 15, 2019
…xcrichton

Utilize -Zbinary-dep-depinfo in rustbuild

The last commit moves us over to using binary-dep-depinfo while the first two permit us to bootstrap from what will become future beta, to be released in the next week (it's the `cfg(bootstrap)` processing).

We no longer utilize stamp-file mtimes at all inside rustbuild, and a future PR may be able to entirely eliminate them by eagerly copying to the appropriate sysroot. The only mtime-based dependency tracking left is for documentation because we lie to Cargo about the rustdoc binary, so Cargo does not track changes to the real binary, and codegen-backends because binary-dep-depinfo does not emit that information into the depfiles.

Both of these are fixable in the longer term but this existing patch gives us the following benefits:
 * We no longer delete Cargo target directories manually within a stage. Cross-stage, changes to codegen backends will still clear out target directories. This means that incremental state persists across individual steps (e.g., rebuilding libstd does not clear out librustc incremental state). Fixes rust-lang#54712.
 * Dependency tracking across steps within a given stage is now fully precise. We will not clear out all codegen backend dependencies due to changes in librustc_driver, for example, only deleting the final librustc_codegen_llvm crate. Fixes rust-lang#54008, fixes rust-lang#50481.
 * We properly track codegen backends as a dependency (equivalent to rustc) across changes. Fixes rust-lang#53284, and fixes rust-lang#52719.
 * Cross-stage dependency tracking of crates is also much more accurate and reliable. Most likely fixes rust-lang#49979 (but no reproduction steps in that issue). Fixes rust-lang#59105.

cc rust-lang#63012
Centril added a commit to Centril/rust that referenced this issue Aug 15, 2019
…xcrichton

Utilize -Zbinary-dep-depinfo in rustbuild

The last commit moves us over to using binary-dep-depinfo while the first two permit us to bootstrap from what will become future beta, to be released in the next week (it's the `cfg(bootstrap)` processing).

We no longer utilize stamp-file mtimes at all inside rustbuild, and a future PR may be able to entirely eliminate them by eagerly copying to the appropriate sysroot. The only mtime-based dependency tracking left is for documentation because we lie to Cargo about the rustdoc binary, so Cargo does not track changes to the real binary, and codegen-backends because binary-dep-depinfo does not emit that information into the depfiles.

Both of these are fixable in the longer term but this existing patch gives us the following benefits:
 * We no longer delete Cargo target directories manually within a stage. Cross-stage, changes to codegen backends will still clear out target directories. This means that incremental state persists across individual steps (e.g., rebuilding libstd does not clear out librustc incremental state). Fixes rust-lang#54712.
 * Dependency tracking across steps within a given stage is now fully precise. We will not clear out all codegen backend dependencies due to changes in librustc_driver, for example, only deleting the final librustc_codegen_llvm crate. Fixes rust-lang#54008, fixes rust-lang#50481.
 * We properly track codegen backends as a dependency (equivalent to rustc) across changes. Fixes rust-lang#53284, and fixes rust-lang#52719.
 * Cross-stage dependency tracking of crates is also much more accurate and reliable. Most likely fixes rust-lang#49979 (but no reproduction steps in that issue). Fixes rust-lang#59105.

cc rust-lang#63012
Centril added a commit to Centril/rust that referenced this issue Aug 15, 2019
…xcrichton

Utilize -Zbinary-dep-depinfo in rustbuild

The last commit moves us over to using binary-dep-depinfo while the first two permit us to bootstrap from what will become future beta, to be released in the next week (it's the `cfg(bootstrap)` processing).

We no longer utilize stamp-file mtimes at all inside rustbuild, and a future PR may be able to entirely eliminate them by eagerly copying to the appropriate sysroot. The only mtime-based dependency tracking left is for documentation because we lie to Cargo about the rustdoc binary, so Cargo does not track changes to the real binary, and codegen-backends because binary-dep-depinfo does not emit that information into the depfiles.

Both of these are fixable in the longer term but this existing patch gives us the following benefits:
 * We no longer delete Cargo target directories manually within a stage. Cross-stage, changes to codegen backends will still clear out target directories. This means that incremental state persists across individual steps (e.g., rebuilding libstd does not clear out librustc incremental state). Fixes rust-lang#54712.
 * Dependency tracking across steps within a given stage is now fully precise. We will not clear out all codegen backend dependencies due to changes in librustc_driver, for example, only deleting the final librustc_codegen_llvm crate. Fixes rust-lang#54008, fixes rust-lang#50481.
 * We properly track codegen backends as a dependency (equivalent to rustc) across changes. Fixes rust-lang#53284, and fixes rust-lang#52719.
 * Cross-stage dependency tracking of crates is also much more accurate and reliable. Most likely fixes rust-lang#49979 (but no reproduction steps in that issue). Fixes rust-lang#59105.

cc rust-lang#63012
Centril added a commit to Centril/rust that referenced this issue Aug 15, 2019
…xcrichton

Utilize -Zbinary-dep-depinfo in rustbuild

The last commit moves us over to using binary-dep-depinfo while the first two permit us to bootstrap from what will become future beta, to be released in the next week (it's the `cfg(bootstrap)` processing).

We no longer utilize stamp-file mtimes at all inside rustbuild, and a future PR may be able to entirely eliminate them by eagerly copying to the appropriate sysroot. The only mtime-based dependency tracking left is for documentation because we lie to Cargo about the rustdoc binary, so Cargo does not track changes to the real binary, and codegen-backends because binary-dep-depinfo does not emit that information into the depfiles.

Both of these are fixable in the longer term but this existing patch gives us the following benefits:
 * We no longer delete Cargo target directories manually within a stage. Cross-stage, changes to codegen backends will still clear out target directories. This means that incremental state persists across individual steps (e.g., rebuilding libstd does not clear out librustc incremental state). Fixes rust-lang#54712.
 * Dependency tracking across steps within a given stage is now fully precise. We will not clear out all codegen backend dependencies due to changes in librustc_driver, for example, only deleting the final librustc_codegen_llvm crate. Fixes rust-lang#54008, fixes rust-lang#50481.
 * We properly track codegen backends as a dependency (equivalent to rustc) across changes. Fixes rust-lang#53284, and fixes rust-lang#52719.
 * Cross-stage dependency tracking of crates is also much more accurate and reliable. Most likely fixes rust-lang#49979 (but no reproduction steps in that issue). Fixes rust-lang#59105.

cc rust-lang#63012
bors added a commit that referenced this issue Aug 16, 2019
Utilize -Zbinary-dep-depinfo in rustbuild

We no longer utilize stamp-file mtimes at all inside rustbuild, and a future PR may be able to entirely eliminate them by eagerly copying to the appropriate sysroot. The only mtime-based dependency tracking left is for documentation because we lie to Cargo about the rustdoc binary, so Cargo does not track changes to the real binary, and codegen-backends because binary-dep-depinfo does not emit that information into the depfiles.

Both of these are fixable in the longer term but this existing patch gives us the following benefits:
 * We no longer delete Cargo target directories manually within a stage. Cross-stage, changes to codegen backends will still clear out target directories. This means that incremental state persists across individual steps (e.g., rebuilding libstd does not clear out librustc incremental state). Fixes #54712.
 * Dependency tracking across steps within a given stage is now fully precise. We will not clear out all codegen backend dependencies due to changes in librustc_driver, for example, only deleting the final librustc_codegen_llvm crate. Fixes #54008, fixes #50481.
 * We properly track codegen backends as a dependency (equivalent to rustc) across changes. Fixes #53284, and fixes #52719.
 * Cross-stage dependency tracking of crates is also much more accurate and reliable. Most likely fixes #49979 (but no reproduction steps in that issue). Fixes #59105.

cc #63012
@Mark-Simulacrum
Copy link
Member

In #68298 we fixed binary dep-depinfo to be less eager to emit dependencies on dylib/rlib files when emitting rlibs and rmeta files, as we only need rmeta input in that case.

It was also noted that we currently do not correctly emit plugin dependencies (I'm not entirely sure of this, but seems not implausible).

@fangism
Copy link

fangism commented Sep 24, 2021

MCP: rust-lang/compiler-team#464

@jonhoo
Copy link
Contributor

jonhoo commented Jun 13, 2022

I wonder if the Cargo side of binary-dep-info could also be taught specifically about the hashes in rustc metadata so that it doesn't just use timestamps for artifacts where a more reliable metric is (I think?) readily available (without going all the way to #75594 / rust-lang/cargo#8623 / rust-lang/cargo#6529).

@jonhoo
Copy link
Contributor

jonhoo commented Jun 13, 2022

Separately, repeating the sentiment from rust-lang/cargo#10664 (comment) that tracking codegen backends should probably not be considered a blocker for landing this — the feature can very much be useful without that I think, and it could be added later on.

@bjorn3
Copy link
Member

bjorn3 commented Jun 13, 2022

I wonder if the Cargo side of binary-dep-info could also be taught specifically about the hashes in rustc metadata so that it doesn't just use timestamps for artifacts where a more reliable metric is (I think?) readily available

  1. That requires some way for cargo to read it directly from the metadata file.
  2. It shouldn't be tied to -Zbinary-dep-depinfo IMO.

@pnkfelix
Copy link
Member

Discussed in T-compiler backlog bonanza

We had some confusion about who the target audience for this feature is. From reading the comments, we understand it was added for certain things in rustbuild. There had been some discussion of making binary-dep-depinfo the default (see e.g. rust-lang/compiler-team#464 ), but we are not clear on whether that is a reasonable thing to do. Likewise, we are not clear on whether upgrading the -Z flag here to a -C flag would be a reasonable thing to do.

Independent of that, there were design questions, e.g. what things should be included. (It doesn't track native dependencies, for example.)

So, I'm adding the design concerns label below, but I would be interested to know if I should also be adding S-tracking-perma-unstable

@rustbot label: S-tracking-design-concerns

@rustbot rustbot added the S-tracking-design-concerns Status: There are blocking ❌ design concerns. label Aug 12, 2022
@pnkfelix
Copy link
Member

Given the confusion we had in our conversation about this, I'm also going to request a summary of this feature and its status. :)

@rustbot label: S-tracking-needs-summary

@rustbot rustbot added the S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. label Aug 12, 2022
@jonhoo
Copy link
Contributor

jonhoo commented Aug 13, 2022

I can speak to the use-case in my case: we're using a large distributed build system at $work in which packages are automatically re-built if their dependencies change. Which, for example, means that if we for example need a patch to rustc to work around some internal issue, then rustc is re-built. And then anything that uses rustc is re-built. But without -Zbinary-dep-depinfo Cargo doesn't understand that rustc has changed since the version number remains the same, and so doesn't realize that it must also re-build the various Rust artifacts.

@est31
Copy link
Member

est31 commented Sep 19, 2022

cargo-udeps uses the feature since the 0.1.33 release to figure out which dependencies were actually used during compilation, since save-analysis is going to be removed.

@est31
Copy link
Member

est31 commented Sep 28, 2022

The linux kernel uses the flag too: Rust-for-Linux/linux#2

Used by: Kbuild.

Status: we could get around it by making the build system more complicated (particularly if the kernel does not upgrade the minimum Make version), but it would be best to avoid that.

@pvdrz
Copy link
Contributor

pvdrz commented Nov 21, 2022

The linux kernel uses the flag too: Rust-for-Linux/linux#2

Used by: Kbuild.
Status: we could get around it by making the build system more complicated (particularly if the kernel does not upgrade the minimum Make version), but it would be best to avoid that.

Apparently it is necessary to avoid recompiling in some scenarios: Rust-for-Linux/linux#2 (comment)

Without -Zbinary-dep-depinfo rustc will only put the source files and the compiled output in the depinfo file. With -Zbinary-dep-depinfo rustc will also add .rmeta, .rlib, ... dependencies to the depinfo file. Without this changing a depensency wouldn't cause make to rebuild dependent crates. Cargo doesn't have this issue as it already knows the dependencies on it's own and only needs the depinfo file for the source file list, but make really needs everything.

@jyn514
Copy link
Member

jyn514 commented May 8, 2023

I can speak to the use-case in my case: we're using a large distributed build system at $work in which packages are automatically re-built if their dependencies change. Which, for example, means that if we for example need a patch to rustc to work around some internal issue, then rustc is re-built. And then anything that uses rustc is re-built. But without -Zbinary-dep-depinfo Cargo doesn't understand that rustc has changed since the version number remains the same, and so doesn't realize that it must also re-build the various Rust artifacts.

I am confused by this statement. binary-dep-depinfo does not emit the path to the toolchain currently: https://github.com/rust-lang/rust/blob/1fa1f5932b487a2ac4105deca26493bb8013a9a6/compiler/rustc_interface/src/passes.rs#L490-L511
Exactly what is your setup? I don't understand why this would be working for you but not for bootstrap.

@jonhoo
Copy link
Contributor

jonhoo commented May 8, 2023

Ah, so, to be clear, we don't currently use -Zbinary-dep-depinfo. Instead, incremental builds currently just break if we ever happen to have to do this. What I wrote was aspirational: we want depinfo tracking in the hopes that it will let Cargo/rustc detect this situation (rustc changing without the version changing) and handle it correctly.

@bjorn3
Copy link
Member

bjorn3 commented May 8, 2023

We do currently use -Zbinary-dep-depinfo for making cargo rebuild everything, however when incremental compilation is enabled, it is not enough to trigger a rebuild. The incr comp cache needs to be cleared too. In addition if a crate version changes, that seems to cause issues too.

@Mark-Simulacrum
Copy link
Member

Cc #111329 (comment) which I believe should help explain why it's not currently enough for dependency version changes, I suspect a similar change for incremental may also be helpful but I haven't looked at incremental encoding/decoding.

@bjorn3
Copy link
Member

bjorn3 commented May 9, 2023

For incremental it wouldn't help. Even if the encoding is exactly identical the encoded query results may have changed between versions. We need to clear the incr cache unconditionally if rustc changes.

@jonhoo
Copy link
Contributor

jonhoo commented May 12, 2023

To loop in some other tickets that have touched on "re-building if rustc changes" in the past: rust-lang/cargo#10664 and rust-lang/cargo#10367 are possibly relevant.

@jyn514
Copy link
Member

jyn514 commented May 12, 2023

rust-lang/cargo#10367 is a different problem. That's about if the path to the compiler changes; the problem in this issue happens when the path and version output stay the same and only the mtine is different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command line interface to the compiler. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. requires-nightly This issue requires a nightly compiler in some way. S-tracking-design-concerns Status: There are blocking ❌ design concerns. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Unstable, no backers
Development

No branches or pull requests