[Crate Universe] Add support for package overrides

[rdelfin]

When incorporating crates using crate_universe, one will every now and then encounter dependencies that can only be resolved by replacing them entirely with a different, bazelified version of a crate. The most common usecase of this are -sys crates, which sometimes generate bindings for a C library in some convoluted ways in their build.rs file.

To enable these kinds of scenarios, cargo raze provides a skipped_deps option. You can see an excerpt from their readme below:

In a few cases, the sys crate may need to be overridden entirely. This can be facilitated by removing and supplementing dependencies in the Cargo.toml, pre-generation:

[package.metadata.raze.crates.sdl2.'0.31.0']
skipped_deps = [
    "sdl2-sys-0.31.0"
]
additional_deps = [
    "@//cargo/overrides/sdl2-sys:sdl2_sys"
]

Unfortunately, this usecase is not currently supported by crate universe, making it impossible to pull in dependencies like sdl2 or ibverbs. This PR contains an implementation of a slightly different form of this feature, where you can provide a dependency override for a given crate. For example, in the case of ibverbs, you might choose to override the dependency as follows:

crates_repository(
[...]
    annotations = {
        [...]
        "ibverbs": [crate.annotation(
            gen_build_script = False,
            dep_overrides = {"ibverbs-sys": "@//overrides/ibverbs:ffi"},
        )],
        [...]
    }
    [...]
)

This will make all ibverbs libraries replace their ibverbs-sys dependency with the target //overrides/ibverbs:ffi (the @ at the front tells Bazel the dependency is in our main workspace, as opposed to the internal crates_repositories one for this dependency).

This resolves #1599

[rdelfin]

I'm getting hyper errors in the tests, but they seem unrelated to my change

[illicitonion]

This generally looks good, thanks for putting it together!

Aside from the one line-comment, I have one slightly broader question:

Should this actually be a per-crate annotation, or should this be a top-level attribute of a crate_universe? I can't think of a context where I'd want some copies of a crate in a build graph but not others to end up being replaced... But I can imagine cases where having two different versions, in a transitive dependency graph you don't completely own/control, leading to very confusing situations...

For prior art, rules_jvm_external has a top-level override_targets attribute which allows you to specify replacement targets for specific maven coords (i.e. crate-names). If we decide it should be a top-level dict, it may be worth following this pattern and naming?

Lastly: Could you add in a test? Probably a unit test in crate_context.rs, and maybe a new folder in examples/crate_universe which uses a new crate_universe which depends on some simple crate like dirs but replaces its dirs-sys dep with a local implementation that always returns a constant home_dir, and a rust_test which calls it and asserts on the output?

[illicitonion]

This feels like it should be an atomic replacement, rather than a remove+insert? In particular, it feels like if the old dep was in a select, so should the new dep be, rather than put into an extra_deps list?

I'd suggest instead adding a replace<T: Clone>(&mut self, old: &T, new: &T) to SelectList which walks all of the relevant BTreeSets doing replacement in each?

[illicitonion]

(Also, sorry for such the long delay in getting this reviewed - thanks a lot for the contribution!)

[rdelfin]

@illicitonion no worries at all!

You make a good point about having them be a top level override. I've put it into the crate partially for simplicity and to mimic how this works on raze, but it would I can't think of a reason you'd only replace the dependency on one crate. I'll do a second version and re-request a review once it's done

[illicitonion]

Amazing, thanks so much!

[rbtcollins]

does this patch need someone to pick it up? @rdelfin - are you still planning on coming back to this?

[rdelfin]

Thanks for the ping @rbtcollins! Yeah, I started working on it because I had been maintaining a patch internally for our repo while this got fixed up, but I haven't had the time to dig into it. I'll work on it now that a recent version broke my rebased patch beyond my ability to fix it, but if you're interested in working on it, feel free to! Happy to help too

[rbtcollins]

If I may suggest, the way I thought about this before reading this PR was that a crate annotation could inject a custom BUILD rule, but still have the crate source http-archive defined, and still participate in other ways in the dependency graph (getting the same original name and so on).

That to me feels like a new option on the annotation of the crate being replaced

[fhilgers]

Does #2674 address the same usecase as this PR?

Allow external crates to be aliased to local targets

In some cases it may be desirable to alias a public crate to a locally
define rust_library target, even for dependencies of crates that are not
being overriden.
Support this use case by swapping out alias for rust_library when
a user overrides the target using an annotation.

[robin-wayve]

I can confirm that #2674 solves this issue for the codebase that @rdelfin originally posted this for 👍🏻