Workspace support

[UebelAndre]

The following pull requests are included in this change set and should be closed first:

• Cleanup #275
• settings.index_url can now be a path #268

This pull requests adds support for cargo workspaces and closes #262

Notable Changes:

1. There is now a new setting workspace_member_dir which is used to determine the location where a package's dependencies will get aliased. Previously, a BUILD file would be rendered into workspace_path which would contain aliases for direct dependencies. Because we now account for there being multiple packages (ie, in a workspace) a directory matching the name specified by workspace_member_dir will be created in each package directory (where a Cargo.toml file lives) containing a BUILD file with the aliases. This is intended to make it easier for users to specify the dependencies they care about as well as maintain the previous behavior (workspace_member_dir should simply match the package name specified by workspace_path to result in no change for existing projects).
2. Binary dependencies are treated as workspace members. This is done by downloading the binary dependency's source into the mock workspace when raze is gathering metadata and modifying the workspace root's Cargo.toml to include a workspace definition that includes the binary dependency. This allows raze to share a single lockfile for all dependencies and ensure all dependencies use compatible versions with each other. (this actually solves a bug I noticed when generating wasm-bindgen-cli for rules_rust. This dependency will update if the file is regenerated and cause a build error). Cargo-raze will now output a Cargo.raze.lock file when binary dependencies are specified. This file is authoritative over subsequent raze runs.
3. Add support for defining Raze Settings via [workspace.metadata.raze]. This will take precedence over package definitions ([package.metadata.raze]) if it's defined. This should provide a consistent central location for settings even in projects that only provide a single root package.

Sorry about the large PR. I did a pass to split out what I could into other pull requests but I found it difficult to do so.

[UebelAndre]

@acmcarther sorry about the large PR. Looking forward to your feedback. I'm really hoping you like some of the changes here 😄

[UebelAndre]

@wchargin do you have any suggestion on how [package.metadata.raze] would work in the context of a workspace? I'm getting warnings output in #277

[UebelAndre]

@wchargin do you have any suggestion on how [package.metadata.raze] would work in the context of a workspace? I'm getting warnings output in #277

Ah, found https://doc.rust-lang.org/cargo/reference/workspaces.html#the-workspacemetadata-table

Sorry to bother 😅

[acmcarther]

I got tired half way through the review (on me, not you, didn't get enough sleep last night), so I'm only half way done. I'll need to continue from about half way through impl/src/metadata.rs on my next review.

Looks good so far. Thank you for your continued work here and throughout the repo!

[UebelAndre]

@acmcarther Hey, I've addressed some stuff here and had questions on a few things but is otherwise ready for another pass 😀

[acmcarther]

This is going to take a very long time to review.

[UebelAndre]

This is going to take a very long time to review.

Why is that? Is there something I can do to help with that?

[UebelAndre]

#276 (comment)
@acmcarther what are you referring to here?

[UebelAndre]

This is going to take a very long time to review.

Why is that? Is there something I can do to help with that?

Oh, is it just that you're busy @acmcarther ? Totally understandable if so.

Also, this PR is ready for another review

[UebelAndre]

@acmcarther Don't know why comments are being hidden (maybe the rebasing?) but I think I've gone over everything now.

[acmcarther]

functions are too long man

too long

[UebelAndre]

#276 (comment)
Ah! I see, I thought you were saying I had duplicate code within the same function.

[acmcarther]

Humble request to stop rebasing the commits unless you have a good reason. It's wrecking the review history.

I try to click https://github.com/google/cargo-raze/pull/276/files/558e3cb7b2259a369880d26642269f376801b73c#diff-c8fe41d4c65c6cc6fc8160d444cb04842ac218c135768f6e4309b18ec27ee784 because you asked me to provide an example, and the link is broken

[UebelAndre]

@acmcarther Updated. Back to you 🙏

[UebelAndre]

I have the sneaking suspicion that this is going to to be a breaking change, but the test suite isn't good enough to catch it, and I'm no longer familiar enough with the code to identify it in review.

Oh well I guess. I think this is the last pass of comments.

This may be a "breaking" change for more obscure projects. Notable the generation of the aliases is controlled by the workspace_member_dir setting and is always relative to the location of Cargo.toml files. That said though, I think with that setting, users should be able to get new generations with these changes to match previous outputs.

[UebelAndre]

Fixed a typo which should fix CI.

[UebelAndre]

#277 is also green with no changes other than a rebase

[UebelAndre]

@acmcarther Hey, are you around to take another quick look? 😅

[UebelAndre]

@acmcarther Removed the whitespace. Ready for another pass 🙏

[acmcarther]

Will wait for green from CI

[UebelAndre]

@acmcarther Just finished 😄

[UebelAndre]

@acmcarther thank you so much for all your support!!! 🙏

[dae]

@UebelAndre would it make sense for workspace_member_dir to default to "." for compatibility? After updating I found I was getting cargo/cargo/BUILD.bazel instead of cargo/BUILD.bazel, and had to dig around to find out how to change it.

[UebelAndre]

@dae I can do that but I originally worried about setting it to "." because I was afraid of overriding valuable build files. Do you feel "." covers the common case?

Also, what do you think of the name workspace_member_dir? It's really just the Bazel package aliases should be rendered into. Is there a better name for it?

[dae]

Would build_file_output_dir work, or does it have other uses I have missed?

[UebelAndre]

@dae Also, is your project public? I'd love to know more about how raze is used. From what I've gathered, there's two kinds of usage:

1. Cargo projects that are being ported to bazel.
2. Bazel projects that want to introduce dependencies on crates.io crates.

I've been focusing on the first case for some time which is partly why workspace_member_dir ended up in the state that it is. I'd love to get to a point where either users don't have to define bogus Cargo.toml files to use this tool. Then for the second point, I'd want to update rules_rust to be able to act as the authority and interract with crate registries from within Bazel entirely. No Cargo.toml files required. Users should be able to author their targets to define all the dependencies they require and the rules should efficiently handle the rest. I think there's a very real path forward to that goal with this tool and maybe some updates to rules_gazelle.

Anywho, I'd love to see your project structure and to know which camp you fall into (camp 1 or camp 2) to give me better context for future changes.

[UebelAndre]

Would build_file_output_dir work, or does it have other uses I have missed?

I feel like that's similar to workspace_member_dir in that it doesn't quite communicate the intent. I was thinking something like package_aliases_dir. Your thoughts there?

[dae]

aliases_build_file_dir? top_build_file_dir?

I run an open source project that was recently converted to Bazel, though it does not currently use workspaces: https://github.com/ankitects/anki

[dae]

(the point being that when the BUILD file moved, I was hunting for something related to the build file that controlled where it was placed)

[UebelAndre]

(the point being that when the BUILD file moved, I was hunting for something related to the build file that controlled where it was placed)

Would I be correct in saying your primary frustration was in the lack of documentation? This I can understand and I'm going to be writing documentation ASAP if that's the case or not. I've created #306 to start addressing this feedback. We should continue the conversation there.

Also, thanks for showing me your project! 🙏