Support renaming dependencies in rust_library and rust_binary

[acmcarther]

With the "2018" edition, Rust library and binary root source files no longer include extern crate statements. This removes the original mechanism by which an external crate dependency can be renamed.

Cargo has implemented dependency renaming. This feature first appeared in this commit:
rust-lang/cargo@79942fe. In the most recent implementation (as of time of writing) this field is renamed multiple times before entering the link_to function where linking actually happens.

• RegistryDependency.package -> Dependency.explicit_name_in_toml
• Dependency.explicit_name_in_toml -> BuildContext.extern_crate_name

I suspect our only real option here will be to add a param "rename_deps" (bikeshed welcome) that maps from string -> string:

rust_library(
  name = "foo",
  deps = [
    "//bar",
  ]
  rename_deps = { "bar": "baz" }
)

---

EDIT: 2018-12-11 ~11:00PST:

"rename_deps" is not equivalent to the Cargo implementation. Cargo implements this via a field on the dependency itself, meaning that it is theoretically possible to use this feature to include two "foo" crates which are independently renamed to something else. I have no idea how we'd replicate this while following the rust_library.deps convention...

[mfarrugi]

Worth mentioning that bazel users can still include extern crate x as y if they want, and this is more about cargo parity / raze support.

[acmcarther]

I actually wasn't aware of this. I've done some digging to figure out what the actual policy is, but my tl;dr is to punt on this unless a new RFC suggests deprecation of extern crate.

---

The RFC for the feature is available here: https://github.com/rust-lang/rfcs/blob/master/text/2126-path-clarity.md. It has some inconclusive verbiage around deprecation of the old syntax

In previous discussions about deprecating extern crate, there were concerns about the impact on non-Cargo tooling, and in overall explicitness. This RFC fully addresses both concerns by leveraging the new, unambiguous nature of fully qualified paths.

... but the most definitive statement of effect is just this

We will provide a high-fidelity rustfix tool that makes changes to the a crate such that the lints proposed in this RFC would not fire. In particular, the tool will introduce crate:: prefixes, downgrade from pub to crate where appropriate, and remove extern crate. It must be sound (i.e. keep the meaning of code intact and keep it compiling) but may not be complete (i.e. you may still get some deprecation warnings after running it).

Such a tool should be working at with very high coverage before we consider changing any of the lints to warn-by-default.

Indicating that the strongest "deprecation" prescribed by this RFC is just a warn-by-default lint. I guess we need to keep an eye out for anything stronger in the future

From the reference, we only have this

Beginning in the 2018 edition, use declarations can reference crates in the extern prelude, so it is considered unidiomatic to use extern crate.

[acmcarther]

Ok well, I now have an addendum.

Out of an abundance of caution, I dropped into mozilla/#rust to see if anyone knew of an explicit initiative to deprecate. Lo and behold: rust-lang/rust#53128

Choice quotes (emphasis mine):

This is a sub-tracking issue for the RFC "Clarify and streamline paths and visibility" (rust-lang/rfcs#2126)
dealing with the deprecation of extern crate as well as having use crate_name::foo::bar and crate_name::foo::bar Just Work™.

I feel that the deprecation of extern crate and the automatic inclusion of external crates in the prelude has gotten enough exposure and positive feedback that it's ready for us to commit to as the path we're going down. We shouldn't stabilize it until we stabilize the macro system changes, but we can handle that via a blocking concern; in the meantime, let's confirm that we have consensus.

Somehow between the actual paths rfc and that issue the idea of "deprecating extern crate" became the understanding. I don't think we can punt on this for an extended duration for that reason.

[mfarrugi]

What is the use case for aliasing crates? I have never seen this, though I've seen it as a convenience in Python numpy)pandas code. I am happy to punt until there's an issue.

…

On Tue, Dec 11, 2018, 13:38 Alex McArther ***@***.***> wrote: Ok well, I now have an addendum. Out of an abudance of caution, I dropped into mozilla/#rust to see if anyone knew of an explicit initiative to deprecate. Lo and behold: rust-lang/rust#53128 <rust-lang/rust#53128> Somehow between the actual paths rfc and that issue the idea of "deprecating extern crate" became the understanding. I don't think we can punt on this for an extended duration for that reason. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#171 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACLjeBOxPc2JOJxaHelzyedE63xhhP14ks5u3_ubgaJpZM4ZMelz> .

[acmcarther]

I only have toy scenarios for this, one of which doesn't even benefit from the feature in hindsight. Anyway, here they are:

Consider a decentralized monorepo. In that situation there couldn't be a single "arbiter" of name uniqueness across the repo -- sub-teams should be empowered to name their crates in a manner that best matches the utility of the crate.

Scenario 1: Consider some cases where there are two net_util crates, or two templates crates, or two all_protos crates in the same monorepo. This may work fine for however long those teams work in a silo, but inevitably there will be a use case that somehow needs to include both crates. It should be noted that renaming (per this issue) alone isn't sufficient or even useful to accomplish this -- it's likely such a team would write a rust_rename rule that generates a source file that just contains pub use crate_name:* and they would depend on that. Anyway, worth noting here.

Scenario 2: Consider a situation where, for whatever crazy reason, Crate util is part of a hierarchy of crates where another crate util exists somewhere downstream. Now, consider that the parent util crate may need to import the other util crate if it needs the types from that crate or whatever. I'm unsure if rust lets a crate foo depend on another crate foo, but I'd discourage the practice regardless and instead suggest that the dependency get renamed in the outer crate via a crate renaming feature.

---

So.. Those are all pretty weak. Lets consult Alex Crichton, in rust-lang/cargo@79942fe which originally added the feature in Cargo, (emphasis mine):

Implement renaming dependencies in the manifest

This commit implements a new unstable feature for manifests which allows
renaming a crate when depending on it. For example you can do:

cargo-features = ["dependencies-as"]

...

[dependencies]
foo = "0.1"
bar = { version = "0.1", registry = "custom", package = "foo" }
baz = { git = "https://github.com/foo/bar", package = "foo" }

Here three crates will be imported but they'll be made available to the Rust
source code via the names foo (crates.io), bar (the custom registry), and
baz (the git dependency). The package name, however, will be foo for all
of them. In other words the git repository here would be searched for a crate
called foo. For example:

extern crate foo; // crates.io
extern crate bar; // registry `custom`
extern crate baz; // git repository

The intention here is to enable a few use cases:

• Enable depending on the same named crate from different registries
• Allow depending on multiple versions of a crate from one registry
• Removing the need for extern crate foo as bar syntactically in Rust source

Currently I don't think we're ready to stabilize this so it's just a nightly
feature, but I'm hoping we can continue to iterate on it!

[ghost]

First of all sorry to bump an old tread, but I think I have just encountered a situation that this issue refers to. I am trying to get new futures 0.3 to compile together with cargo-raze. I am encountering an annoying problem, which I think touches on this thread to enable to rename dependencies.

Futures 0.3 with compat feature enabled depend on it's own 0.1 version of the crate. They do this using the package syntax in the Cargo.toml file. I have a workaround for that in cargo-raze that overloads the futures 0.1 library and alias it as futures_01 as needed by futures_util crate. This however causes trait conflict as compiler sees futures::Future and future_01::Future as two different trait's regardless that they are both originating from futures-0.1.29.

I would love instead of recompile the whole futures-0.1.29 crate as futures_01 to just import it into futures-0.3.1 as a rename.

Would this represent a valid case for this feature, or am I missing something very simple?

I can provide a simple repo that shows this problem.

[kornholi]

I just ran into the same exact issue with futures compat.

I implemented something very similar to what @acmcarther described: renamed_deps (label -> str) on top of rust_library. This let me manually adjust the futures-util's BUILD file from cargo-raze to:

rust_library(
    ...    
    renamed_deps = {
        "@third_party__futures__0_1_29//:futures": "futures_01"
    },
)

It wasn't that hard to implement (<25 lines), but I'm not sure how much refactoring I'd need to do to get it mergeable. Right now I re-create CrateInfos in collect_deps with the desired name, which I find pretty hack-y. There are also some unresolved questions e.g. if renamed_deps should override crates from deps, or act as a second deps-like field.

Instead, I've been thinking about a different approach: a rust_aliased_crate rule that points to a rust target (maybe rust_renamed_crate? bikeshedding welcome). It would essentially output a CrateInfo with a different name, but point to the same output rlib (which we need to prevent trait conflicts, etc).

rust_aliased_crate(
    name = "futures_01",
    crate = "@third_party__futures__0_1_29//:futures"
)

We'd probably also need an optional crate_name attr to disconnect it from Bazel labels:

rust_aliased_crate(
    name = "__cargo_renamed__futures_01",
    crate_name = "futures_01",
    crate = "@third_party__futures__0_1_29//:futures"
)

This would let cargo-raze-like tools to generate rules without name clashes. Cases like a package declaring a test target foo, but also renaming a dependency bar to foo. I admit I haven't thought about this edge case too much.

cargo-raze would output something like this for futures-util:

rust_library(
    name = "futures_util",
    deps = [
        ":__cargo_renamed__futures_01",
        ...
    ],
    ...
)

rust_aliased_crate(
    name = "__cargo_renamed__futures_01",
    crate_name = "futures_01",
    crate = "@third_party__futures__0_1_29//:futures",
    visibility = ["//visibility:private"]
)

This would also support renaming in the root Cargo.toml consumed by cargo-raze. It could output a rust_aliased_crate rule instead of alias.

We could also include two "foo" crates which are independently renamed to something else:

rust_library(
    name = "foo",
    deps = [
        ":bar",
        ":baz"
    ],
)
rust_aliased_crate(
    name = "bar",
    crate = "//:foo"
)
rust_aliased_crate(
    name = "baz",
    crate = "//another:foo"
)

What do you think @acmcarther @mfarrugi @Sythanis? Am I missing something?

[acmcarther]

@Sythanis, @kornholi Apologies on the delay getting back to the both of you.

I think @kornholi 's suggestion makes sense and seems like what I had in mind a year ago and I think merging such a change would make sense.

Between "rust_library.renamed_deps" and a new "rust_aliased_crate", I think my preference would be for the former, regardless of how janky the implementation is. My rationale is that, afaict, a user could implement "rust_aliased_crate" in their own Bazel workspace, but they could not implement "renamed_deps" without forking and changing rules_rust. I also think "renamed_deps" is somewhat closer to the semantics of the analogous Cargo feature. I don't feel strongly about the above though and I think a PR containing either approach could make sense to merge.

[kornholi]

A similar approach has been implemented in #285