Skip to content

RDR: avoid rebuilding dependent crates after comment changes #143249

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yaahc
Copy link
Member

@yaahc yaahc commented Jun 30, 2025

an initial step of Relink don't Rebuild

The goal is to avoid needing to rebuild downstream crates just because an upstream crate changed a comment.

Our current plan is to treat a hash of the rmeta as the indicator that "something that would require a rebuild has changed" and add an unstable compiler flag to start moving things that are "relink independent" (I don't know what to call this, "relink compatible" maybe? "relink safe"? /shrug) into a separate file alongside the rmeta.

In this PR I've taken the baby step of simply removing the data that is causing rebuilds from the rmeta (source map and span information). This has a significant detrimental impact on diagnostics and causes code that depends on libraries that depend on span information to disambiguate bindings to fail to compile (libraries that depend on thiserror such as sqlx-core for example).

This is fine because we don't necessarily care that this initial flag can build all code, as long as it can be used for basic cargo integration and a simple proof of concept cargo project with multiple dependencies where we can enable the flag, edit a comment, and show downstream crates not re-compiling. Once we have that cargo integration we should be able to work on the real impl that moves the data somewhere else rather than simply deleting it and use that with the already setup cargo integration to realize the same gains as the proof of concept against real world codebases.

This is broken down into two commits as follows:

  1. Tests for current behavior and the impact of editing comments on the hash of the output rmeta file
  2. The changes to remove sourcemap and span information alongside an updated version of the test to enable the associated flag and verify that the hashes no longer change.

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 30, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member

bjorn3 commented Jul 1, 2025

The crate metadata contains a serialized version of the source map, which in turn contains hashes of all source files. In addition the crate hash also has all source files as one of the inputs.

@yaahc yaahc force-pushed the test-lib-reuse branch from 643a476 to 884e375 Compare July 3, 2025 00:07
@rust-log-analyzer

This comment has been minimized.

@yaahc yaahc force-pushed the test-lib-reuse branch 2 times, most recently from 860979e to 74d596f Compare July 3, 2025 00:23
@rust-log-analyzer

This comment has been minimized.

@yaahc yaahc force-pushed the test-lib-reuse branch from 74d596f to 289cbc4 Compare July 3, 2025 00:34
@rust-log-analyzer

This comment has been minimized.

@yaahc

This comment was marked as outdated.

@yaahc

This comment was marked as outdated.

@yaahc
Copy link
Member Author

yaahc commented Jul 3, 2025

based on a comment from bjorn in https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/RDR.3A.20insensitivity.20to.20comment.20changes/with/527063126 I'm gonna go ahead and start with not even emitting a second file and just killing the sourcemap when the rdr flag is enabled

@yaahc yaahc force-pushed the test-lib-reuse branch 2 times, most recently from 1ab2d47 to c471f79 Compare July 3, 2025 19:45
@rust-log-analyzer

This comment has been minimized.

@yaahc yaahc force-pushed the test-lib-reuse branch from c471f79 to 9e3d79b Compare July 3, 2025 20:33
@rust-log-analyzer

This comment has been minimized.

@yaahc yaahc force-pushed the test-lib-reuse branch 3 times, most recently from 678e288 to 0a6146b Compare July 3, 2025 23:20
@yaahc yaahc force-pushed the test-lib-reuse branch 3 times, most recently from 43ad473 to 99c5876 Compare July 9, 2025 22:55
@yaahc yaahc marked this pull request as ready for review July 9, 2025 22:57
@rustbot
Copy link
Collaborator

rustbot commented Jul 9, 2025

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 rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 9, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 9, 2025

This PR modifies run-make tests.

cc @jieyouxu

@yaahc yaahc force-pushed the test-lib-reuse branch from 99c5876 to 086f6ac Compare July 9, 2025 22:57
@jieyouxu
Copy link
Member

r? @bjorn3 (as you're likely a better reviewer for the design)

@rustbot rustbot assigned bjorn3 and unassigned jieyouxu Jul 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants