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

Renaming dependencies in Cargo.toml #5653

Closed
matklad opened this Issue Jun 26, 2018 · 15 comments

Comments

Projects
None yet
8 participants
@matklad
Copy link
Member

matklad commented Jun 26, 2018

Implementation PR: #4953

Summary:

With cargo-features = ["rename-dependency"] in Cargo.toml, you will be able to specify dependencies as bar = { package = "foo", version = "0.1" }. That adds dependency to foo package from crates.io, which will be visible as extern crate bar.

Steps:

  • cargo metadata should include information about renames, #5583.

  • outstanding bugs:

  • Figure out to do about --features on the command line? Does --features foo/bar imply the package foo or the crate that's renamed to foo? We probably want the latter, and that's probably buggy right now.

Stabilization TODO:

@matklad

This comment has been minimized.

Copy link
Member Author

matklad commented Jun 26, 2018

Note: preferably, this should be stabilized before or in tandem with rust-lang/rust#44660

@ehuss

This comment was marked as resolved.

Copy link
Contributor

ehuss commented Jun 26, 2018

Some bugs have been noted recently:

Also, there is another tracking issue for this: #5422

@matklad

This comment was marked as resolved.

Copy link
Member Author

matklad commented Jun 26, 2018

Excellent, thanks @ehuss! I've updated the original issue.

@joshtriplett joshtriplett changed the title Reaming dependencies in Cargo.toml Renaming dependencies in Cargo.toml Jul 5, 2018

@withoutboats

This comment was marked as resolved.

Copy link
Contributor

withoutboats commented Jul 20, 2018

Added another bug: #5753.

@TheDan64

This comment has been minimized.

Copy link

TheDan64 commented Aug 20, 2018

rust#51796 and #5753 seem to be fixed and closed according to those threads. Can the checklist be updated?

Edit: Thanks!

@Nemo157

This comment has been minimized.

Copy link
Contributor

Nemo157 commented Sep 2, 2018

New bug with optional renamed dependencies once published: #5962

@joshtriplett

This comment has been minimized.

Copy link
Member

joshtriplett commented Sep 5, 2018

I'd definitely like to see this stabilized. One note: we need to consider the implications of stabilizing a feature like this, including to cargo dependency resolution, and to tools that parse Cargo.toml files. It would be nice to have a mechanism that specifies Cargo features the parsing tool must understand to successfully process a crate, and to incorporate that mechanism into the Cargo crate index. I'm working on a proposal for this (much simpler than past proposals).

@TheDan64

This comment has been minimized.

Copy link

TheDan64 commented Oct 17, 2018

Anecdotally, I'd like to share my experience trying out this really cool feature in my project, inkwell, which is a safe wrapper over LLVM:

In inkwell we support multiple LLVM versions, by using feature flags mutually exclusively. This lead us to having version branches which only differ from master by setting a default version feature flag and a llvm-sys dependency version.

I was hoping this cargo feature could greatly simplify this approach and allow us to simply have a master branch with this kind of configuration:

[features]
default = []
llvm3-6 = []
# ...
llvm7-0 = []

[dependencies]
llvm-sys-36 = { package = "llvm-sys", version = "36.2" }
# ...
llvm-sys-70 = { package = "llvm-sys", version = "70.0" }
// lib.rs
#[cfg(feature = "llvm3-6")]
extern crate llvm_sys_36 as llvm_sys;
// ...
#[cfg(feature = "llvm7-0")]
extern crate llvm_sys_70 as llvm_sys;

use llvm_sys::X;

I got the distinct impression that this should work, however, in practice there's one massive problem which seems obvious in retrospect: linker conflicts. Indeed, cargo ends up spitting out this sort of error message:

    Updating crates.io index
error: failed to select a version for `llvm-sys`.
    ... required by package `inkwell v0.1.0 (/mnt/d/LinuxData/repos/inkwell)`
versions that meet the requirements `^50.2` are: 50.2.0

the package `llvm-sys` links to the native library `llvm`, but it conflicts with a previous package which links to `llvm` as well:
package `llvm-sys v40.2.0`
    ... which is depended on by `inkwell v0.1.0 (/mnt/d/LinuxData/repos/inkwell)`

failed to select a version for `llvm-sys` which could resolve this conflict

Anyway, I totally understand my use case here is very very niche and possibly misusing the intent of rust feature flags and/or this feature. I just thought I'd mention that it would be nice to be able to let cargo know you'd like to use different versions of a crate in a mutually exclusive way, if at all possible, so that you can avoid such linker errors when using ffi/-sys crates with this feature.

@withoutboats

This comment has been minimized.

Copy link
Contributor

withoutboats commented Oct 17, 2018

@TheDan64 I notice in your code sample you didn't make those dependencies optional? I don't know that that will actually impact linking, but it seems like making them optional and only available when the feature is turned on might help your situation.

@TheDan64

This comment has been minimized.

Copy link

TheDan64 commented Oct 17, 2018

@withoutboats thanks for the idea! I didn't even think about that. Unfortunately I still end up with the same linking error, which is surprising.

@Nemo157

This comment has been minimized.

Copy link
Contributor

Nemo157 commented Oct 17, 2018

@TheDan64 testing a similar situation (two optional dependencies that both link against the same native library, neither actually enabled) without using renaming gives the same error. So if that is a supported usecase (I'm not sure it is since an optional dependency is a feature, and features are supposed to be purely additive, but enabling both would have to give the linking error) then using renaming with it seems like it should work (and likely would Just Work™️).

I'm surprised I can't find an issue about that situation, might be worth opening one to clarify whether it should be possible.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 17, 2018

Thanks for the comment @TheDan64! I think though that's unrelated to this in that crate renaming is probably working but you're blocked by a different error in Cargo. Unfortunately Cargo has to pessimistically assume that you could enable all the features all the time, so it's not possible to resolve that crate graph as it'd bring in two versions of LLVM

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Nov 13, 2018

It would be nice to stabilize this feature and backport it to 1.31 beta before the code freeze https://internals.rust-lang.org/t/beta-1-31-code-freeze/8825, since that is the first version shipping with the 2018 edition.

@Nemo157

This comment has been minimized.

Copy link
Contributor

Nemo157 commented Nov 13, 2018

And there is a published version of futures-preview using this feature without issues (this time), so it has been tested in the wild on nightly.

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Nov 13, 2018

Stabilize the `rename-dependency` feature
This commit stabilizes Cargo's `rename-dependency` feature which allows
renaming packages in `Cargo.toml`, enabling importing multiple versions
of a single crate or simply avoiding a `use foo as bar` in `src/lib.rs`.

Closes rust-lang#5653
@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Nov 13, 2018

Thanks for the reminder @SimonSapin, I've posted that as #6311

bors added a commit that referenced this issue Nov 15, 2018

Auto merge of #6311 - alexcrichton:stabilize-renames, r=the-whole-team
Stabilize the `rename-dependency` feature

This commit stabilizes Cargo's `rename-dependency` feature which allows
renaming packages in `Cargo.toml`, enabling importing multiple versions
of a single crate or simply avoiding a `use foo as bar` in `src/lib.rs`.

Closes #5653

bors added a commit that referenced this issue Nov 15, 2018

Auto merge of #6311 - alexcrichton:stabilize-renames, r=the-whole-team
Stabilize the `rename-dependency` feature

This commit stabilizes Cargo's `rename-dependency` feature which allows
renaming packages in `Cargo.toml`, enabling importing multiple versions
of a single crate or simply avoiding a `use foo as bar` in `src/lib.rs`.

Closes #5653

bors added a commit that referenced this issue Nov 15, 2018

Auto merge of #6311 - alexcrichton:stabilize-renames, r=the-whole-team
Stabilize the `rename-dependency` feature

This commit stabilizes Cargo's `rename-dependency` feature which allows
renaming packages in `Cargo.toml`, enabling importing multiple versions
of a single crate or simply avoiding a `use foo as bar` in `src/lib.rs`.

Closes #5653

@bors bors closed this in #6311 Nov 15, 2018

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Nov 16, 2018

Stabilize the `rename-dependency` feature
This commit stabilizes Cargo's `rename-dependency` feature which allows
renaming packages in `Cargo.toml`, enabling importing multiple versions
of a single crate or simply avoiding a `use foo as bar` in `src/lib.rs`.

Closes rust-lang#5653

synek317 pushed a commit to synek317/cargo that referenced this issue Nov 17, 2018

Stabilize the `rename-dependency` feature
This commit stabilizes Cargo's `rename-dependency` feature which allows
renaming packages in `Cargo.toml`, enabling importing multiple versions
of a single crate or simply avoiding a `use foo as bar` in `src/lib.rs`.

Closes rust-lang#5653

untitaker added a commit to untitaker/tarpaulin that referenced this issue Jan 2, 2019

Upgrade cargo to allow new manifest features
This fixes "unable to get packages from source" errors when a dependency
uses 2018 edition features such as dependency renaming: rust-lang/cargo#5653

Centril added a commit to Centril/rust that referenced this issue Feb 13, 2019

Rollup merge of rust-lang#58273 - taiki-e:rename-dependency, r=matthe…
…wjasper

Rename rustc_errors dependency in rust 2018 crates

I think this is a better solution than `use rustc_errors as errors` in `lib.rs` and `use crate::errors` in modules.

Related: rust-lang/cargo#5653

cc rust-lang#58099

r? @Centril

Centril added a commit to Centril/rust that referenced this issue Feb 13, 2019

Rollup merge of rust-lang#58273 - taiki-e:rename-dependency, r=matthe…
…wjasper

Rename rustc_errors dependency in rust 2018 crates

I think this is a better solution than `use rustc_errors as errors` in `lib.rs` and `use crate::errors` in modules.

Related: rust-lang/cargo#5653

cc rust-lang#58099

r? @Centril

Centril added a commit to Centril/rust that referenced this issue Feb 13, 2019

Rollup merge of rust-lang#58273 - taiki-e:rename-dependency, r=matthe…
…wjasper

Rename rustc_errors dependency in rust 2018 crates

I think this is a better solution than `use rustc_errors as errors` in `lib.rs` and `use crate::errors` in modules.

Related: rust-lang/cargo#5653

cc rust-lang#58099

r? @Centril
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.