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

Feature Request: Support Cargo Workspaces #262

Closed
UebelAndre opened this issue Oct 18, 2020 · 14 comments · Fixed by #276
Closed

Feature Request: Support Cargo Workspaces #262

UebelAndre opened this issue Oct 18, 2020 · 14 comments · Fixed by #276

Comments

@UebelAndre
Copy link
Collaborator

I decided to make a top level issue to track all information about cargo workspaces. This tool should be updated to be able to generate Bazel files for projects that make use of cargo workspaces.

@UebelAndre
Copy link
Collaborator Author

If anyone was interested in implementing this I'd be forever grateful!!

@acmcarther
Copy link
Member

Do you have opinions about the desired behavior? Do you think we should represent the workspaces somehow in the generated output, or just generate "the minimal thing that works"?

@UebelAndre
Copy link
Collaborator Author

I would be happy with "the minimal thing that works". I do think the best approach for the time being would be to create aliases in the appropriate subpackages that contain aliases to the dependencies, but outside of that, I don't think it should matter how the dependencies are rendered.

My thought is that the outputs would look something like the following:

├── BUILD.bazel
├── Cargo.lock
├── Cargo.toml
├── WORKSPACE.bazel
├── cargo
│   ├── BUILD.bazel
│   └── remote
│       └── BUILD.deps.bazel
├── lib_a
│   ├── BUILD.bazel
│   ├── Cargo.toml
│   ├── cargo
│   │   └── BUILD.bazel
│   └── src
│       └── lib.rs
└── lib_b
    ├── BUILD.bazel
    ├── Cargo.toml
    ├── cargo
    │   └── BUILD.bazel
    └── src
        └── lib.rs

where ./cargo is where all build files are generated and the directories ./lib_a/cargo and ./lib_b/cargo` would have generated aliases for their correct dependencies.

That said though, I do feel at some point raze should interact with bazel-gazelle at some point such that users no longer have to manage dependencies in two places (their BUILD files and the Cargo.toml files). Without giving it too much though, I feel the source of truth should remain the Cargo.toml files and I like this because it makes it easier for existing Rust projects to become integrated with Bazel (though there may be better solutions for this).

So long as I can run cargo-raze on a project that's using cargo workspaces and have all my dependencies correctly generated and a way to relatively easily hook them up, I'll be thrilled.

@UebelAndre
Copy link
Collaborator Author

I would say if I got to pick the next feature cargo-raze got, it would definitely be this one.

@nikclayton-dfinity
Copy link

nikclayton-dfinity commented Oct 21, 2020

FWIW, I've been trying another approach with a project that uses workspaces.

Orginally, the directory tree looked like (just the Rust parts)

├── rs
│   ├── Cargo.lock               # <-- all dependencies      
│   ├── Cargo.toml               # <-- defines the workspace      
│   ├── a_crate
│   │   └── Cargo.toml
│   │   └── src
│   │       └── ...
│   ├── another_crate
│   │   └── Cargo.toml
│   │   └── src
│   │       └── ...
│   ├── yet_another_crate
│   │   └── Cargo.toml
│   │   └── src
│   │       └── ...
│

Now it looks like this:

├── WORKSPACE
├── cargo                        # Should probably be third_party/cargo
│   └── BUILD.bazel              # <-- generated by "cargo raze"      
│   └── crates.bzl               # <-- generated by "cargo raze"      
│   └── Cargo.toml               # <-- lists all crates, overrides, etc      
│   └── remote                   # <-- generated by "cargo raze"      
│       └── ...
├─ rs
│   ├── a_crate
│   │   └── BUILD
│   │   └── Cargo.toml
│   │   └── src
│   │       └── ...
│   ├── another_crate
│   │   └── BUILD
│   │   └── Cargo.toml
│   │   └── src
│   │       └── ...
│   ├── yet_another_crate
│   │   └── BUILD
│   │   └── Cargo.toml
│   │   └── src
│   │       └── ...
│

When I want to add a new dependency I edit cargo/Cargo.toml, and run cargo raze in cargo/ to regenerate everything.

So cargo/Cargo.toml file serves the purpose of the Cargo.lock file in a workspace.

Then in each crate's BUILD file I reference dependencies as either //rs/another_crate:something or //cargo:something_else.

This ensures that everything is built with the same version of all the dependencies.

It's still tedious having both BUILD files and Cargo.toml files in the individual crate directories. I think the right thing to do is have something that can generate the Cargo.toml file from the BUILD file, so that the Bazel build information remains the single source of truth, and so that any Rust tooling that expects a Cargo.toml, cargo metadata, etc, can still work.

@UebelAndre
Copy link
Collaborator Author

I've used this approach as well but I don't quite feel like that satisfies the thing I'm looking for. I don't want to have to write invalid (or maybe "abnormal" would be a better word?) Cargo.toml files to use cargo-raze. It's important to me that existing and valid project configurations can be updated to use Bazel by adding the appropriate [raze] sections to one or a few files.

I'm not super particular on whether or not the Cargo.toml files or BUILD.bazel files are the source of truth. I do agree there should be a single source though. I do feel there's a larger benefit it allowing Cargo.toml to be the source of truth as it would make integrating Bazel into other projects easier (since you don't actually have to translate anything). I would even go so far as to say cargo-raze should gain the ability to generate the targets within your project. So instead of adding a dependency, running raze, then adding that dependency to the target that is supposed to use it, raze would parse the appropriate BUILD file and add the dependency. Though this is definitely out of scope for this request. I just want to be able to run raze on projects using Cargo workspaces and have the same experience of maintaining the dependencies in Bazel as I do currently for projects not using Cargo workspaces.

@UebelAndre
Copy link
Collaborator Author

I think is was able to get this working over the weekend. There's a couple of issues to iron out then I'll start opening pull requests 😀

@TheButlah
Copy link

TheButlah commented Dec 8, 2020

Is there documentation on how to use the new changes in #276? Just came back to this after the dust settled, but I'm a bit lost on what to do to get things working.

@TheButlah
Copy link

TheButlah commented Dec 8, 2020

I spoke too soon - there is already an example available. See #277. Thanks for the hard work!

@UebelAndre
Copy link
Collaborator Author

The changes haven't been released yet. As for docs, I've updated them in #302 That PR addresses some usability issues with the workspace support which you can read up there.

As far as using workspace support, in your top level Cargo.toml where [workspace] is defined, add a [workspace.metadata.raze] section for your raze settings. That's where all settings will go including crate settings (until #302 is merged, then crate settings can go in the package that defines it as a dependency).

@UebelAndre
Copy link
Collaborator Author

Also, be careful, there are some things scheduled to be updated in #306 so some variables and defaults will change.

@TheButlah
Copy link

TheButlah commented Dec 8, 2020

Hmm, yeah I just saw #306 and #302. I'll probably hold off on adopting this for another few weeks then to avoid churn, or till things get released. Thanks for the heads up!

@UebelAndre
Copy link
Collaborator Author

#304 is the issue I opened to track a new release that includes these changes. You can subscribe to that to find out when everything is done and the new release is available 😄

@UebelAndre
Copy link
Collaborator Author

This is now released in version 0.8.0 as per #304

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 a pull request may close this issue.

4 participants