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

Point out which crates have a wildcard version that isn't allowed #6827

Closed
carols10cents opened this issue Jul 18, 2023 · 4 comments · Fixed by #6829
Closed

Point out which crates have a wildcard version that isn't allowed #6827

carols10cents opened this issue Jul 18, 2023 · 4 comments · Fixed by #6829

Comments

@carols10cents
Copy link
Member

carols10cents commented Jul 18, 2023

Current Behavior

A user tried to publish a crate with this Cargo.toml:

`Cargo.toml` content
[package]
name = "arbitrary"
version = "0.4.9"
description = "arbitrary"
edition = "2021"
license = "MIT"
readme = "README.md"

[lib]
crate-type = ["cdylib", "lib"]
name = "amm"

[features]
no-entrypoint = []
no-idl = []
no-log-ix-name = []
cpi = ["no-entrypoint"]
default = []
staging = []
test-bpf = []
test-bpf-snapshot = []

[profile.release]
overflow-checks = true

[dependencies]
anchor-lang = "0.26.0"
anchor-spl = "0.26.0"
mercurial-vault = { version = "0.4.7", features = ["cpi"] }
spl-token-swap = { version = "3.0.0", features = ["no-entrypoint"] }
meteora-stable-swap-math = "1.8.1"
meteora-stable-swap-client = "1.8.1"
meteora-marinade-sdk = { version = "0.1.0", features = ["cpi"] }
spl-stake-pool = { ver="0.6.4", features = ["no-entrypoint"] }
anyhow = "1.0.71"

[dev-dependencies]
solana-program-test = "~1.14.17"
solana-sdk = "~1.14.17"
solana-account-decoder = "~1.14.17"
solana-client = "~1.14.17"
spl-associated-token-account = { version = "1.1.2", features = ["no-entrypoint"] }
serde_json = "1.0.95"
serde = "1.0.159"
bincode = "1.3.3"

and got this error:

Caused by:
  the remote server responded with an error: wildcard (`*`) dependency constraints are not allowed 
on crates.io. See 
https://doc.rust-lang.org/cargo/faq.html#can-libraries-use--as-a-version-for-their-dependencies 
for more information

Can you spot the problem? How long did it take you?

Answer

The problem was this line:

spl-stake-pool = { ver="0.6.4", features = ["no-entrypoint"] }

that had ver instead of version, which is interpreted as "no version" or "wildcard" by crates.io.

Expected Behavior

If crates.io instead returned an error like this, pointing out which crate or crates in the list of dependencies was causing the problem:

Caused by:
  the remote server responded with an error: wildcard (`*`) dependency constraints are not allowed 
on crates.io. Dependencies with this problem: crate-x[, crate-y, crate-z, ...]. See 
https://doc.rust-lang.org/cargo/faq.html#can-libraries-use--as-a-version-for-their-dependencies for more information

it would be a lot easier to figure out what the problem was.

Steps To Reproduce

  1. Attempt to publish a crate with a bunch of dependencies, some of which have a wildcard version constraint or no version constraint.
  2. Get error message.

Environment

  • Browser: all
  • OS: all

Anything else?

No response

@carols10cents
Copy link
Member Author

carols10cents commented Jul 18, 2023

I'm happy to help anyone who wants to take this one on. Here's where the code needs to change:

  1. The WILDCARD_ERROR_MESSAGE needs to be different, maybe split up so the crate list can go in the middle? The error message doesn't have to be exactly what I said in the issue description, up to the implementer what you think is nice.
    pub const WILDCARD_ERROR_MESSAGE: &str = "wildcard (`*`) dependency constraints are not allowed \
    on crates.io. See https://doc.rust-lang.org/cargo/faq.html#can-\
    libraries-use--as-a-version-for-their-dependencies for more \
    information";
  2. The spot where that constant is used needs to change:
    if let Ok(version_req) = semver::VersionReq::parse(&dep.version_req.0) {
    if version_req == semver::VersionReq::STAR {
    return Err(cargo_err(WILDCARD_ERROR_MESSAGE));
    }
    }
    Oh, maybe this should just not be a constant anymore, like some of the other errors in here:
    if let Some(registry) = &dep.registry {
    if !registry.is_empty() {
    return Err(cargo_err(&format_args!("Dependency `{}` is hosted on another registry. Cross-registry dependencies are not permitted on crates.io.", &*dep.name)));
    }
    }
    // Match only identical names to ensure the index always references the original crate name
    let krate:Crate = Crate::by_exact_name(&dep.name)
    .first(conn)
    .map_err(|_| cargo_err(&format_args!("no known crate named `{}`", &*dep.name)))?;
  3. This test needs to be updated:
    #[test]
    fn new_krate_with_wildcard_dependency() {
    let (app, _, user, token) = TestApp::full().with_token();
    app.db(|conn| {
    // Insert a crate directly into the database so that new_wild can depend on it
    CrateBuilder::new("foo_wild", user.as_model().id).expect_build(conn);
    });
    let dependency = DependencyBuilder::new("foo_wild").version_req("*");
    let crate_to_publish = PublishBuilder::new("new_wild")
    .version("1.0.0")
    .dependency(dependency);
    let response = token.publish_crate(crate_to_publish);
    assert_eq!(response.status(), StatusCode::OK);
    assert_eq!(
    response.into_json(),
    json!({ "errors": [{ "detail": WILDCARD_ERROR_MESSAGE }] })
    );
    assert!(app.stored_files().is_empty());
    }

And actually, in the issue description, I was envisioning that if multiple dependencies had this problem, we'd list them all. However, this code collects all the Results and then uses ?, which will only return the first error. That's a bigger problem that this issue doesn't need to solve; I'm going to file another issue for that.

@carols10cents
Copy link
Member Author

Ah, that issue already exists 😅 #3054

@codewithgun
Copy link
Contributor

Hey, I could like to try this.

@codewithgun
Copy link
Contributor

Created #6829 for this. Please take a look when have the time. Thanks!

@Turbo87 Turbo87 linked a pull request Jul 19, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants