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

Ignore path deps in metadata hash #2687

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

Conversation

illicitonion
Copy link
Collaborator

@illicitonion illicitonion commented Jun 11, 2024

They don't end up factoring into what we generate, so we pretend they don't exist. This avoids people who are using both Cargo and Bazel needing to spuriously repin just because they edited Cargo-only dependency information.

The first few commits are #2688

@illicitonion illicitonion requested a review from UebelAndre June 11, 2024 14:38
@illicitonion illicitonion force-pushed the ignore-path-deps-for-hash branch 3 times, most recently from 08c9ea3 to fd76f9a Compare June 11, 2024 17:31
Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Are path dependencies not desired?

If this support was implemented, would there be anything else being filtered out?

@illicitonion
Copy link
Collaborator Author

Are path dependencies not desired?

I'm actually not sure what this would reasonably look like. It feels like maybe they are via #2674 and maybe we can automate setting some overrides up for path dependencies, but it's not super clear to me what target we can practically implicitly add a dependency on from a generated file into the local workspace. If we were to implement that kind of support, we would want to factor the replacement target, rather than the path dep value, into the cache key.

If this support was implemented, would there be anything else being filtered out?

Yeah, we should probably be filtering out everything that doesn't factor into dep resolution. So we should be stripping out things like authors, description, documentation, readme, homepage, ... - all of these things will currently require a repin if they change, even though they're just metadata for Cargo which Bazel will completely ignore.

They don't end up factoring into what we generate, so we pretend they don't exist.
This avoids people who are using both Cargo and Bazel needing to spuriously repin just because they edited Cargo-only dependency information.
@illicitonion illicitonion force-pushed the ignore-path-deps-for-hash branch from fd76f9a to d94bc1e Compare June 13, 2024 16:27
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