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

Aspect to generate Cargo.toml files for tooling support #71

Open
mfarrugi opened this issue Feb 14, 2018 · 12 comments
Open

Aspect to generate Cargo.toml files for tooling support #71

mfarrugi opened this issue Feb 14, 2018 · 12 comments

Comments

@mfarrugi
Copy link
Collaborator

mfarrugi commented Feb 14, 2018

Almost all tooling in the rust ecosystem depends on at least Cargo.toml files and cargo metadata. It would be good to generate the manifests.

https://github.com/mfarrugi/rules_rust/tree/marco-generate-cargo-toml/aspect is a WIP that uses an aspect to generate the information, and then a shell script to move it to the right places. I'm not confident that is the correct way to work with aspects, if anyone has thoughts I would be glad to hear them.

A little bit of the tangent is the aspect uses some of the utilities in toolchain.bzl, it would be good to separate those into another file, are there any qualms with that? And then there is bike-shedding on where the aspect/shell-script should go directory-structure-wise.

Edit:
Updated wip to https://github.com/mfarrugi/rules_rust/blob/marco-generate-cargo-toml/rust/aspect/generate_cargo_workspace.sh

@damienmg
Copy link
Collaborator

damienmg commented Jun 9, 2018

As stated in #95 that would also be a way to open to RLS support so nice integration in IDEs.

A few comment on the prototype:

  1. I have not gotten any dependencies when I tried it.
  2. It moves cargo.toml in my workspace as an external process of bazel which make it unsuitable for use inside bazel-watcher.

For 1. the corner-cases to support, I have:

  • Generated sources files (protobuf/gRPC)
  • External crates
  • Multiple target per package (for now only a lib and a binary)

For 2. I suggest to use include and path attribute of Cargo.toml so there is no need to have the cargo file in the same directory tree (it understands relative path with ..). The root Cargo.toml should probably behave all the time as if it is in the root directory, so the only thing I have to do would be symlinking that file in my workspace (and then I can do bazel-watcher build //... --aspects @io_bazel_rules_rust//aspect:cargo_manifest.bzl%cargo_manifest_aspect --output_groups=all_files, that would keep my Cargo files consitent). Maybe make the kind of files (relative to the source dir or relative to the bazel output tree) something that is configurable so people can either get file to vendor or file that stays in the output tree?

@damienmg
Copy link
Collaborator

damienmg commented Jun 9, 2018

So the dependencies issue is indeed because the only direct dependencies was on a rust_grpc_library, if I add other dependencies they get added but the grpc crates does not.

I would rely on the version attribute for the version number in the deps now that it is supported in rules_rust.

@mfarrugi
Copy link
Collaborator Author

I took another stab at it here.

I didn't realize cargo supported a workspace setup like

├── Cargo.toml
├── example
│  ├── example.rs
│  └── libexample.rs
└── modules
   └── example
      ├── _example
      │  └── Cargo.toml
      └── _libexample
         └── Cargo.toml

but that made this a lot simpler, and should cover most of the edge cases (though I haven't tested the gensrc case).
Each target can generate its own Cargo.toml, then we can point to them from the workspace root.

I'm not sure how to generate the workspace Cargo.toml in a way that is ammenable to bazel-watcher usage, but this is a bit closer to done. Any specific ideas on this, @damienmg?

I also ran into this error w/ multiple versions, which is a blocker to this being useful..

 error: two packages named `slab` in this workspace:
- /data/code/cargo-raze-example-stdx/bazel-bin/cargo/vendor/slab-0.3.0/_slab/Cargo.toml
- /data/code/cargo-raze-example-stdx/bazel-bin/cargo/vendor/slab-0.4.0/_slab/Cargo.toml

Looks like a cargo workspace requires 1 version of each crate, and dealing with multiple versions will require some additional finagling.

@mfarrugi
Copy link
Collaborator Author

mfarrugi commented Aug 2, 2018

I ran into cbindgen and more tools depending on Cargo.tomls, so thought it might be reasonable to generate them as part of the rust_library/binary/test rules. This would also help make generated code traversible.

@damienmg @acmcarther any thoughts on doing that?

@acmcarther
Copy link
Collaborator

You mean generate them in tree? Or just in genfiles?

What about the objective? Do you want just to express the dependency tree via tomls, or is having the ensemble of crates be compilable a goal?


I'm of the opinion that generating Cargo.tomls isn't possible in the general case, as Bazel's feature set is basically a superset of Cargo's feature set. That is to say that you don't have to look very far to find situations that can't be expressed via Cargo.tomls. Perhaps a means of siloing these parts of a source tree away (via hacky workarounds) could still prove useful.

Part of the problem here as I see it (or conversely, the biggest fix that would move the needle) would be Cargo workspaces supporting arbitrary nesting with arbitrarily versioned crates. If that functionality can be added, I'd be a lot more confident about an effort to generate synthetic tomls in all but the trickiest code generation situations.

Regardless of all of that, if you put together PRs I'd be happy to review them and help work through design. At some point RLS needs to be made to work with Bazel and generating synthetic tomls is a fair enough first approach.

@mfarrugi
Copy link
Collaborator Author

mfarrugi commented Aug 2, 2018 via email

@dayfine
Copy link
Contributor

dayfine commented Dec 19, 2019

So, any updates on this? I am looking to manually creating these Cargo.toml now to make the tooling understand where my extern crate comes from, but I am not sure, say:

I have a rust_proto_library target, which is depended by another rust_library target. So the rust package for the rust_library would have a Cargo.toml with the rust_proto_library as dependency. I can use the path property in the Cargo.toml file, but what path should I put then? bazel-bin/...? I hope not ....

@mfarrugi
Copy link
Collaborator Author

mfarrugi commented Dec 19, 2019 via email

@dayfine
Copy link
Contributor

dayfine commented Dec 20, 2019

Interesting. I wonder if I need to generate a Cargo.toml for the generated proto lib so it can be recognized by the code depending on the proto lib.

Anyway, FYI I noticed Fuchsia has a tooling fargo for doing exactly this, although their toolchain is different (not Bazel but with a similar rustc_library target system). I wonder it that could be integrated here or at least used as an inspiration.

https://fuchsia.dev/fuchsia-src/development/languages/rust

@epigramengineer
Copy link

I attempted to get "updated wip: https://github.com/mfarrugi/rules_rust/blob/marco-generate-cargo-toml/rust/aspect/generate_cargo_workspace.sh" to work as a rule within my own repo, but am so far unsuccessful.

I'm getting the error (rule 'rust_binary') doesn't have provider 'crate_root' for on a certain dependency in my //3rdparty/cargo/vendor directory; which doesn't have a rust_binary declared in the build file.

Should I abandon getting this to work? Has development on this and #95 effectively stopped? As far as I can tell the "recommended" approach is to just manually keep a Cargo.toml up to date with my external and generated (proto) dependencies?

@damienmg
Copy link
Collaborator

damienmg commented Sep 7, 2020

@epigramengineer : yes I think those dev are stopped. I never got around to work on this and i don't have active project that use Bazel and Rust, so the needs for this for myself is pretty low.

@UebelAndre
Copy link
Collaborator

Just FYI, I needed to implement something like this for a cbindgen rule I was working on in #392. If anyone needs an aspect that generates a Cargo.toml file you can check that PR out as a starting point in addition to the implementation @mfarrugi did.

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

No branches or pull requests

6 participants