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

crate_universe rule #598

Merged
merged 7 commits into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .bazelignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
docs
examples
examples/crate_universe
examples/cargo_manifest_dir/external_crate
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# bazel
/bazel-*
/examples/bazel-*
/examples/*/bazel-*
/examples/crate_universe/*/bazel-*
/docs/bazel-*

# rustfmt
Expand Down
40 changes: 40 additions & 0 deletions cargo/crate_universe_resolver/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Contributing

## Tour of the codebase

We start at `workspace.bzl`, which invokes the resolver.

The resolver:
* `config.rs`: Deserializes the Config
* `parser.rs`: Parses the Config, any `Cargo.toml` files, and any additional packages, into a single unified `Cargo.toml` file
* `resolver.rs`: Resolves all of the crates.
* `consolidator.rs`: Patches in any WORKSPACE-specified overrides, and deals with platform-specific concerns.
* `renderer.rs`: Generates BUILD files for each crate, as well as functions that can be called from BUILD files.

The code started off as a very hacky Week of Code project, and there are some clear remnants of that in the codebase - nothing is sacred, feel free to improve anything!

Some areas have unit testing, there are a few (very brittle) integration tests, and some examples.

## How to test local changes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this all be put in a script?

I made https://github.com/google/cargo-raze/blob/master/tools/bootstrap.sh and it works quite nicely IMO. I'd imagine you could do something similar where you write a small shell script to build the resolver using cargo and use an environment variable to to toggle whether or not a bootstrapped binary is used. Having to specify the path I think is unnecessarily cumbersome and would like to tighten the iteration loop for developers working on this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will depend on how we decide to approach bootstrapping, so I'm hesitant to automate things before we know what they'll look like

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few options I can immediately think of for solving this

  1. Commit pre-built binaries to the repo
    This is my favorite option because it means if a user has already downloaded rules_rust then they've already got all the requirements to fetch dependencies. This would also likely setup a nice developer workflow for recompiling the binary for their host and replacing it for testing. I think there may be something we can do with Github Actions to build the binaries and commit them back to the repo, I can look into that if there's interest.

  2. Build artifacts that are pinned in the repository and downloaded
    I think this is the expected solution, it requires something to be hosting (and likely building) the files which is probably quite doable with the current CI but would also probably mean a release cadence is adopted.

  3. Setup some boostrapping mechanism that builds the target on the host before it's needed... somehow
    This would likely require some magic with tools/bazel which sounds not ideal (awful, in-fact), but might be a path forward.

Copy link
Collaborator

@UebelAndre UebelAndre Mar 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having dug into this a bit more, I feel option 1 is likely the most effective thing to do. rules_python has done something similar with .pem files (@rules_python//tools:piptool.par, @rules_python//tools:whltool.par) and have added notes to their pull requests and contributing documentation with some guidelines for testing this. Perhaps we could write a rule to test, byte-for-byte that a compiled version of the binaries that are checked in matches what would otherwise get built? There may also be a way to have a github action check for changes to the resolver and create a unique commit and merge it back. Though this then raises the question of whether or not the rules support cross compilation. If not then a github action might be the appropriate course of action.

Would love to also get thoughts from @hlopko

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry this took me this long.

I don't have any great ideas how to solve bootstrapping :( If I had to get inspiration from the ecosystem, I'd look at and talk with folks at https://github.com/bazelbuild/bazel-gazelle.


To use a local version, first build it:
```console
resolver $ cargo build
```

Then run a bazel build with this environment variable:
```console
RULES_RUST_RESOLVER_URL_OVERRIDE=file:///path/to/resolver/target/debug/resolver bazel build //whatever
```

To get verbose logging, edit `workspace.bzl` to set `RUST_LOG` to `debug` or `trace` instead of `info`. In particular, that will print out the generated Cargo.toml, and the path to the generated workspace file.

Note that you may need to `bazel shutdown` between bazel commands. Hopefully not, but if you're seeing stale results, try it out. This only affects local development.

## Testing

To test with the `resolver` built from source:

```bash
bin/test
```
Loading