Support subcrate in repo (#991)

Fixed #838

 - Add new key `subcrate` for rendering `pkg-url`
 - Add new release paths in GitHub, GitLab & SourceForge using key `subcrate` for auto-detection
 - Add subcrate detection for GitHub and GitLab
 - Add `debug!` when using gh api token in `GhApiClient::new`
 - Add subcrate testing to `e2e-tests/subcrate.sh`
 - Bump cargo-release to 0.24.9 in e2e-tests/live.sh
   to fix test failure on MacOS without libssl installed in `/usr/local/`.
 - Optimize GhCrateMeta: Detect subcrate and repo-host in `Data::get_repo_info`
    to cache the result and avoid duplicate works, this also makes the code
    more ergonomic by removing the need to some `unwrap()` plus making it
    more efficient since we don't need to clone the url just to modify it.
 - Add instrument to `Data::get_repo_info`
 - Fix `shellcheck` err in `e2e-tests/*.sh`

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>

crates/binstalk-downloader/src/gh_api_client.rs

@@ -7,7 +7,7 @@ use std::{
 
 use compact_str::{CompactString, ToCompactString};
 use tokio::sync::OnceCell;
-use tracing::warn;
+use tracing::{debug, warn};
 
 use crate::remote;
 
@@ -106,6 +106,7 @@ impl GhApiClient {
     pub fn new(client: remote::Client, auth_token: Option<CompactString>) -> Self {
         let auth_token = auth_token.and_then(|auth_token| {
             if gh_prefixed(&auth_token) {
+                debug!("Using gh api token");
                 Some(auth_token)
             } else {
                 warn!("Invalid auth_token, expected 'gh*_' or `github_*`, fallback to unauthorized mode");

crates/binstalk/src/fetchers.rs

@@ -4,6 +4,7 @@ use compact_str::CompactString;
 pub use gh_crate_meta::*;
 pub use quickinstall::*;
 use tokio::sync::OnceCell;
+use tracing::{debug, instrument};
 use url::Url;
 
 use crate::{
@@ -18,6 +19,8 @@ use crate::{
 pub(crate) mod gh_crate_meta;
 pub(crate) mod quickinstall;
 
+use gh_crate_meta::hosting::RepositoryHost;
+
 #[async_trait::async_trait]
 pub trait Fetcher: Send + Sync {
     /// Create a new fetcher from some data
@@ -71,13 +74,20 @@ pub trait Fetcher: Send + Sync {
     fn target(&self) -> &str;
 }
 
+#[derive(Clone, Debug)]
+struct RepoInfo {
+    repo: Url,
+    repository_host: RepositoryHost,
+    subcrate: Option<String>,
+}
+
 /// Data required to fetch a package
 #[derive(Clone, Debug)]
 pub struct Data {
     name: CompactString,
     version: CompactString,
     repo: Option<String>,
-    repo_final_url: OnceCell<Option<Url>>,
+    repo_info: OnceCell<Option<RepoInfo>>,
 }
 
 impl Data {
@@ -86,18 +96,28 @@ impl Data {
             name,
             version,
             repo,
-            repo_final_url: OnceCell::new(),
+            repo_info: OnceCell::new(),
         }
     }
 
-    async fn resolve_final_repo_url(&self, client: &Client) -> Result<&Option<Url>, BinstallError> {
+    #[instrument(level = "debug")]
-        self.repo_final_url
+    async fn get_repo_info(&self, client: &Client) -> Result<&Option<RepoInfo>, BinstallError> {
+        self.repo_info
             .get_or_try_init(move || {
                 Box::pin(async move {
                     if let Some(repo) = self.repo.as_deref() {
-                        Ok(Some(
+                        let mut repo = client.get_redirected_final_url(Url::parse(repo)?).await?;
-                            client.get_redirected_final_url(Url::parse(repo)?).await?,
+                        let repository_host = RepositoryHost::guess_git_hosting_services(&repo);
-                        ))
+
+                        let repo_info = RepoInfo {
+                            subcrate: RepoInfo::detect_subcrate(&mut repo, repository_host),
+                            repo,
+                            repository_host,
+                        };
+
+                        debug!("Resolved repo_info = {repo_info:#?}");
+
+                        Ok(Some(repo_info))
                     } else {
                         Ok(None)
                     }
@@ -107,9 +127,113 @@ impl Data {
     }
 }
 
+impl RepoInfo {
+    /// If `repo` contains a subcrate, then extracts and returns it.
+    /// It will also remove that subcrate path from `repo` to match
+    /// `scheme:/{repo_owner}/{repo_name}`
+    fn detect_subcrate(repo: &mut Url, repository_host: RepositoryHost) -> Option<String> {
+        match repository_host {
+            RepositoryHost::GitHub => Self::detect_subcrate_common(repo, &["tree"]),
+            RepositoryHost::GitLab => Self::detect_subcrate_common(repo, &["-", "blob"]),
+            _ => None,
+        }
+    }
+
+    fn detect_subcrate_common(repo: &mut Url, seps: &[&str]) -> Option<String> {
+        let mut path_segments = repo.path_segments()?;
+
+        let _repo_owner = path_segments.next()?;
+        let _repo_name = path_segments.next()?;
+
+        // Skip separators
+        for sep in seps.iter().copied() {
+            if path_segments.next()? != sep {
+                return None;
+            }
+        }
+
+        // Skip branch name
+        let _branch_name = path_segments.next()?;
+
+        let subcrate = path_segments.next()?;
+
+        if path_segments.next().is_some() {
+            // A subcrate url should not contain anything more.
+            None
+        } else {
+            let subcrate = subcrate.to_string();
+
+            // Pop subcrate path to match regular repo style:
+            //
+            // scheme:/{addr}/{repo_owner}/{repo_name}
+            //
+            // path_segments() succeeds, so path_segments_mut()
+            // must also succeeds.
+            let mut paths = repo.path_segments_mut().unwrap();
+
+            paths.pop(); // pop subcrate
+            paths.pop(); // pop branch name
+            seps.iter().for_each(|_| {
+                paths.pop();
+            }); // pop separators
+
+            Some(subcrate)
+        }
+    }
+}
+
 /// Target specific data required to fetch a package
 #[derive(Clone, Debug)]
 pub struct TargetData {
     pub target: String,
     pub meta: PkgMeta,
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_detect_subcrate_github() {
+        let urls = [
+            "https://github.com/RustSec/rustsec/tree/main/cargo-audit",
+            "https://github.com/RustSec/rustsec/tree/master/cargo-audit",
+        ];
+        for url in urls {
+            let mut repo = Url::parse(url).unwrap();
+
+            let repository_host = RepositoryHost::guess_git_hosting_services(&repo);
+            assert_eq!(repository_host, RepositoryHost::GitHub);
+
+            let subcrate_prefix = RepoInfo::detect_subcrate(&mut repo, repository_host).unwrap();
+            assert_eq!(subcrate_prefix, "cargo-audit");
+
+            assert_eq!(
+                repo,
+                Url::parse("https://github.com/RustSec/rustsec").unwrap()
+            );
+        }
+    }
+
+    #[test]
+    fn test_detect_subcrate_gitlab() {
+        let urls = [
+            "https://gitlab.kitware.com/NobodyXu/hello/-/blob/main/cargo-binstall",
+            "https://gitlab.kitware.com/NobodyXu/hello/-/blob/master/cargo-binstall",
+        ];
+        for url in urls {
+            let mut repo = Url::parse(url).unwrap();
+
+            let repository_host = RepositoryHost::guess_git_hosting_services(&repo);
+            assert_eq!(repository_host, RepositoryHost::GitLab);
+
+            let subcrate_prefix = RepoInfo::detect_subcrate(&mut repo, repository_host).unwrap();
+            assert_eq!(subcrate_prefix, "cargo-binstall");
+
+            assert_eq!(
+                repo,
+                Url::parse("https://gitlab.kitware.com/NobodyXu/hello").unwrap()
+            );
+        }
+    }
+}

crates/binstalk/src/fetchers/gh_crate_meta.rs

@@ -20,10 +20,9 @@ use crate::{
     manifests::cargo_toml_binstall::{PkgFmt, PkgMeta},
 };
 
-use super::{Data, TargetData};
+use super::{Data, RepoInfo, TargetData};
 
 pub(crate) mod hosting;
-use hosting::RepositoryHost;
 
 pub struct GhCrateMeta {
     client: Client,
@@ -40,9 +39,16 @@ impl GhCrateMeta {
         pkg_fmt: PkgFmt,
         pkg_url: &Template<'_>,
         repo: Option<&str>,
+        subcrate: Option<&str>,
     ) {
         let render_url = |ext| {
-            let ctx = Context::from_data_with_repo(&self.data, &self.target_data.target, ext, repo);
+            let ctx = Context::from_data_with_repo(
+                &self.data,
+                &self.target_data.target,
+                ext,
+                repo,
+                subcrate,
+            );
             match ctx.render_url_with_compiled_tt(pkg_url) {
                 Ok(url) => Some(url),
                 Err(err) => {
@@ -99,7 +105,10 @@ impl super::Fetcher for GhCrateMeta {
 
     fn find(self: Arc<Self>) -> AutoAbortJoinHandle<Result<bool, BinstallError>> {
         AutoAbortJoinHandle::spawn(async move {
-            let repo = self.data.resolve_final_repo_url(&self.client).await?;
+            let info = self.data.get_repo_info(&self.client).await?.as_ref();
+
+            let repo = info.map(|info| &info.repo);
+            let subcrate = info.and_then(|info| info.subcrate.as_deref());
 
             let mut pkg_fmt = self.target_data.meta.pkg_fmt;
 
@@ -143,11 +152,23 @@ impl super::Fetcher for GhCrateMeta {
                 }
 
                 Either::Left(iter::once(template))
-            } else if let Some(repo) = repo.as_ref() {
+            } else if let Some(RepoInfo {
-                if let Some(pkg_urls) =
+                repo,
-                    RepositoryHost::guess_git_hosting_services(repo)?.get_default_pkg_url_template()
+                repository_host,
-                {
+                ..
-                    Either::Right(pkg_urls.map(Template::cast))
+            }) = info
+            {
+                if let Some(pkg_urls) = repository_host.get_default_pkg_url_template() {
+                    let has_subcrate = subcrate.is_some();
+
+                    Either::Right(
+                        pkg_urls
+                            .map(Template::cast)
+                            // If subcrate is Some, then all templates will be included.
+                            // Otherwise, only templates without key "subcrate" will be
+                            // included.
+                            .filter(move |template| has_subcrate || !template.has_key("subcrate")),
+                    )
                 } else {
                     warn!(
                         concat!(
@@ -172,7 +193,7 @@ impl super::Fetcher for GhCrateMeta {
             };
 
             // Convert Option<Url> to Option<String> to reduce size of future.
-            let repo = repo.as_ref().map(|u| u.as_str().trim_end_matches('/'));
+            let repo = repo.map(|u| u.as_str().trim_end_matches('/'));
 
             // Use reference to self to fix error of closure
             // launch_baseline_find_tasks which moves `this`
@@ -193,7 +214,7 @@ impl super::Fetcher for GhCrateMeta {
                 //             basically cartesian product.
                 //             |
                 for pkg_fmt in pkg_fmts.clone() {
-                    this.launch_baseline_find_tasks(&resolver, pkg_fmt, &pkg_url, repo);
+                    this.launch_baseline_find_tasks(&resolver, pkg_fmt, &pkg_url, repo, subcrate);
                 }
             }
 
@@ -271,6 +292,9 @@ struct Context<'c> {
 
     /// Filename extension on the binary, i.e. .exe on Windows, nothing otherwise
     pub binary_ext: &'c str,
+
+    /// Workspace of the crate inside the repository.
+    pub subcrate: Option<&'c str>,
 }
 
 impl leon::Values for Context<'_> {
@@ -290,6 +314,8 @@ impl leon::Values for Context<'_> {
 
             "binary-ext" => Some(Cow::Borrowed(self.binary_ext)),
 
+            "subcrate" => self.subcrate.map(Cow::Borrowed),
+
             _ => None,
         }
     }
@@ -301,6 +327,7 @@ impl<'c> Context<'c> {
         target: &'c str,
         archive_suffix: Option<&'c str>,
         repo: Option<&'c str>,
+        subcrate: Option<&'c str>,
     ) -> Self {
         let archive_format = archive_suffix.map(|archive_suffix| {
             if archive_suffix.is_empty() {
@@ -325,12 +352,19 @@ impl<'c> Context<'c> {
             } else {
                 ""
             },
+            subcrate,
         }
     }
 
     #[cfg(test)]
     pub(self) fn from_data(data: &'c Data, target: &'c str, archive_format: &'c str) -> Self {
-        Self::from_data_with_repo(data, target, Some(archive_format), data.repo.as_deref())
+        Self::from_data_with_repo(
+            data,
+            target,
+            Some(archive_format),
+            data.repo.as_deref(),
+            None,
+        )
     }
 
     /// * `tt` - must have added a template named "pkg_url".

crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs

@@ -3,9 +3,7 @@ use leon::{Item, Template};
 use leon_macros::template;
 use url::Url;
 
-use crate::errors::BinstallError;
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-
-#[derive(Copy, Clone, Debug)]
 pub enum RepositoryHost {
     GitHub,
     GitLab,
@@ -35,11 +33,17 @@ pub const NOVERSION_FILENAMES: &[Template<'_>] = &[
 const GITHUB_RELEASE_PATHS: &[Template<'_>] = &[
     template!("{ repo }/releases/download/{ version }"),
     template!("{ repo }/releases/download/v{ version }"),
+    // %2F is escaped form of '/'
+    template!("{ repo }/releases/download/{ subcrate }%2F{ version }"),
+    template!("{ repo }/releases/download/{ subcrate }%2Fv{ version }"),
 ];
 
 const GITLAB_RELEASE_PATHS: &[Template<'_>] = &[
     template!("{ repo }/-/releases/{ version }/downloads/binaries"),
     template!("{ repo }/-/releases/v{ version }/downloads/binaries"),
+    // %2F is escaped form of '/'
+    template!("{ repo }/-/releases/{ subcrate }%2F{ version }/downloads/binaries"),
+    template!("{ repo }/-/releases/{ subcrate }%2Fv{ version }/downloads/binaries"),
 ];
 
 const BITBUCKET_RELEASE_PATHS: &[Template<'_>] = &[template!("{ repo }/downloads")];
@@ -47,18 +51,21 @@ const BITBUCKET_RELEASE_PATHS: &[Template<'_>] = &[template!("{ repo }/downloads
 const SOURCEFORGE_RELEASE_PATHS: &[Template<'_>] = &[
     template!("{ repo }/files/binaries/{  version }"),
     template!("{ repo }/files/binaries/v{ version }"),
+    // %2F is escaped form of '/'
+    template!("{ repo }/files/binaries/{ subcrate }%2F{  version }"),
+    template!("{ repo }/files/binaries/{ subcrate }%2Fv{ version }"),
 ];
 
 impl RepositoryHost {
-    pub fn guess_git_hosting_services(repo: &Url) -> Result<Self, BinstallError> {
+    pub fn guess_git_hosting_services(repo: &Url) -> Self {
         use RepositoryHost::*;
 
         match repo.domain() {
-            Some(domain) if domain.starts_with("github") => Ok(GitHub),
+            Some(domain) if domain.starts_with("github") => GitHub,
-            Some(domain) if domain.starts_with("gitlab") => Ok(GitLab),
+            Some(domain) if domain.starts_with("gitlab") => GitLab,
-            Some(domain) if domain == "bitbucket.org" => Ok(BitBucket),
+            Some(domain) if domain == "bitbucket.org" => BitBucket,
-            Some(domain) if domain == "sourceforge.net" => Ok(SourceForge),
+            Some(domain) if domain == "sourceforge.net" => SourceForge,
-            _ => Ok(Unknown),
+            _ => Unknown,
         }
     }
 

e2e-tests/live.sh

@@ -4,9 +4,10 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-crates="b3sum@1.3.3 cargo-release@0.24.5 cargo-binstall@0.20.1 cargo-watch@8.4.0 miniserve@0.23.0 sccache@0.3.3"
+crates="b3sum@1.3.3 cargo-release@0.24.9 cargo-binstall@0.20.1 cargo-watch@8.4.0 miniserve@0.23.0 sccache@0.3.3"
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 othertmpdir=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-test')
 export PATH="$CARGO_HOME/bin:$othertmpdir/bin:$PATH"
 

e2e-tests/manifest-path.sh

@@ -4,7 +4,8 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 export PATH="$CARGO_HOME/bin:$PATH"
 
 # Install binaries using `--manifest-path`

e2e-tests/other-repos.sh

@@ -4,7 +4,8 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 export PATH="$CARGO_HOME/bin:$PATH"
 
 # Test default GitLab pkg-url templates

e2e-tests/self-upgrade-no-symlink.sh

@@ -4,7 +4,8 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 export PATH="$CARGO_HOME/bin:$PATH"
 
 # first boostrap-install into the CARGO_HOME

e2e-tests/strategies.sh

@@ -4,7 +4,8 @@ set -uxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 export PATH="$CARGO_HOME/bin:$PATH"
 
 ## Test --disable-strategies

e2e-tests/subcrate.sh

@@ -0,0 +1,18 @@
+#!/bin/bash
+
+set -euxo pipefail
+
+unset CARGO_INSTALL_ROOT
+
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
+othertmpdir=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-test')
+export PATH="$CARGO_HOME/bin:$othertmpdir/bin:$PATH"
+
+mkdir -p "$othertmpdir/bin"
+# Copy it to bin to test use of env var `CARGO`
+cp "./$1" "$othertmpdir/bin/"
+
+cargo binstall --no-confirm cargo-audit@0.17.5 --strategies crate-meta-data
+
+cargo audit --version

e2e-tests/tls.sh

@@ -4,7 +4,8 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 export PATH="$CARGO_HOME/bin:$PATH"
 
 "./$1" binstall \

e2e-tests/uninstall.sh

@@ -4,7 +4,8 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 othertmpdir=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-test')
 export PATH="$CARGO_HOME/bin:$othertmpdir/bin:$PATH"
 

e2e-tests/upgrade.sh

@@ -4,7 +4,8 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 export PATH="$CARGO_HOME/bin:$PATH"
 
 # Test skip when installed

e2e-tests/version-syntax.sh

@@ -4,7 +4,8 @@ set -euxo pipefail
 
 unset CARGO_INSTALL_ROOT
 
-export CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
+export CARGO_HOME
 export PATH="$CARGO_HOME/bin:$PATH"
 
 # Test --version

justfile

@@ -188,6 +188,7 @@ e2e-test file *arguments: (get-binary "e2e-tests")
     cd e2e-tests && env -u RUSTFLAGS bash {{file}}.sh {{output-filename}} {{arguments}}
 
 e2e-test-live: (e2e-test "live")
+e2e-test-subcrate: (e2e-test "subcrate")
 e2e-test-manifest-path: (e2e-test "manifest-path")
 e2e-test-other-repos: (e2e-test "other-repos")
 e2e-test-strategies: (e2e-test "strategies")
@@ -203,7 +204,7 @@ e2e-test-tls: (e2e-test "tls" "1.2")
 [macos]
 e2e-test-tls: (e2e-test "tls" "1.2") (e2e-test "tls" "1.3")
 
-e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall
+e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall e2e-test-subcrate
 
 unit-tests: print-env
     {{cargo-bin}} test {{cargo-build-args}}