-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
860979e
to
74d596f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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 |
1ab2d47
to
c471f79
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
678e288
to
0a6146b
Compare
43ad473
to
99c5876
Compare
This PR modifies cc @jieyouxu |
r? @bjorn3 (as you're likely a better reviewer for the design) |
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: