Skip to content

Add support for new feature syntax (RFC 3143) #3867

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

Merged
merged 3 commits into from
Dec 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/controllers/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use flate2::read::GzDecoder;
use hex::ToHex;
use sha2::{Digest, Sha256};
use std::collections::HashMap;
use std::io::Read;
use std::sync::Arc;
use swirl::Job;
Expand Down Expand Up @@ -213,15 +214,29 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult {
.uploader()
.upload_crate(&app, tarball, &krate, vers)?;

let (features, features2): (HashMap<_, _>, HashMap<_, _>) =
features.into_iter().partition(|(_k, vals)| {
!vals
.iter()
.any(|v| v.starts_with("dep:") || v.contains("?/"))
});
let (features2, v) = if features2.is_empty() {
(None, None)
} else {
(Some(features2), Some(2))
};

// Register this crate in our local git repo.
let git_crate = git::Crate {
name: name.0,
vers: vers.to_string(),
cksum: hex_cksum,
features,
features2,
deps: git_deps,
yanked: Some(false),
links,
v,
};
worker::add_crate(git_crate).enqueue(&conn)?;

Expand Down
32 changes: 32 additions & 0 deletions src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,41 @@ pub struct Crate {
pub deps: Vec<Dependency>,
pub cksum: String,
pub features: HashMap<String, Vec<String>>,
/// This field contains features with new, extended syntax. Specifically,
/// namespaced features (`dep:`) and weak dependencies (`pkg?/feat`).
///
/// It is only populated if a feature uses the new syntax. Cargo merges it
/// on top of the `features` field when reading the entries.
///
/// This is separated from `features` because versions older than 1.19
/// will fail to load due to not being able to parse the new syntax, even
/// with a `Cargo.lock` file.
#[serde(skip_serializing_if = "Option::is_none")]
pub features2: Option<HashMap<String, Vec<String>>>,
pub yanked: Option<bool>,
#[serde(default)]
pub links: Option<String>,
/// The schema version for this entry.
///
/// If this is None, it defaults to version 1. Entries with unknown
/// versions are ignored by cargo starting with 1.51.
///
/// Version `2` format adds the `features2` field.
///
/// This provides a method to safely introduce changes to index entries
/// and allow older versions of cargo to ignore newer entries it doesn't
/// understand. This is honored as of 1.51, so unfortunately older
/// versions will ignore it, and potentially misinterpret version 2 and
/// newer entries.
///
/// The intent is that versions older than 1.51 will work with a
/// pre-existing `Cargo.lock`, but they may not correctly process `cargo
/// update` or build a lock from scratch. In that case, cargo may
/// incorrectly select a new package that uses a new index format. A
/// workaround is to downgrade any packages that are incompatible with the
/// `--precise` flag of `cargo update`.
#[serde(skip_serializing_if = "Option::is_none")]
pub v: Option<u32>,
}

#[derive(Serialize, Deserialize, Debug)]
Expand Down
18 changes: 12 additions & 6 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,13 @@ impl Crate {

/// Validates a whole feature string, `features = ["THIS", "ALL/THIS"]`.
pub fn valid_feature(name: &str) -> bool {
let mut parts = name.split('/');
let name_part = parts.next_back(); // required
let prefix_part = parts.next_back(); // optional
parts.next().is_none()
&& name_part.map_or(false, Crate::valid_feature_name)
&& prefix_part.map_or(true, Crate::valid_feature_prefix)
match name.split_once('/') {
Some((dep, dep_feat)) => {
let dep = dep.strip_suffix('?').unwrap_or(dep);
Crate::valid_feature_prefix(dep) && Crate::valid_feature_name(dep_feat)
}
None => Crate::valid_feature_name(name.strip_prefix("dep:").unwrap_or(name)),
}
}

/// Return both the newest (most recently updated) and
Expand Down Expand Up @@ -498,6 +499,11 @@ mod tests {
assert!(Crate::valid_feature("c++20"));
assert!(Crate::valid_feature("krate/c++20"));
assert!(!Crate::valid_feature("c++20/wow"));
assert!(Crate::valid_feature("foo?/bar"));
assert!(Crate::valid_feature("dep:foo"));
assert!(!Crate::valid_feature("dep:foo?/bar"));
assert!(!Crate::valid_feature("foo/?bar"));
assert!(!Crate::valid_feature("foo?bar"));
}
}

Expand Down
15 changes: 14 additions & 1 deletion src/tests/builders/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub struct PublishBuilder {
readme: Option<String>,
tarball: Vec<u8>,
version: semver::Version,
features: HashMap<u::EncodableFeatureName, Vec<u::EncodableFeature>>,
}

impl PublishBuilder {
Expand All @@ -55,6 +56,7 @@ impl PublishBuilder {
readme: None,
tarball: EMPTY_TARBALL_BYTES.to_vec(),
version: semver::Version::parse("1.0.0").unwrap(),
features: HashMap::new(),
}
}

Expand Down Expand Up @@ -166,11 +168,22 @@ impl PublishBuilder {
self
}

// Adds a feature.
pub fn feature(mut self, name: &str, values: &[&str]) -> Self {
let values = values
.iter()
.map(|s| u::EncodableFeature(s.to_string()))
.collect();
self.features
.insert(u::EncodableFeatureName(name.to_string()), values);
self
}

pub fn build(self) -> (String, Vec<u8>) {
let new_crate = u::EncodableCrateUpload {
name: u::EncodableCrateName(self.krate_name.clone()),
vers: u::EncodableCrateVersion(self.version),
features: HashMap::new(),
features: self.features,
deps: self.deps,
description: self.desc,
homepage: None,
Expand Down
69 changes: 69 additions & 0 deletions src/tests/http-data/krate_publish_features_version_2
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
[
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0.crate",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"content-length",
"35"
],
[
"host",
"alexcrichton-test.s3.amazonaws.com"
],
[
"accept-encoding",
"gzip"
],
[
"content-type",
"application/x-tar"
],
[
"authorization",
"AWS AKIAICL5IWUZYWWKA7JA:uDc39eNdF6CcwB+q+JwKsoDLQc4="
],
[
"date",
"Fri, 15 Sep 2017 07:53:06 -0700"
]
],
"body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
},
"response": {
"status": 200,
"headers": [
[
"x-amz-request-id",
"26589A5E52F8395C"
],
[
"ETag",
"\"f9016ad360cebb4fe2e6e96e5949f022\""
],
[
"date",
"Fri, 15 Sep 2017 14:53:07 GMT"
],
[
"content-length",
"0"
],
[
"x-amz-id-2",
"JdIvnNTw53aqXjBIqBLNuN4kxf/w1XWX+xuIiGBDYy7yzOSDuAMtBSrTW4ZWetcCIdqCUHuQ51A="
],
[
"Server",
"AmazonS3"
]
],
"body": ""
}
}
]
34 changes: 34 additions & 0 deletions src/tests/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use flate2::Compression;
use http::StatusCode;
use std::collections::HashMap;
use std::io::Read;
use std::iter::FromIterator;
use std::time::Duration;
use std::{io, thread};

Expand Down Expand Up @@ -954,3 +955,36 @@ fn publish_rate_limit_doesnt_affect_existing_crates() {
token.enqueue_publish(new_version).good();
app.run_pending_background_jobs();
}

#[test]
fn features_version_2() {
let (app, _, user, token) = TestApp::full().with_token();

app.db(|conn| {
// Insert a crate directly into the database so that foo_new can depend on it
CrateBuilder::new("bar", user.as_model().id).expect_build(conn);
});

let dependency = DependencyBuilder::new("bar");

let crate_to_publish = PublishBuilder::new("foo")
.version("1.0.0")
.dependency(dependency)
.feature("new_feat", &["dep:bar", "bar?/feat"])
.feature("old_feat", &[]);
token.enqueue_publish(crate_to_publish).good();
app.run_pending_background_jobs();

let crates = app.crates_from_index_head("foo");
assert_eq!(crates.len(), 1);
assert_eq!(crates[0].name, "foo");
assert_eq!(crates[0].deps.len(), 1);
assert_eq!(crates[0].v, Some(2));
let features = HashMap::from_iter([("old_feat".to_string(), vec![])]);
assert_eq!(crates[0].features, features);
let features2 = HashMap::from_iter([(
"new_feat".to_string(),
vec!["dep:bar".to_string(), "bar?/feat".to_string()],
)]);
assert_eq!(crates[0].features2, Some(features2));
}