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:`.

Author: Nemo157

src/db/add_package.rs

@@ -63,7 +63,7 @@ pub(crate) fn add_package_into_database(
          VALUES (
             $1,  $2,  $3,  $4,  $5,  $6,  $7,  $8,  $9,
             $10, $11, $12, $13, $14, $15, $16, $17, $18,
-            $19, $20, $21, $22, $23, $24, $25, $26, $27 
+            $19, $20, $21, $22, $23, $24, $25, $26, $27
          )
          ON CONFLICT (crate_id, version) DO UPDATE
             SET release_time = $3,
@@ -233,33 +233,17 @@ fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)>
 fn get_features(pkg: &MetadataPackage) -> Vec<Feature> {
     let mut features = Vec::with_capacity(pkg.features.len());
     if let Some(subfeatures) = pkg.features.get("default") {
-        features.push(Feature::new("default".into(), subfeatures.clone(), false));
+        features.push(Feature::new("default".into(), subfeatures.clone()));
     };
     features.extend(
         pkg.features
             .iter()
             .filter(|(name, _)| *name != "default")
-            .map(|(name, subfeatures)| Feature::new(name.clone(), subfeatures.clone(), false)),
+            .map(|(name, subfeatures)| Feature::new(name.clone(), subfeatures.clone())),
     );
-    features.extend(get_optional_dependencies(pkg));
     features
 }
 
-fn get_optional_dependencies(pkg: &MetadataPackage) -> Vec<Feature> {
-    pkg.dependencies
-        .iter()
-        .filter(|dep| dep.optional)
-        .map(|dep| {
-            let name = if let Some(rename) = &dep.rename {
-                rename.clone()
-            } else {
-                dep.name.clone()
-            };
-            Feature::new(name, Vec::new(), true)
-        })
-        .collect()
-}
-
 /// Reads readme if there is any read defined in Cargo.toml of a Package
 fn get_readme(pkg: &MetadataPackage, source_dir: &Path) -> Result<Option<String>> {
     let readme_path = source_dir.join(pkg.readme.as_deref().unwrap_or("README.md"));
@@ -358,9 +342,9 @@ fn add_keywords_into_database(
     }
 
     conn.execute(
-        "INSERT INTO keyword_rels (rid, kid) 
-        SELECT $1 as rid, id as kid 
-        FROM keywords 
+        "INSERT INTO keyword_rels (rid, kid)
+        SELECT $1 as rid, id as kid
+        FROM keywords
         WHERE slug = ANY($2)
         ON CONFLICT DO NOTHING;",
         &[&release_id, &wanted_keywords.keys().collect::<Vec<_>>()],
@@ -414,15 +398,15 @@ fn update_owners_in_database(
         "INSERT INTO owner_rels (cid, oid)
              SELECT $1,oid
              FROM UNNEST($2::int[]) as oid
-             ON CONFLICT (cid,oid) 
+             ON CONFLICT (cid,oid)
              DO NOTHING",
         &[&crate_id, &oids],
     )?;
 
     conn.execute(
         "DELETE FROM owner_rels
-         WHERE 
-            cid = $1 AND 
+         WHERE
+            cid = $1 AND
             NOT (oid = ANY($2))",
         &[&crate_id, &oids],
     )?;
@@ -466,7 +450,7 @@ mod test {
 
             let kw_r = conn
                 .query(
-                    "SELECT kw.name,kw.slug 
+                    "SELECT kw.name,kw.slug
                     FROM keywords as kw
                     INNER JOIN keyword_rels as kwr on kw.id = kwr.kid
                     WHERE kwr.rid = $1
@@ -531,7 +515,7 @@ mod test {
             let mut conn = env.db().conn();
             let kw_r = conn
                 .query(
-                    "SELECT kw.name,kw.slug 
+                    "SELECT kw.name,kw.slug
                     FROM keywords as kw
                     INNER JOIN keyword_rels as kwr on kw.id = kwr.kid
                     WHERE kwr.rid = $1
@@ -585,18 +569,18 @@ mod test {
             update_owners_in_database(&mut conn, &[owner1.clone()], crate_id)?;
 
             let owner_def = conn.query_one(
-                "SELECT login, avatar 
+                "SELECT login, avatar
                 FROM owners",
                 &[],
             )?;
             assert_eq!(owner_def.get::<_, String>(0), owner1.login);
             assert_eq!(owner_def.get::<_, String>(1), owner1.avatar);
 
             let owner_rel = conn.query_one(
-                "SELECT o.login 
-                FROM owners o, owner_rels r 
-                WHERE 
-                    o.id = r.oid AND 
+                "SELECT o.login
+                FROM owners o, owner_rels r
+                WHERE
+                    o.id = r.oid AND
                     r.cid = $1",
                 &[&crate_id],
             )?;
@@ -633,10 +617,10 @@ mod test {
             assert_eq!(owner_def.get::<_, String>(1), updated_owner.avatar);
 
             let owner_rel = conn.query_one(
-                "SELECT o.login 
-                FROM owners o, owner_rels r 
-                WHERE 
-                    o.id = r.oid AND 
+                "SELECT o.login
+                FROM owners o, owner_rels r
+                WHERE
+                    o.id = r.oid AND
                     r.cid = $1",
                 &[&crate_id],
             )?;
@@ -690,10 +674,10 @@ mod test {
 
             let crate_owners: Vec<String> = conn
                 .query(
-                    "SELECT o.login 
-                     FROM owners o, owner_rels r 
-                     WHERE 
-                         o.id = r.oid AND 
+                    "SELECT o.login
+                     FROM owners o, owner_rels r
+                     WHERE
+                         o.id = r.oid AND
                          r.cid = $1",
                     &[&crate_id],
                 )?

src/db/migrate.rs

@@ -861,7 +861,7 @@ pub fn migrate(version: Option<Version>, conn: &mut Client) -> crate::error::Res
         ),
         sql_migration!(
             context, 37, "add cdn-invalidation-queue table",
-            " 
+            "
             CREATE TABLE cdn_invalidation_queue (
                 id BIGSERIAL,
                 crate VARCHAR(255) NOT NULL,
@@ -882,6 +882,13 @@ pub fn migrate(version: Option<Version>, conn: &mut Client) -> crate::error::Res
             DROP TABLE cdn_invalidation_queue;
             "
         ),
+        sql_migration!(
+            context,
+            38,
+            "Remove mark for features that are derived from optional dependencies",
+            "ALTER TYPE feature DROP ATTRIBUTE optional_dependency;",
+            "ALTER TYPE feature ADD ATTRIBUTE optional_dependency BOOL;"
+        ),
     ];
 
     for migration in migrations {

src/db/types.rs

@@ -6,17 +6,11 @@ use serde::Serialize;
 pub struct Feature {
     pub(crate) name: String,
     pub(crate) subfeatures: Vec<String>,
-    /// [`None`] when this crate was built before optional dependencies were tracked
-    pub(crate) optional_dependency: Option<bool>,
 }
 
 impl Feature {
-    pub fn new(name: String, subfeatures: Vec<String>, optional_dependency: bool) -> Self {
-        Feature {
-            name,
-            subfeatures,
-            optional_dependency: Some(optional_dependency),
-        }
+    pub fn new(name: String, subfeatures: Vec<String>) -> Self {
+        Feature { name, subfeatures }
     }
 
     pub fn is_private(&self) -> bool {

src/docbuilder/rustwide_builder.rs

@@ -1160,4 +1160,56 @@ mod tests {
             Ok(())
         });
     }
+
+    #[test]
+    #[ignore]
+    fn test_implicit_features_for_optional_dependencies() {
+        wrapper(|env| {
+            let crate_ = "serde";
+            let version = "1.0.152";
+            let mut builder = RustwideBuilder::init(env).unwrap();
+            assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?);
+
+            let mut conn = env.db().conn();
+            let features: Vec<crate::db::types::Feature> = conn
+                .query_opt(
+                    "SELECT releases.features FROM releases
+                     INNER JOIN crates ON crates.id = releases.crate_id
+                     WHERE crates.name = $1 AND releases.version = $2",
+                    &[&crate_, &version],
+                )?
+                .context("missing release")?
+                .get(0);
+
+            assert!(features.iter().any(|f| f.name == "serde_derive"));
+
+            Ok(())
+        });
+    }
+
+    #[test]
+    #[ignore]
+    fn test_no_implicit_features_for_optional_dependencies_with_dep_syntax() {
+        wrapper(|env| {
+            let crate_ = "stylish-core";
+            let version = "0.1.1";
+            let mut builder = RustwideBuilder::init(env).unwrap();
+            assert!(builder.build_package(crate_, version, PackageKind::CratesIo)?);
+
+            let mut conn = env.db().conn();
+            let features: Vec<crate::db::types::Feature> = conn
+                .query_opt(
+                    "SELECT releases.features FROM releases
+                     INNER JOIN crates ON crates.id = releases.crate_id
+                     WHERE crates.name = $1 AND releases.version = $2",
+                    &[&crate_, &version],
+                )?
+                .context("missing release")?
+                .get(0);
+
+            assert!(!features.iter().any(|f| f.name == "with_builtin_macros"));
+
+            Ok(())
+        });
+    }
 }

src/web/features.rs

@@ -140,8 +140,8 @@ mod tests {
 
     #[test]
     fn test_feature_map_filters_private() {
-        let private1 = Feature::new("_private1".into(), vec!["feature1".into()], false);
-        let feature2 = Feature::new("feature2".into(), Vec::new(), false);
+        let private1 = Feature::new("_private1".into(), vec!["feature1".into()]);
+        let feature2 = Feature::new("feature2".into(), Vec::new());
 
         let raw = vec![private1.clone(), feature2.clone()];
         let feature_map = get_feature_map(raw);
@@ -153,15 +153,14 @@ mod tests {
 
     #[test]
     fn test_default_tree_structure_with_nested_default() {
-        let default = Feature::new(DEFAULT_NAME.into(), vec!["feature1".into()], false);
-        let non_default = Feature::new("non-default".into(), Vec::new(), false);
+        let default = Feature::new(DEFAULT_NAME.into(), vec!["feature1".into()]);
+        let non_default = Feature::new("non-default".into(), Vec::new());
         let feature1 = Feature::new(
             "feature1".into(),
             vec!["feature2".into(), "feature3".into()],
-            false,
         );
-        let feature2 = Feature::new("feature2".into(), Vec::new(), false);
-        let feature3 = Feature::new("feature3".into(), Vec::new(), false);
+        let feature2 = Feature::new("feature2".into(), Vec::new());
+        let feature3 = Feature::new("feature3".into(), Vec::new());
 
         let raw = vec![
             default.clone(),
@@ -188,10 +187,9 @@ mod tests {
         let feature1 = Feature::new(
             "feature1".into(),
             vec!["feature2".into(), "feature3".into()],
-            false,
         );
-        let feature2 = Feature::new("feature2".into(), Vec::new(), false);
-        let feature3 = Feature::new("feature3".into(), Vec::new(), false);
+        let feature2 = Feature::new("feature2".into(), Vec::new());
+        let feature3 = Feature::new("feature3".into(), Vec::new());
 
         let raw = vec![feature3.clone(), feature2.clone(), feature1.clone()];
         let mut feature_map = get_feature_map(raw);
@@ -206,8 +204,8 @@ mod tests {
 
     #[test]
     fn test_default_tree_structure_single_default() {
-        let default = Feature::new(DEFAULT_NAME.into(), Vec::new(), false);
-        let non_default = Feature::new("non-default".into(), Vec::new(), false);
+        let default = Feature::new(DEFAULT_NAME.into(), Vec::new());
+        let non_default = Feature::new("non-default".into(), Vec::new());
 
         let raw = vec![default.clone(), non_default.clone()];
         let mut feature_map = get_feature_map(raw);
@@ -225,10 +223,9 @@ mod tests {
         let feature1 = Feature::new(
             "feature1".into(),
             vec!["feature10".into(), "feature11".into()],
-            false,
         );
-        let feature2 = Feature::new("feature2".into(), vec!["feature20".into()], false);
-        let feature3 = Feature::new("feature3".into(), Vec::new(), false);
+        let feature2 = Feature::new("feature2".into(), vec!["feature20".into()]);
+        let feature3 = Feature::new("feature3".into(), Vec::new());
 
         let raw = vec![feature3.clone(), feature2.clone(), feature1.clone()];
         let (features, default_len) = order_features_and_count_default_len(raw);
@@ -242,8 +239,8 @@ mod tests {
 
     #[test]
     fn test_order_features_and_get_len_single_default() {
-        let default = Feature::new(DEFAULT_NAME.into(), Vec::new(), false);
-        let non_default = Feature::new("non-default".into(), Vec::new(), false);
+        let default = Feature::new(DEFAULT_NAME.into(), Vec::new());
+        let non_default = Feature::new("non-default".into(), Vec::new());
 
         let raw = vec![default.clone(), non_default.clone()];
         let (features, default_len) = order_features_and_count_default_len(raw);