-
Notifications
You must be signed in to change notification settings - Fork 9
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
Support for a workspace that uses Cargo.toml
files as the source of truth
#5
Comments
This plugin does not currently look at any |
Right on, that worked! Thanks for the workaround! |
As far as the original issue, any thoughts on having it respect As I understand it–and as explained in this SO answer–the Rust hierarchy consists of: The Bazel hierarchy consists of: It seems that the mapping between the two is:
More visually, $ tree foo
foo
├── Cargo.toml
└── src
└── lib.rs
1 directory, 2 files And generating something like this: ❯ tree foo
foo
├── Cargo.toml
└── src
├── BUILD.bazel
└── lib.rs
1 directory, 3 files Where load("@rules_rust//rust:defs.bzl", "rust_library")
rust_library(
name = "lib",
srcs = ["lib.rs"],
) But $ tree foo
foo
├── BUILD.bazel
├── Cargo.toml
└── src
└── lib.rs
1 directory, 3 files Where load("@rules_rust//rust:defs.bzl", "rust_library")
rust_library(
name = "lib",
crate_name = "foo",
srcs = ["src/lib.rs"],
) That seems to align better with conventions in both models, at least in my head. Does that seem like a thing you'd want to support in |
Agree with everything you've said. I would like to support cargo-compatible workspaces in some capacity, but there's multiple potential ways to do this and I haven't yet figured out precisely how it should work.
There's some more nuance I'd like to add here:
Now there are multiple ways that one could imagine wanting their project's build to be set up with gazelle_rust:
Currently gazelle_rust does option 5. This is what I use for my personal projects that work with bazel, and also what I am prototyping for use at my company. It might be possible to maintain But what I find most interesting is that using bazel and gazelle, it is easy to make lots of small targets rather than large monolithic ones. In the projects that I have used gazelle for, I typically have targets with a small number of files (like <5), and multiple targets per package. This is what we use for C++ and Python at work, and I think this is definitely the right decision if you have a gazelle plugin that can manage all those targets for you. I haven't yet scaled a rust project to the point where this has felt necessary, but that's why I have a strong preference for making small targets over large ones. For rust, this means largely not making submodules, unless necessary due to dependency cycles. This ends up being entirely incompatible with cargo's project structure, which is why I gave up on cargo compatibility for the rust projects that I build with bazel. That being said, maintaining I think option 2 is the easiest, but it does mean you have to update In practice gazelle_rust would operate in different modes, and we would probably control this with a directive like There's also other things that gazelle_rust should support, like automatically grouping child modules into the parent target. I guess I haven't added support for this because I mostly create separate crates and not modules, but this is required for the pure-bazel to be correct as well. Is there a reason we would want options 3 or 4, or perhaps something else? |
Thank you for the very thorough reply! Without multiple years of experience writing Rust, I would actually lean toward the next option to add support for being option 3. As mentioned, I'm still fairly new at writing Rust. I don't know if my thoughts are colored by my previous experiences, or if they're in line with the bigger picture. So, take them for what they're worth. That said, I think having both the manifest and the sources as inputs for For option 2, the thing that pops to mind is I also don't know enough about Rust yet to know if there are other pieces of information that can only be found within source files. If so, that's another point against option 2. But if not, maybe trying to find a resolution to For option 4, I get the sense that For option 3, it seems to be the option that provides the least amount of friction. If I'm understanding it properly, someone would be able to take most any Cargo project, run through the setup instruction in the README, and have Option 3 feels like it's in line with how Gazelle works with Go: you write Go code like normal, and don't have to think about what's generated in the vast majority of the cases. Granted, Go has a simpler manifest and package structure convention. But, I hope the point is coming across. |
This sounds very similar to what I started doing for a PureScript integration with Bazel a while back. Started rolling it out at a previous job, and it showed potential. Being able to incrementally compile individual modules, and cut-off the build when possible was a huge increase in build efficiency. Having the ability to break down a PureScript package into its constituent modules was great for really narrowing down the actual dependencies you had to think about (and Bazel had to compile). We could put the code in a structure that made sense, instead of being forced to follow the convention of putting all the source files in a Something that I realized at that job (and that is reinforced at my current job) is that most people see Bazel as a means to an end. It would be nice to have everything work through the I think option 5 (the current behavior) is the right way to go. But I also recognize that until Bazel (or something else) eats the world, the other tools are going to need to exist, people are going to interact with them, and I don't have the energy to change minds. |
As far as what to do next, I know I said that option 3 seems like the best, but I'd also be fine with option 2 for now and work around the |
Thanks for the thoughts. There's a lot here and I'll think about this for a bit. I am convinced now that option 3 is the way to go. I think the approach is basically make a
At my job we have tried our dardnest to make this a reality for C++ and Python, to large success. I am experimenting with the same for Rust and I think it's pretty close. |
I've added experimental support for generating from I also added a I think the real test is whether it's usable at scale in a hybrid cargo/bazel repo, but that requires time and investment. |
That all sounds great! Thanks for implementing it. I'll give it a whirl when I get a minute. |
I've been attempting to build an existing rust project of mine with bazel. I had hoped to use gazelle_rust to generate the Most of the crates in my project are open source crates so they need to be buildable by cargo, and ideally I'd like bazel to be building the same thing as cargo. In order for that to be the case I want a single source of truth for both of them. It doesn't seem like that's possible with the current modes? Of the options mentioned towards the start of the issue I think Option 2 would work best for me - I'm already maintaining Option 1 could potentially also work, though it looks like the parser in |
@obmarg I believe the current approach as implemented could still work for you, although there are gaps. In practice the detection of which crates are used in a given file should work quite well, and if it doesn't this would also be an issue when used in a pure-bazel context. I'm happy to fix such issues. But you've listed other gaps that are also issues with the pure-bazel version of gazelle_rust: Platforms-specific cfg attributes - parsing this isn't too difficult (it's similar to the existing handling of test-only dependencies), but gazelle does not currently support merging expressions that contain arbitrary selects. (Limited support is available, but only for golang. My colleague has a proposal that would improve that situation: bazel-contrib/bazel-gazelle#1072). For this reason the best option I have at the moment is tricks like this: https://github.com/Calsign/gazelle_rust#ignoring-dependencies. Feature flags - it's not clear to me how this should work. The current behavior is to effectively assume that all features are enabled for tracking dependencies, but not do anything to enable those features. With rules_rust, the features that are enabled for a crate are determined at the site of the Renamed modules - I assume you are referring to rust-lang/cargo#5653. rules_rust also supports this with the Each of the above items potentially warrant their own issues. I haven't tried maintaining a large rust project that builds with both cargo and bazel using gazelle_rust, so there may be a lot that I'm missing. But my experience with bazel-built projects has been that while these missing features hurt, in practice it is possible to work around them, and once those workarounds are implemented, they do not need to be changed frequently. So it's a matter of one's appetite for managing two parallel build systems. |
I don't think it can. I wasn't trying to suggest that the gazelle_rust approach couldn't be made to build my project. I was saying that I do not want two sources of truth for my builds, and that seems unavoidable with the current approach. No worries if this isn't a use case you want to support - I ended up hacking together something closer to my preferred approach anyway. Just thought I might offer the feedback.
Agreed - some of these are problems in a pure bazel world too. Wasn't trying to suggest you couldn't fix the problems - I just think they could be sidestepped entirely with a different approach. Just my 2c though.
Can't say which would be best here, but I think the active set of features needs to be user controlled somehow. If you're just enabling all the features that exist then why even have the features.
I was actually referring to using the path attribute to rename modules. Probably fairly easy to support that one, but there's other variations of the same problem:
But anyway, agree these are potentially separate issues, don't want to derail this thread too much. |
I'm having some troubles with a Cargo workspace that uses
Cargo.toml
files as the source of truth. Gazelle will generate third-party dependencies fine. But, I can't seem to get Gazelle to automatically generate thedeps
attributes properly for other workspace members.Let's say I have a some
Cargo.toml
s like this:And I import
bar
infoo
:Whenever I run Gazelle, it does not find the dependency for
bar
:I'm not 100% sure I've got the workspace setup properly. As mentioned, Gazelle can find third-party dependencies alright (I use things like
clap
andtracing
in the actual workspace). But, that might be handled differently from first-party dependencies.Looking at the examples/tests, nothing seems to use
Cargo.toml
files as the source of truth withgazelle_rust
. Is it that this use case hasn't been fleshed out yet? If not, any thoughts on what the issue could be?The text was updated successfully, but these errors were encountered: