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

cargo add cannot enable features if version="*" in Cargo.toml #14648

Closed
georgmu opened this issue Oct 6, 2024 · 8 comments
Closed

cargo add cannot enable features if version="*" in Cargo.toml #14648

georgmu opened this issue Oct 6, 2024 · 8 comments
Labels
C-bug Category: bug Command-add S-propose-close Status: A team member has nominated this for closing, pending further input from the team

Comments

@georgmu
Copy link

georgmu commented Oct 6, 2024

Problem

When trying to run cargo add crate -F feature with crate = "*" or crate = { version="*" } in Cargo.toml, an error is returned.

Steps

  1. Add time = "*" to Cargo.toml
  2. Run the following command:
$ cargo +nightly add time -F serde
    Updating crates.io index
      Adding time * to dependencies
error: unrecognized feature for crate time: serde
no features available for crate time
  1. Change time = "*" to time = "0.3"
  2. Run the command again:
$ cargo +nightly add time -F serde
    Updating crates.io index
      Adding time v0.3 to dependencies
             Features as of v0.3.0:
             + alloc
             + serde
             + std
             - formatting
             - itoa
             - large-dates
             - local-offset
             - macros
             - parsing
             - quickcheck
             - quickcheck-dep
             - rand
             - serde-human-readable
             - time-macros

List of features changes when using the last concrete version (0.3.36).

Possible Solution(s)

For version="*", I would expect that it uses the feature list of the used root dependency (as in Cargo.lock) or of the latest version available.

Notes

No response

Version

cargo 1.83.0-nightly (80d82ca 2024-09-27)
release: 1.83.0-nightly
commit-hash: 80d82ca
commit-date: 2024-09-27
host: x86_64-unknown-linux-gnu
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w 11 Sep 2023
os: Fedora 41.0.0 (FortyOne) [64-bit]

@georgmu georgmu added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Oct 6, 2024
@epage
Copy link
Contributor

epage commented Oct 7, 2024

When cargo add looks for available features to display and validate against, it looks to the oldest version compatible with your version requirement, assuming

  1. You set the minimum supported version in your version requirement
  2. The dependency doesn't remove features, making this the lowest common denominator for your version requirement. Granted, this gets fuzzy with multi-major version requirements.

In your case, its picking time = "0.1.0" and only allowing features from that version.

@Kelvin-1013

This comment has been minimized.

@weihanglo

This comment has been minimized.

@weihanglo
Copy link
Member

The behavior is written down here in code:

// Ensure widest feature flag compatibility by picking the earliest version that could show up
// in the lock file for a given version requirement.
let lowest_common_denominator = possibilities
.iter()
.map(|s| s.as_summary())
.min_by_key(|s| {
// Fallback to a pre-release if no official release is available by sorting them as
// more.
let is_pre = !s.version().pre.is_empty();
(is_pre, s.version())
})
.ok_or_else(|| {
anyhow::format_err!("the crate `{dependency}` could not be found in registry index.")
})?;
dependency.apply_summary(&lowest_common_denominator);

It seems pretty like an implementation detail relying on the assumption in the comment. I don't think this should be documented, and picking up the latest version isn't better either. I would propose we close this as the behavior is intended.

@weihanglo weihanglo added S-propose-close Status: A team member has nominated this for closing, pending further input from the team and removed S-triage Status: This issue is waiting on initial triage. labels Nov 9, 2024
@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Nov 9, 2024
@Kelvin-1013
Copy link

Thank you for your detailed report on the issue with cargo add and the version="*" specification. I appreciate the clarity in your steps and the examples provided.

It's clear that the current behavior is tied to how Cargo determines the lowest common denominator for features based on the earliest compatible version. Your observation about it defaulting to time = "0.1.0" and the limitations that come with that is insightful.

While I understand the rationale behind this implementation, I agree that it can lead to confusion, especially when users expect the latest features to be available with a wildcard version. However, since this behavior seems intentional and documented in the code, I support closing this issue as 'not planned.'

It might be worth considering whether we could improve the documentation around this behavior in the future to help users understand the implications of using wildcard versions better. Thanks again for bringing this to our attention!

@epage
Copy link
Contributor

epage commented Nov 11, 2024

imo if we did anything, it would likely to be lint against wildcard dependencies, see #5340

@ctz
Copy link
Contributor

ctz commented Nov 18, 2024

It seems odd that cargo, which has a ton of code about resolving versions of crates, doesn't reuse this for determining which version's features are in play here? Especially in this case where the actual version is already known in Cargo.lock?

A diagnostic sounds good to me -- the current behaviour is surprising.

@epage
Copy link
Contributor

epage commented Nov 18, 2024

The current version is in Cargo.lock but your dependency specification says any version in existence could be used instead. We are trying to pick features that work for your dependency specification. However, as I said earlier

Granted, this gets fuzzy with multi-major version requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-add S-propose-close Status: A team member has nominated this for closing, pending further input from the team
Projects
None yet
Development

No branches or pull requests

5 participants