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

Pass a hash of the modification date for cache control #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JustusAdam
Copy link
Contributor

@JustusAdam JustusAdam commented Jul 31, 2023

This passes an additional --exec-hash argument to rustc which contains a hash of the modification dates for both the cli and the driver application.

This invalidates the build cache when the plugin is recompiled.

Caveats: The argument is passed via (encoded) rustflags, which means we can't use CARGO_WORKSPACE_WRAPPER anymore, because we need the driver to filter out the --exec-hash argument before it hits regular rustc.

I'm happy to discuss alternate namings of this argument or alternative means of delivering it but from my experimentation it will only work if we pass it as a rustflags argument or it will not contribute to the cache key cargo/rustc uses.

@willcrichton
Copy link
Contributor

This seems good. Can you add a test to rustc_plugin/tests/test_example.rs? You could install the example plugin, run it, touch a source file in the example plugin, and then run the plugin again and ensure that the output isn't cached.

@JustusAdam
Copy link
Contributor Author

I've also thought about adding this as a feature actually, where you can pass some sort of force parameter to the plugin and it'll execute a touch on the wrapper binary before calculating the new hash.

I will make that a separate PR though

@@ -152,7 +152,7 @@ fn place_components_conflict<'tcx>(
// are disjoint
//
// Our invariant is, that at each step of the iteration:
// - If we didn't run out of access to match, our borrow and access are comparable
// - If we didn't run out of access to match, our borrow and access are comparalegal_flow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your find/replace was too aggressive :P

cmd
.env("RUSTC_WORKSPACE_WRAPPER", path)
.env("RUSTC_WRAPPER", path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a brief comment that explains why we have to use RUSTC_WRAPPER instead of RUSTC_WORKSPACE_WRAPPER? Just for posterity. Or you can point to this PR.

.unwrap();

orig_args.remove(hash_check_arg);
orig_args.remove(hash_check_arg);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to .remove twice?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it's because you're deleting both arguments. Maybe orig_args.drain(hash_check_arg .. hash_check_arg + 2) is clearer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants