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

Example of local path crate dependency? #2961

Closed
sthornington opened this issue Oct 24, 2024 · 12 comments
Closed

Example of local path crate dependency? #2961

sthornington opened this issue Oct 24, 2024 · 12 comments
Assignees

Comments

@sthornington
Copy link
Contributor

Does anyone have a working example of a cargo crate dependency in a crate_universe which is referred to by path on disk? Or any way to depend upon a local copy of a crate that you are actively working on?

I am having terrible trouble actively iterating on crates that I depend upon alongside a Bazel workspace that uses them...

@timbess
Copy link

timbess commented Oct 24, 2024

@sthornington I had to symlink external directories into the workspace to get it to work for me.

@sthornington
Copy link
Contributor Author

But they are still crates and lacking BUILD files? I haven't even been able to get this working... cargo-bazel gives some sort of splicing error. What version of rules_rust are you on?

@sthornington
Copy link
Contributor Author

I can't even figure out how to get [patch] nor [replace] sections to work, crate_universe doesn't seem to accept a root [workspace] manifest as valid, since Each manifest should have a root package ...

@sthornington
Copy link
Contributor Author

I narrowed my problem down to this code:

https://github.com/bazelbuild/rules_rust/blob/main/crate_universe/src/splicing/splicer.rs#L617-L621

... it works if I just cut that out and evaluate to false. I'm happy to submit a PR for that change, but it would be helpful to know why that logic is there in the first place ...?

@illicitonion any ideas?

@sthornington
Copy link
Contributor Author

Never mind, that was insufficient, getting Error: The package 'Name("munge-bindings") Version { major: 0, minor: 9, patch: 4 }' has no source info so no annotation can be made now. Investigating ...

@sthornington
Copy link
Contributor Author

I have no idea how this is supposed to work, what would it put into cargo-bazel-lock.json ? The SourceAnnotation only has arms for Http and Git. I did manage to (I think) get [patch] to work but it gives me the same error about "has no source info".

@sthornington
Copy link
Contributor Author

@illicitonion sorry to ping you on this - hypothetically, is getting this to work something I could sponsor you or someone to do? I do a lot of co-iteration on crates and bazel projects which depend on them, so it would be very convenient to be able to change a Cargo.toml crate dependency to refer to a local relative or absolute path to the work-in-progress and have the cargo-bazel/crates_universe stuff work as expected. It could have a big red error message saying the build is currently not hermetic or whatever, but editing a crate, compiling, bumping the version, uploading to a repository, just to test the changes in the bazel build that depends on it, is a chore. Feel free to email me to follow up if you prefer.

@illicitonion
Copy link
Collaborator

Hi @sthornington - certainly happy to discuss how we can get this working :)

There are a few levels of support I can imagine here - can we establish exactly what you're needing?

At the simplest end, I am imagining basically just dropping in a replacement for the repo right at the end of any other work. So we'd do any resolving, generating, and such using the released upstream metadata, and then just replace one of the crates with a local directory. Basically equivalent to --override_repository + a generated BUILD file. The simplest version of this would copy in a snapshot at a point of time, a more fully-featured version would allow live editing.

At the most fully-featured end, I can imagine wanting to factor the locally forked Cargo.toml file in when resolving. This would allow the forked Cargo.toml file to e.g. force extra dependencies to be resolved, change versions that get resolved, etc.

All of these things are possible, but if you're really just looking to support the "one-off edit/debug" use-case with no dependency changes, we can probably get away with a lot less complexity than the "fully treat local crates the same as published ones" feature.

@sthornington
Copy link
Contributor Author

I had imagined that a path-local crate would be treated as it is in cargo - it could pull new dependencies (i.e. if necessary trigger a re-pin), so I think it makes sense for it to be noticed ahead of hashing/splicing. If I were to attempt this myself, it looked at a glance like a new SourceAnnotation or something would be necessary and some sort of replacement for semver and URL to identify changes would be necessary? I only looked for maybe 10 minutes.

If I understand correctly, that is closer to your second proposal...?

@illicitonion
Copy link
Collaborator

Yeah, that sounds reasonable. I'll do a little thinking about how I imagine this looking (and how much work it would be) and get back to you.

@sthornington
Copy link
Contributor Author

Thanks!

@illicitonion illicitonion self-assigned this Nov 24, 2024
@illicitonion
Copy link
Collaborator

Fixed by #3025

There are a few follow-ups coming to fix up a few edge-cases, but this should work now.

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

No branches or pull requests

3 participants