Skip to content

Commit 9a92f4d

Browse files
Nemo157syphar
Nemo157
authored and
syphar
committed
Rely on cargo adding features for optional dependencies
Since the `dep:` syntax was stabilized Cargo now reports optional dependencies as features for `cargo metadata`, so we don't need to manually add these ourselves, and it takes into account whether they've been hidden via `dep:` which we didn't. We don't get told which features are from optional dependencies, which is something we have been tracking in the database. But we haven't started using this data in the UI in the 2 years it's been tracked so we may as well drop it. The `serde` test checks that we don't regress on crates that expose their optional dependencies, the `stylish-core` test did not originally pass and checks that we now strip out optional dependencies hidden behind `dep:`.
1 parent f5359d4 commit 9a92f4d

File tree

5 files changed

+100
-66
lines changed

5 files changed

+100
-66
lines changed

‎src/db/add_package.rs

Lines changed: 24 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub(crate) fn add_package_into_database(
6363
VALUES (
6464
$1, $2, $3, $4, $5, $6, $7, $8, $9,
6565
$10, $11, $12, $13, $14, $15, $16, $17, $18,
66-
$19, $20, $21, $22, $23, $24, $25, $26, $27
66+
$19, $20, $21, $22, $23, $24, $25, $26, $27
6767
)
6868
ON CONFLICT (crate_id, version) DO UPDATE
6969
SET release_time = $3,
@@ -233,33 +233,17 @@ fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)>
233233
fn get_features(pkg: &MetadataPackage) -> Vec<Feature> {
234234
let mut features = Vec::with_capacity(pkg.features.len());
235235
if let Some(subfeatures) = pkg.features.get("default") {
236-
features.push(Feature::new("default".into(), subfeatures.clone(), false));
236+
features.push(Feature::new("default".into(), subfeatures.clone()));
237237
};
238238
features.extend(
239239
pkg.features
240240
.iter()
241241
.filter(|(name, _)| *name != "default")
242-
.map(|(name, subfeatures)| Feature::new(name.clone(), subfeatures.clone(), false)),
242+
.map(|(name, subfeatures)| Feature::new(name.clone(), subfeatures.clone())),
243243
);
244-
features.extend(get_optional_dependencies(pkg));
245244
features
246245
}
247246

248-
fn get_optional_dependencies(pkg: &MetadataPackage) -> Vec<Feature> {
249-
pkg.dependencies
250-
.iter()
251-
.filter(|dep| dep.optional)
252-
.map(|dep| {
253-
let name = if let Some(rename) = &dep.rename {
254-
rename.clone()
255-
} else {
256-
dep.name.clone()
257-
};
258-
Feature::new(name, Vec::new(), true)
259-
})
260-
.collect()
261-
}
262-
263247
/// Reads readme if there is any read defined in Cargo.toml of a Package
264248
fn get_readme(pkg: &MetadataPackage, source_dir: &Path) -> Result<Option<String>> {
265249
let readme_path = source_dir.join(pkg.readme.as_deref().unwrap_or("README.md"));
@@ -358,9 +342,9 @@ fn add_keywords_into_database(
358342
}
359343

360344
conn.execute(
361-
"INSERT INTO keyword_rels (rid, kid)
362-
SELECT $1 as rid, id as kid
363-
FROM keywords
345+
"INSERT INTO keyword_rels (rid, kid)
346+
SELECT $1 as rid, id as kid
347+
FROM keywords
364348
WHERE slug = ANY($2)
365349
ON CONFLICT DO NOTHING;",
366350
&[&release_id, &wanted_keywords.keys().collect::<Vec<_>>()],
@@ -414,15 +398,15 @@ fn update_owners_in_database(
414398
"INSERT INTO owner_rels (cid, oid)
415399
SELECT $1,oid
416400
FROM UNNEST($2::int[]) as oid
417-
ON CONFLICT (cid,oid)
401+
ON CONFLICT (cid,oid)
418402
DO NOTHING",
419403
&[&crate_id, &oids],
420404
)?;
421405

422406
conn.execute(
423407
"DELETE FROM owner_rels
424-
WHERE
425-
cid = $1 AND
408+
WHERE
409+
cid = $1 AND
426410
NOT (oid = ANY($2))",
427411
&[&crate_id, &oids],
428412
)?;
@@ -466,7 +450,7 @@ mod test {
466450

467451
let kw_r = conn
468452
.query(
469-
"SELECT kw.name,kw.slug
453+
"SELECT kw.name,kw.slug
470454
FROM keywords as kw
471455
INNER JOIN keyword_rels as kwr on kw.id = kwr.kid
472456
WHERE kwr.rid = $1
@@ -531,7 +515,7 @@ mod test {
531515
let mut conn = env.db().conn();
532516
let kw_r = conn
533517
.query(
534-
"SELECT kw.name,kw.slug
518+
"SELECT kw.name,kw.slug
535519
FROM keywords as kw
536520
INNER JOIN keyword_rels as kwr on kw.id = kwr.kid
537521
WHERE kwr.rid = $1
@@ -585,18 +569,18 @@ mod test {
585569
update_owners_in_database(&mut conn, &[owner1.clone()], crate_id)?;
586570

587571
let owner_def = conn.query_one(
588-
"SELECT login, avatar
572+
"SELECT login, avatar
589573
FROM owners",
590574
&[],
591575
)?;
592576
assert_eq!(owner_def.get::<_, String>(0), owner1.login);
593577
assert_eq!(owner_def.get::<_, String>(1), owner1.avatar);
594578

595579
let owner_rel = conn.query_one(
596-
"SELECT o.login
597-
FROM owners o, owner_rels r
598-
WHERE
599-
o.id = r.oid AND
580+
"SELECT o.login
581+
FROM owners o, owner_rels r
582+
WHERE
583+
o.id = r.oid AND
600584
r.cid = $1",
601585
&[&crate_id],
602586
)?;
@@ -633,10 +617,10 @@ mod test {
633617
assert_eq!(owner_def.get::<_, String>(1), updated_owner.avatar);
634618

635619
let owner_rel = conn.query_one(
636-
"SELECT o.login
637-
FROM owners o, owner_rels r
638-
WHERE
639-
o.id = r.oid AND
620+
"SELECT o.login
621+
FROM owners o, owner_rels r
622+
WHERE
623+
o.id = r.oid AND
640624
r.cid = $1",
641625
&[&crate_id],
642626
)?;
@@ -690,10 +674,10 @@ mod test {
690674

691675
let crate_owners: Vec<String> = conn
692676
.query(
693-
"SELECT o.login
694-
FROM owners o, owner_rels r
695-
WHERE
696-
o.id = r.oid AND
677+
"SELECT o.login
678+
FROM owners o, owner_rels r
679+
WHERE
680+
o.id = r.oid AND
697681
r.cid = $1",
698682
&[&crate_id],
699683
)?

‎src/db/migrate.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,7 @@ pub fn migrate(version: Option<Version>, conn: &mut Client) -> crate::error::Res
861861
),
862862
sql_migration!(
863863
context, 37, "add cdn-invalidation-queue table",
864-
"
864+
"
865865
CREATE TABLE cdn_invalidation_queue (
866866
id BIGSERIAL,
867867
crate VARCHAR(255) NOT NULL,
@@ -882,6 +882,13 @@ pub fn migrate(version: Option<Version>, conn: &mut Client) -> crate::error::Res
882882
DROP TABLE cdn_invalidation_queue;
883883
"
884884
),
885+
sql_migration!(
886+
context,
887+
38,
888+
"Remove mark for features that are derived from optional dependencies",
889+
"ALTER TYPE feature DROP ATTRIBUTE optional_dependency;",
890+
"ALTER TYPE feature ADD ATTRIBUTE optional_dependency BOOL;"
891+
),
885892
];
886893

887894
for migration in migrations {

‎src/db/types.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,11 @@ use serde::Serialize;
66
pub struct Feature {
77
pub(crate) name: String,
88
pub(crate) subfeatures: Vec<String>,
9-
/// [`None`] when this crate was built before optional dependencies were tracked
10-
pub(crate) optional_dependency: Option<bool>,
119
}
1210

1311
impl Feature {
14-
pub fn new(name: String, subfeatures: Vec<String>, optional_dependency: bool) -> Self {
15-
Feature {
16-
name,
17-
subfeatures,
18-
optional_dependency: Some(optional_dependency),
19-
}
12+
pub fn new(name: String, subfeatures: Vec<String>) -> Self {
13+
Feature { name, subfeatures }
2014
}
2115

2216
pub fn is_private(&self) -> bool {

‎src/docbuilder/rustwide_builder.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,4 +1160,56 @@ mod tests {
11601160
Ok(())
11611161
});
11621162
}
1163+
1164+
#[test]
1165+
#[ignore]
1166+
fn test_implicit_features_for_optional_dependencies() {
1167+
wrapper(|env| {
1168+
let crate_ = "serde";
1169+
let version = "1.0.152";
1170+
let mut builder = RustwideBuilder::init(env).unwrap();
1171+
assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?);
1172+
1173+
let mut conn = env.db().conn();
1174+
let features: Vec<crate::db::types::Feature> = conn
1175+
.query_opt(
1176+
"SELECT releases.features FROM releases
1177+
INNER JOIN crates ON crates.id = releases.crate_id
1178+
WHERE crates.name = $1 AND releases.version = $2",
1179+
&[&crate_, &version],
1180+
)?
1181+
.context("missing release")?
1182+
.get(0);
1183+
1184+
assert!(features.iter().any(|f| f.name == "serde_derive"));
1185+
1186+
Ok(())
1187+
});
1188+
}
1189+
1190+
#[test]
1191+
#[ignore]
1192+
fn test_no_implicit_features_for_optional_dependencies_with_dep_syntax() {
1193+
wrapper(|env| {
1194+
let crate_ = "stylish-core";
1195+
let version = "0.1.1";
1196+
let mut builder = RustwideBuilder::init(env).unwrap();
1197+
assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?);
1198+
1199+
let mut conn = env.db().conn();
1200+
let features: Vec<crate::db::types::Feature> = conn
1201+
.query_opt(
1202+
"SELECT releases.features FROM releases
1203+
INNER JOIN crates ON crates.id = releases.crate_id
1204+
WHERE crates.name = $1 AND releases.version = $2",
1205+
&[&crate_, &version],
1206+
)?
1207+
.context("missing release")?
1208+
.get(0);
1209+
1210+
assert!(!features.iter().any(|f| f.name == "with_builtin_macros"));
1211+
1212+
Ok(())
1213+
});
1214+
}
11631215
}

‎src/web/features.rs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ mod tests {
140140

141141
#[test]
142142
fn test_feature_map_filters_private() {
143-
let private1 = Feature::new("_private1".into(), vec!["feature1".into()], false);
144-
let feature2 = Feature::new("feature2".into(), Vec::new(), false);
143+
let private1 = Feature::new("_private1".into(), vec!["feature1".into()]);
144+
let feature2 = Feature::new("feature2".into(), Vec::new());
145145

146146
let raw = vec![private1.clone(), feature2.clone()];
147147
let feature_map = get_feature_map(raw);
@@ -153,15 +153,14 @@ mod tests {
153153

154154
#[test]
155155
fn test_default_tree_structure_with_nested_default() {
156-
let default = Feature::new(DEFAULT_NAME.into(), vec!["feature1".into()], false);
157-
let non_default = Feature::new("non-default".into(), Vec::new(), false);
156+
let default = Feature::new(DEFAULT_NAME.into(), vec!["feature1".into()]);
157+
let non_default = Feature::new("non-default".into(), Vec::new());
158158
let feature1 = Feature::new(
159159
"feature1".into(),
160160
vec!["feature2".into(), "feature3".into()],
161-
false,
162161
);
163-
let feature2 = Feature::new("feature2".into(), Vec::new(), false);
164-
let feature3 = Feature::new("feature3".into(), Vec::new(), false);
162+
let feature2 = Feature::new("feature2".into(), Vec::new());
163+
let feature3 = Feature::new("feature3".into(), Vec::new());
165164

166165
let raw = vec![
167166
default.clone(),
@@ -188,10 +187,9 @@ mod tests {
188187
let feature1 = Feature::new(
189188
"feature1".into(),
190189
vec!["feature2".into(), "feature3".into()],
191-
false,
192190
);
193-
let feature2 = Feature::new("feature2".into(), Vec::new(), false);
194-
let feature3 = Feature::new("feature3".into(), Vec::new(), false);
191+
let feature2 = Feature::new("feature2".into(), Vec::new());
192+
let feature3 = Feature::new("feature3".into(), Vec::new());
195193

196194
let raw = vec![feature3.clone(), feature2.clone(), feature1.clone()];
197195
let mut feature_map = get_feature_map(raw);
@@ -206,8 +204,8 @@ mod tests {
206204

207205
#[test]
208206
fn test_default_tree_structure_single_default() {
209-
let default = Feature::new(DEFAULT_NAME.into(), Vec::new(), false);
210-
let non_default = Feature::new("non-default".into(), Vec::new(), false);
207+
let default = Feature::new(DEFAULT_NAME.into(), Vec::new());
208+
let non_default = Feature::new("non-default".into(), Vec::new());
211209

212210
let raw = vec![default.clone(), non_default.clone()];
213211
let mut feature_map = get_feature_map(raw);
@@ -225,10 +223,9 @@ mod tests {
225223
let feature1 = Feature::new(
226224
"feature1".into(),
227225
vec!["feature10".into(), "feature11".into()],
228-
false,
229226
);
230-
let feature2 = Feature::new("feature2".into(), vec!["feature20".into()], false);
231-
let feature3 = Feature::new("feature3".into(), Vec::new(), false);
227+
let feature2 = Feature::new("feature2".into(), vec!["feature20".into()]);
228+
let feature3 = Feature::new("feature3".into(), Vec::new());
232229

233230
let raw = vec![feature3.clone(), feature2.clone(), feature1.clone()];
234231
let (features, default_len) = order_features_and_count_default_len(raw);
@@ -242,8 +239,8 @@ mod tests {
242239

243240
#[test]
244241
fn test_order_features_and_get_len_single_default() {
245-
let default = Feature::new(DEFAULT_NAME.into(), Vec::new(), false);
246-
let non_default = Feature::new("non-default".into(), Vec::new(), false);
242+
let default = Feature::new(DEFAULT_NAME.into(), Vec::new());
243+
let non_default = Feature::new("non-default".into(), Vec::new());
247244

248245
let raw = vec![default.clone(), non_default.clone()];
249246
let (features, default_len) = order_features_and_count_default_len(raw);

0 commit comments

Comments
 (0)