Merge pull request #8859 from Turbo87/lib-or-bin

Add `has_lib` and `bin_names` fields to the versions API

crates/crates_io_tarball/src/lib.rs

@@ -7,11 +7,12 @@ pub use crate::builder::TarballBuilder;
 use crate::limit_reader::LimitErrorReader;
 use crate::manifest::validate_manifest;
 pub use crate::vcs_info::CargoVcsInfo;
+use cargo_manifest::AbstractFilesystem;
 pub use cargo_manifest::{Manifest, StringOrBool};
 use flate2::read::GzDecoder;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 use std::io::Read;
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 use std::str::FromStr;
 use tracing::instrument;
 
@@ -66,6 +67,7 @@ pub fn process_tarball<R: Read>(
     let pkg_root = Path::new(&pkg_name);
 
     let mut vcs_info = None;
+    let mut paths = Vec::new();
     let mut manifests = BTreeMap::new();
 
     for entry in archive.entries()? {
@@ -77,9 +79,9 @@ pub fn process_tarball<R: Read>(
         // as `bar-0.1.0/` source code, and this could overwrite other crates in
         // the registry!
         let entry_path = entry.path()?;
-        if !entry_path.starts_with(pkg_name) {
+        let Ok(in_pkg_path) = entry_path.strip_prefix(pkg_root) else {
             return Err(TarballError::InvalidPath(entry_path.display().to_string()));
-        }
+        };
 
         // Historical versions of the `tar` crate which Cargo uses internally
         // don't properly prevent hard links and symlinks from overwriting
@@ -93,6 +95,8 @@ pub fn process_tarball<R: Read>(
             ));
         }
 
+        paths.push(in_pkg_path.to_path_buf());
+
         // Let's go hunting for the VCS info and crate manifest. The only valid place for these is
         // in the package root in the tarball.
         if entry_path.parent() == Some(pkg_root) {
@@ -127,7 +131,7 @@ pub fn process_tarball<R: Read>(
     // on case-insensitive filesystems, to match the behaviour of cargo we should only actually
     // accept `Cargo.toml` and (the now deprecated) `cargo.toml` as valid options for the
     // manifest.
-    let Some((path, manifest)) = manifests.pop_first() else {
+    let Some((path, mut manifest)) = manifests.pop_first() else {
         return Err(TarballError::MissingManifest);
     };
 
@@ -136,15 +140,45 @@ pub fn process_tarball<R: Read>(
         return Err(TarballError::IncorrectlyCasedManifest(file.into()));
     }
 
+    manifest.complete_from_abstract_filesystem(&PathsFileSystem(paths))?;
+
     Ok(TarballInfo { manifest, vcs_info })
 }
 
+struct PathsFileSystem(Vec<PathBuf>);
+
+impl AbstractFilesystem for PathsFileSystem {
+    fn file_names_in<T: AsRef<Path>>(&self, rel_path: T) -> std::io::Result<BTreeSet<Box<str>>> {
+        let mut rel_path = rel_path.as_ref();
+
+        // Deal with relative paths that start with `./`
+        let mut components = rel_path.components();
+        while components.next() == Some(Component::CurDir) {
+            rel_path = components.as_path();
+        }
+
+        let paths = &self.0;
+        let file_names = paths
+            .iter()
+            .filter_map(move |p| p.strip_prefix(rel_path).ok())
+            .filter_map(|name| match name.components().next() {
+                // We can skip non-utf8 paths, since those are not checked by `cargo_manifest` anyway
+                Some(Component::Normal(p)) => p.to_str(),
+                _ => None,
+            })
+            .map(From::from)
+            .collect();
+
+        Ok(file_names)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::process_tarball;
     use crate::TarballBuilder;
     use cargo_manifest::{MaybeInherited, StringOrBool};
-    use insta::assert_snapshot;
+    use insta::{assert_debug_snapshot, assert_snapshot};
 
     const MANIFEST: &[u8] = b"[package]\nname = \"foo\"\nversion = \"0.0.1\"\n";
     const MAX_SIZE: u64 = 512 * 1024 * 1024;
@@ -157,6 +191,9 @@ mod tests {
 
         let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, MAX_SIZE));
         assert_none!(tarball_info.vcs_info);
+        assert_none!(tarball_info.manifest.lib);
+        assert_eq!(tarball_info.manifest.bin, vec![]);
+        assert_eq!(tarball_info.manifest.example, vec![]);
 
         let err = assert_err!(process_tarball("bar-0.0.1", &*tarball, MAX_SIZE));
         assert_snapshot!(err, @"invalid path found: foo-0.0.1/Cargo.toml");
@@ -309,4 +346,48 @@ mod tests {
         let err = assert_err!(process(vec!["Cargo.toml", "cargo.toml", "CARGO.TOML"]));
         assert_snapshot!(err, @r###"more than one Cargo.toml manifest in tarball: ["foo-0.0.1/CARGO.TOML", "foo-0.0.1/Cargo.toml", "foo-0.0.1/cargo.toml"]"###);
     }
+
+    #[test]
+    fn test_lib() {
+        let tarball = TarballBuilder::new()
+            .add_file("foo-0.0.1/Cargo.toml", MANIFEST)
+            .add_file("foo-0.0.1/src/lib.rs", b"pub fn foo() {}")
+            .build();
+
+        let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, MAX_SIZE));
+        let lib = assert_some!(tarball_info.manifest.lib);
+        assert_debug_snapshot!(lib);
+        assert_eq!(tarball_info.manifest.bin, vec![]);
+        assert_eq!(tarball_info.manifest.example, vec![]);
+    }
+
+    #[test]
+    fn test_lib_with_bins_and_example() {
+        let tarball = TarballBuilder::new()
+            .add_file("foo-0.0.1/Cargo.toml", MANIFEST)
+            .add_file("foo-0.0.1/examples/how-to-use-foo.rs", b"fn main() {}")
+            .add_file("foo-0.0.1/src/lib.rs", b"pub fn foo() {}")
+            .add_file("foo-0.0.1/src/bin/foo.rs", b"fn main() {}")
+            .add_file("foo-0.0.1/src/bin/bar.rs", b"fn main() {}")
+            .build();
+
+        let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, MAX_SIZE));
+        let lib = assert_some!(tarball_info.manifest.lib);
+        assert_debug_snapshot!(lib);
+        assert_debug_snapshot!(tarball_info.manifest.bin);
+        assert_debug_snapshot!(tarball_info.manifest.example);
+    }
+
+    #[test]
+    fn test_app() {
+        let tarball = TarballBuilder::new()
+            .add_file("foo-0.0.1/Cargo.toml", MANIFEST)
+            .add_file("foo-0.0.1/src/main.rs", b"fn main() {}")
+            .build();
+
+        let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, MAX_SIZE));
+        assert_none!(tarball_info.manifest.lib);
+        assert_debug_snapshot!(tarball_info.manifest.bin);
+        assert_eq!(tarball_info.manifest.example, vec![]);
+    }
 }

crates/crates_io_tarball/src/snapshots/crates_io_tarball__tests__app.snap

@@ -0,0 +1,28 @@
+---
+source: crates/crates_io_tarball/src/lib.rs
+expression: tarball_info.manifest.bin
+---
+[
+    Product {
+        path: Some(
+            "src/main.rs",
+        ),
+        name: Some(
+            "foo",
+        ),
+        test: true,
+        doctest: true,
+        bench: true,
+        doc: true,
+        plugin: false,
+        proc_macro: false,
+        harness: true,
+        edition: None,
+        required_features: [],
+        crate_type: Some(
+            [
+                "bin",
+            ],
+        ),
+    },
+]

crates/crates_io_tarball/src/snapshots/crates_io_tarball__tests__lib.snap

@@ -0,0 +1,26 @@
+---
+source: crates/crates_io_tarball/src/lib.rs
+expression: lib
+---
+Product {
+    path: Some(
+        "src/lib.rs",
+    ),
+    name: Some(
+        "foo",
+    ),
+    test: true,
+    doctest: true,
+    bench: true,
+    doc: true,
+    plugin: false,
+    proc_macro: false,
+    harness: true,
+    edition: None,
+    required_features: [],
+    crate_type: Some(
+        [
+            "lib",
+        ],
+    ),
+}

crates/crates_io_tarball/src/snapshots/crates_io_tarball__tests__lib_with_bins_and_example-2.snap

@@ -0,0 +1,50 @@
+---
+source: crates/crates_io_tarball/src/lib.rs
+expression: tarball_info.manifest.bin
+---
+[
+    Product {
+        path: Some(
+            "src/bin/bar.rs",
+        ),
+        name: Some(
+            "bar",
+        ),
+        test: true,
+        doctest: true,
+        bench: true,
+        doc: true,
+        plugin: false,
+        proc_macro: false,
+        harness: true,
+        edition: None,
+        required_features: [],
+        crate_type: Some(
+            [
+                "bin",
+            ],
+        ),
+    },
+    Product {
+        path: Some(
+            "src/bin/foo.rs",
+        ),
+        name: Some(
+            "foo",
+        ),
+        test: true,
+        doctest: true,
+        bench: true,
+        doc: true,
+        plugin: false,
+        proc_macro: false,
+        harness: true,
+        edition: None,
+        required_features: [],
+        crate_type: Some(
+            [
+                "bin",
+            ],
+        ),
+    },
+]

crates/crates_io_tarball/src/snapshots/crates_io_tarball__tests__lib_with_bins_and_example-3.snap

@@ -0,0 +1,28 @@
+---
+source: crates/crates_io_tarball/src/lib.rs
+expression: tarball_info.manifest.example
+---
+[
+    Product {
+        path: Some(
+            "examples/how-to-use-foo.rs",
+        ),
+        name: Some(
+            "how-to-use-foo",
+        ),
+        test: true,
+        doctest: true,
+        bench: true,
+        doc: true,
+        plugin: false,
+        proc_macro: false,
+        harness: true,
+        edition: None,
+        required_features: [],
+        crate_type: Some(
+            [
+                "bin",
+            ],
+        ),
+    },
+]

crates/crates_io_tarball/src/snapshots/crates_io_tarball__tests__lib_with_bins_and_example.snap

@@ -0,0 +1,26 @@
+---
+source: crates/crates_io_tarball/src/lib.rs
+expression: lib
+---
+Product {
+    path: Some(
+        "src/lib.rs",
+    ),
+    name: Some(
+        "foo",
+    ),
+    test: true,
+    doctest: true,
+    bench: true,
+    doc: true,
+    plugin: false,
+    proc_macro: false,
+    harness: true,
+    edition: None,
+    required_features: [],
+    crate_type: Some(
+        [
+            "lib",
+        ],
+    ),
+}

migrations/2024-06-14-081653_add-lib-and-bins-columns/down.sql

@@ -0,0 +1,3 @@
+alter table versions
+    drop column has_lib,
+    drop column bin_names;

migrations/2024-06-14-081653_add-lib-and-bins-columns/up.sql

@@ -0,0 +1,6 @@
+alter table versions
+    add has_lib boolean,
+    add bin_names text[];
+
+comment on column versions.has_lib is 'TRUE if the version has a library (e.g. `src/lib.rs`), FALSE if no library was detected, or NULL if the version has not been analyzed yet.';
+comment on column versions.bin_names is 'list of the names of all detected binaries in the version. the list may be empty which indicates that no binaries were detected in the version. the column may be NULL is the version has not been analyzed yet.';

src/controllers/krate/publish.rs

@@ -338,6 +338,14 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
                 }
             }
 
+            // https://doc.rust-lang.org/cargo/reference/cargo-targets.html#the-name-field says that
+            // the `name` field is required for `bin` targets, so we can ignore `None` values via
+            // `filter_map()` here.
+            let bin_names = tarball_info.manifest.bin
+                .into_iter()
+                .filter_map(|bin| bin.name.clone())
+                .collect();
+
             // Read tarball from request
             let hex_cksum: String = Sha256::digest(&tarball_bytes).encode_hex();
 
@@ -352,6 +360,8 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
                 .checksum(hex_cksum)
                 .links(package.links)
                 .rust_version(rust_version)
+                .has_lib(tarball_info.manifest.lib.is_some())
+                .bin_names(bin_names)
                 .build()
                 .map_err(|error| internal(error.to_string()))?
                 .save(conn, &verified_email_address)?;

src/models/version.rs

@@ -28,6 +28,8 @@ pub struct Version {
     pub checksum: String,
     pub links: Option<String>,
     pub rust_version: Option<String>,
+    pub has_lib: Option<bool>,
+    pub bin_names: Option<Vec<Option<String>>>,
 }
 
 impl Version {
@@ -82,6 +84,10 @@ pub struct NewVersion {
     links: Option<String>,
     #[builder(default)]
     rust_version: Option<String>,
+    #[builder(default, setter(strip_option))]
+    pub has_lib: Option<bool>,
+    #[builder(default, setter(strip_option))]
+    pub bin_names: Option<Vec<String>>,
 }
 
 impl NewVersionBuilder {

src/schema.rs

@@ -981,6 +981,10 @@ diesel::table! {
         ///
         /// (Automatically generated by Diesel.)
         rust_version -> Nullable<Varchar>,
+        /// TRUE if the version has a library (e.g. `src/lib.rs`), FALSE if no library was detected, or NULL if the version has not been analyzed yet.
+        has_lib -> Nullable<Bool>,
+        /// list of the names of all detected binaries in the version. the list may be empty which indicates that no binaries were detected in the version. the column may be NULL is the version has not been analyzed yet.
+        bin_names -> Nullable<Array<Nullable<Text>>>,
     }
 }