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.

Sign up for GitHub

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

Jump to bottom

cargo doesn't consider crate renaming for optional dependency #5962

Closed
Arnavion opened this issue Sep 2, 2018 · 2 comments · Fixed by #5993
Closed

cargo doesn't consider crate renaming for optional dependency #5962

Arnavion opened this issue Sep 2, 2018 · 2 comments · Fixed by #5993

Comments

@Arnavion
Copy link

Arnavion commented Sep 2, 2018

cargo can't install futures-util-preview:0.3.0-alpha.4. That crate uses the rename-dependency feature like this:

cargo-features = ["rename-dependency"]

[features]
compat = ["futures01"]

[dependencies]
futures01 = { package = "futures", version = "0.1", optional = true }

I have a Cargo.toml that depends on futures-util-preview

[dependencies]
futures-util-preview = "0.3.0-alpha.4"

cargo update fails with

error: no matching version `^0.3.0-alpha.4` found for package `futures-util-preview`
location searched: registry `https://github.com/rust-lang/crates.io-index`
versions found: 0.2.2

With RUST_LOG=debug :

INFO 2018-09-02T20:33:03Z: cargo::sources::registry::index: failed to parse `futures-util-preview` registry package: Feature `compat` includes `futures01` which is neither a dependency nor another feature

This is with latest nightly cargo cargo 1.29.0-nightly (0ec7281b9 2018-08-20).

@Nemo157 suggested

Seems related to #5753, probably the registry related code wasn’t changed to behave in the same way.

Ref: rust-lang/futures-rs#1247
@Arnavion Arnavion mentioned this issue Sep 2, 2018
Closed
@Nemo157 Nemo157 mentioned this issue Sep 2, 2018
Closed
4 tasks
@Nemo157
Copy link
Member

Nemo157 commented Sep 3, 2018

Dumping the registry entry for futures-util-preview(0.3.0-alpha.4) it doesn't mention the renaming in any way,

{
  "name": "futures-util-preview",
  "vers": "0.3.0-alpha.4",
  "deps": [
    ...
    {
      "name": "futures",
      "req": "^0.1",
      "features": [],
      "optional": true,
      "default_features": true,
      "target": null,
      "kind": "normal"
    },
    ...
  ],
  "features": {
    ...
    "compat": [
      "std",
      "futures01"
    ]
  },
  ...
}

It seems like either renaming will have to be included in the registry, or Cargo can't check features until it has the full manifest available (would this impact which crates are downloaded? I seem to recall that Cargo will download the entire dependency tree ignoring features and only use features to determine which crates to build, but I'm not certain of that).

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Sep 7, 2018
@alexcrichton
Fix publishing renamed dependencies to crates.io
e82443d 
This commit fixes publishing crates which contain locally renamed dependencies
to crates.io. Previously this lack of information meant that although we could
resolve the crate graph correctly it wouldn't work well with respect to optional
features and optional dependencies. The fix here is to persist this information
into the registry about the crate being renamed in `Cargo.toml`, allowing Cargo
to correctly deduce feature names as it does when it has `Cargo.toml` locally.

A dual side of this commit is to publish this information to crates.io. We'll
want to merge the associated PR (link to come soon) on crates.io first and make
sure that's deployed as well before we stabilize the crate renaming feature.

The index format is updated as well as part of this change. The `name` key for
dependencies is now unconditionally what was written in `Cargo.toml` as the
left-hand-side of the dependency specification. In other words this is the raw
crate name, but only for the local crate. A new key, `package`, is added to
dependencies (and it can be `None`). This key indicates the crates.io package is
being linked against, an represents the `package` key in `Cargo.toml`.

It's important to consider the interaction with older Cargo implementations
which don't support the `package` key in the index. In these situations older
Cargo binaries will likely fail to resolve entirely as the renamed name is
unlikely to exist on crates.io. For example the `futures` crate now has an
optional dependency with the name `futures01` which depends on an older version
of `futures` on crates.io. The string `futures01` will be listed in the index
under the `"name"` key, but no `futures01` crate exists on crates.io so older
Cargo will generate an error. If the crate does exist on crates.io, then even
weirder error messages will likely result.

Closes rust-lang#5962
@alexcrichton alexcrichton mentioned this issue Sep 7, 2018
Merged
@alexcrichton
Copy link
Member

Thanks for the detailed report here! I believe this should be fixed in #5993

bors added a commit that referenced this issue Sep 18, 2018
@bors
Auto merge of #5993 - alexcrichton:publish-renames, r=ehuss
74b4f4f 
Fix publishing renamed dependencies to crates.io

This commit fixes publishing crates which contain locally renamed dependencies
to crates.io. Previously this lack of information meant that although we could
resolve the crate graph correctly it wouldn't work well with respect to optional
features and optional dependencies. The fix here is to persist this information
into the registry about the crate being renamed in `Cargo.toml`, allowing Cargo
to correctly deduce feature names as it does when it has `Cargo.toml` locally.

A dual side of this commit is to publish this information to crates.io. We'll
want to merge the associated PR (link to come soon) on crates.io first and make
sure that's deployed as well before we stabilize the crate renaming feature.

The index format is updated as well as part of this change. The `name` key for
dependencies is now unconditionally what was written in `Cargo.toml` as the
left-hand-side of the dependency specification. In other words this is the raw
crate name, but only for the local crate. A new key, `package`, is added to
dependencies (and it can be `None`). This key indicates the crates.io package is
being linked against, an represents the `package` key in `Cargo.toml`.

It's important to consider the interaction with older Cargo implementations
which don't support the `package` key in the index. In these situations older
Cargo binaries will likely fail to resolve entirely as the renamed name is
unlikely to exist on crates.io. For example the `futures` crate now has an
optional dependency with the name `futures01` which depends on an older version
of `futures` on crates.io. The string `futures01` will be listed in the index
under the `"name"` key, but no `futures01` crate exists on crates.io so older
Cargo will generate an error. If the crate does exist on crates.io, then even
weirder error messages will likely result.

Closes #5962
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix publishing renamed dependencies to crates.io alexcrichton/cargo
3 participants
@alexcrichton @Nemo157 @Arnavion