Improve auto discover (rust-lang#433)

* Add new field `gh_crate_meta::Context::archive_suffix` * Support `archive_suffix = ""` for unix * Use `archive-suffix` in `FULL_FILENAMES` & `NOVERSION_FILENAMES` to support `PkgFmt::Bin` * Simplify `PkgFmt::extensions` * Fix `default_bin_dir_template` in `infer_bin_dir_template`: Fix typo `archive_suffix` * Test `default_bin_dir_template` in CI * Fix installation of `PkgFmt::Bin`: `chmod +x $bin` on unix if fmt == Bin * Add miniserve to `.github/scripts/tests.sh` * Update `SUPPORT.md` * Add github-test-Cargo2.toml to test Github pkg-url with bin-dir provided * Avoid allocation on processing `path_normalized` in `BinFile::from_product` Signed-off-by: Jiahao XU <[email protected]>
EFanZh · Sep 28, 2022 · 4ba1e22 · 4ba1e22
1 parent c159036
commit 4ba1e22
Show file tree

Hide file tree

Showing 10 changed files with 94 additions and 51 deletions.
diff --git a/.github/scripts/cargo-tomls/bitbucket-test-Cargo.toml b/.github/scripts/cargo-tomls/bitbucket-test-Cargo.toml
@@ -8,9 +8,6 @@ authors = ["ryan <[email protected]>"]
 edition = "2021"
 license = "GPL-3.0"
 
-[package.metadata.binstall]
-bin-dir = "{ bin }{ binary-ext }"
-
 [[bin]]
 name = "cargo-binstall"
 path = "src/main.rs"
diff --git a/.github/scripts/cargo-tomls/github-test-Cargo.toml b/.github/scripts/cargo-tomls/github-test-Cargo.toml
@@ -8,9 +8,6 @@ authors = ["ryan <[email protected]>"]
 edition = "2021"
 license = "GPL-3.0"
 
-[package.metadata.binstall]
-bin-dir = "{ bin }{ binary-ext }"
-
 [[bin]]
 name = "cargo-binstall"
 path = "src/main.rs"
diff --git a/.github/scripts/cargo-tomls/github-test-Cargo2.toml b/.github/scripts/cargo-tomls/github-test-Cargo2.toml
@@ -0,0 +1,16 @@
+[package]
+name = "cargo-binstall"
+description = "Rust binary package installer for CI integration"
+repository = "https://github.com/cargo-bins/cargo-binstall"
+version = "0.12.0"
+rust-version = "1.61.0"
+authors = ["ryan <[email protected]>"]
+edition = "2021"
+license = "GPL-3.0"
+
+[package.metadata.binstall]
+bin-dir = "{ bin }{ binary-ext }"
+
+[[bin]]
+name = "cargo-binstall"
+path = "src/main.rs"
diff --git a/.github/scripts/cargo-tomls/gitlab-test-Cargo.toml b/.github/scripts/cargo-tomls/gitlab-test-Cargo.toml
@@ -8,9 +8,6 @@ authors = ["ryan <[email protected]>"]
 edition = "2021"
 license = "GPL-3.0"
 
-[package.metadata.binstall]
-bin-dir = "{ bin }{ binary-ext }"
-
 [[bin]]
 name = "cargo-binstall"
 path = "src/main.rs"
diff --git a/.github/scripts/tests.sh b/.github/scripts/tests.sh
@@ -7,19 +7,21 @@ unset CARGO_HOME
 
 # Install binaries using cargo-binstall
 # shellcheck disable=SC2086
-"./$1" binstall --log-level debug --no-confirm b3sum cargo-release cargo-binstall cargo-watch
+"./$1" binstall --log-level debug --no-confirm b3sum cargo-release cargo-binstall cargo-watch miniserve
 
 # Test that the installed binaries can be run
 b3sum --version
 cargo-release release --version
 cargo-binstall --help >/dev/null
 cargo binstall --help >/dev/null
 cargo watch -V
+miniserve -V
 
 test_resources=".github/scripts/cargo-tomls"
 
 # Install binaries using `--manifest-path`
-"./$1" binstall --force --log-level debug --manifest-path "$test_resources/gitlab-test-Cargo.toml" --no-confirm cargo-binstall
+# Also test default github template
+"./$1" binstall --force --log-level debug --manifest-path "$test_resources/github-test-Cargo.toml" --no-confirm cargo-binstall
 # Test that the installed binaries can be run
 cargo binstall --help >/dev/null
 
@@ -63,6 +65,7 @@ cargo binstall --help >/dev/null
 # FIXME: remove/replace once #136 lands
 PATH="$test_resources/fake-cargo:$PATH"
 
+# Test default GitLab pkg-url templates
 "./$1" binstall \
     --force \
     --manifest-path "$test_resources/gitlab-test-Cargo.toml" \
@@ -77,3 +80,12 @@ PATH="$test_resources/fake-cargo:$PATH"
     --log-level debug \
     --no-confirm \
     cargo-binstall
+
+# Test default Github pkg-url templates,
+# with bin-dir provided
+"./$1" binstall \
+    --force \
+    --manifest-path "$test_resources/github-test-Cargo2.toml" \
+    --log-level debug \
+    --no-confirm \
+    cargo-binstall
diff --git a/SUPPORT.md b/SUPPORT.md
@@ -11,7 +11,7 @@ As an example, the configuration would be like this:
 
 ```toml
 [package.metadata.binstall]
-pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }-v{ version }.{ archive-format }"
+pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }-v{ version }{ archive-suffix }"
 bin-dir = "{ name }-{ target }-v{ version }/{ bin }{ binary-ext }"
 pkg-fmt = "tgz"
 ```
@@ -30,7 +30,8 @@ Template variables use the format `{ VAR }` where `VAR` is the name of the varia
 - `repo` is the repository linked in `Cargo.toml`
 - `bin` is the name of a specific binary, inferred from the crate configuration
 - `target` is the rust target name (defaults to your architecture, but can be overridden using the `--target` command line option if required()
-- `archive-format` is the filename extension of the package archive format
+- `archive-suffix` is the filename extension of the package archive format that includes the prefix `.`, e.g. `.tgz` for tgz or `.exe`/`""` for bin.
+- `archive-format` is the soft-deprecated filename extension of the package archive format that does not include the prefix `.`, e.g. `tgz` for tgz or `exe`/`""` for bin.
 - `binary-ext` is the string `.exe` if the `target` is for Windows, or the empty string otherwise
 - `format` is a soft-deprecated alias for `archive-format` in `pkg-url`, and alias for `binary-ext` in `bin-dir`; in the future, this may warn at install time.
 
@@ -61,13 +62,13 @@ The URLs are derived from a set of filenames and a set of paths, which are
 "multiplied together": every filename appended to every path. The filenames
 are:
 
-- `{ name }-{ target }-{ version }.{ archive-format }`
-- `{ name }-{ target }-v{ version }.{ archive-format }`
-- `{ name }-{ version }-{ target }.{ archive-format }`
-- `{ name }-v{ version }-{ target }.{ archive-format }`
-- `{ name }-{ version }-{ target }.{ archive-format }`
-- `{ name }-v{ version }-{ target }.{ archive-format }`
-- `{ name }-{ target }.{ archive-format }` ("versionless")
+- `{ name }-{ target }-{ version }{ archive-suffix }`
+- `{ name }-{ target }-v{ version }{ archive-suffix }`
+- `{ name }-{ version }-{ target }{ archive-suffix }`
+- `{ name }-v{ version }-{ target }{ archive-suffix }`
+- `{ name }-{ version }-{ target }{ archive-suffix }`
+- `{ name }-v{ version }-{ target }{ archive-suffix }`
+- `{ name }-{ target }{ archive-suffix }` ("versionless")
 
 The paths are:
 
@@ -130,7 +131,7 @@ As is common with libraries/utilities (and the `radio-sx128x` example), this can
 
 ```toml
 [package.metadata.binstall]
-pkg-url = "{ repo }/releases/download/v{ version }/sx128x-util-{ target }-v{ version }.{ archive-format }"
+pkg-url = "{ repo }/releases/download/v{ version }/sx128x-util-{ target }-v{ version }{ archive-suffix }"
 ```
 
 Which provides a download URL of `https://github.com/rust-iot/rust-radio-sx128x/releases/download/v0.14.1-alpha.5/sx128x-util-x86_64-unknown-linux-gnu-v0.14.1-alpha.5.tgz`

diff --git a/crates/binstalk/src/bins.rs b/crates/binstalk/src/bins.rs
@@ -1,5 +1,6 @@
 use std::{
     borrow::Cow,
+    fs,
     path::{Component, Path, PathBuf},
 };
 
@@ -47,7 +48,7 @@ pub fn infer_bin_dir_template(data: &Data) -> Cow<'static, str> {
         name.to_string(),
     ];
 
-    let default_bin_dir_template = Cow::Borrowed("{ bin }{ binary_ext }");
+    let default_bin_dir_template = Cow::Borrowed("{ bin }{ binary-ext }");
 
     possible_dirs
         .into_iter()
@@ -67,6 +68,7 @@ pub struct BinFile {
     pub source: PathBuf,
     pub dest: PathBuf,
     pub link: Option<PathBuf>,
+    pub pkg_fmt: Option<PkgFmt>,
 }
 
 impl BinFile {
@@ -94,7 +96,9 @@ impl BinFile {
             binary_ext,
         };
 
-        let source = if data.meta.pkg_fmt == Some(PkgFmt::Bin) {
+        let pkg_fmt = data.meta.pkg_fmt;
+
+        let source = if pkg_fmt == Some(PkgFmt::Bin) {
             data.bin_path.clone()
         } else {
             // Generate install paths
@@ -113,12 +117,7 @@ impl BinFile {
                 });
             }
 
-            let source_file_path = match path_normalized {
-                Cow::Borrowed(..) => path,
-                Cow::Owned(path) => path.to_string_lossy().into_owned(),
-            };
-
-            data.bin_path.join(&source_file_path)
+            data.bin_path.join(path_normalized.as_ref())
         };
 
         // Destination at install dir + base-name{.extension}
@@ -141,6 +140,7 @@ impl BinFile {
             source,
             dest,
             link,
+            pkg_fmt,
         })
     }
 
@@ -183,6 +183,15 @@ impl BinFile {
             self.source.display(),
             self.dest.display()
         );
+
+        if self.pkg_fmt == Some(PkgFmt::Bin) {
+            #[cfg(unix)]
+            fs::set_permissions(
+                &self.source,
+                std::os::unix::fs::PermissionsExt::from_mode(0o755),
+            )?;
+        }
+
         atomic_install(&self.source, &self.dest)?;
 
         Ok(())

diff --git a/crates/binstalk/src/fetchers/gh_crate_meta.rs b/crates/binstalk/src/fetchers/gh_crate_meta.rs
@@ -203,6 +203,9 @@ struct Context<'c> {
     #[serde(rename = "archive-format")]
     pub archive_format: &'c str,
 
+    #[serde(rename = "archive-suffix")]
+    pub archive_suffix: &'c str,
+
     /// Filename extension on the binary, i.e. .exe on Windows, nothing otherwise
     #[serde(rename = "binary-ext")]
     pub binary_ext: &'c str,
@@ -211,16 +214,26 @@ struct Context<'c> {
 impl<'c> Context<'c> {
     pub(self) fn from_data_with_repo(
         data: &'c Data,
-        archive_format: &'c str,
+        archive_suffix: &'c str,
         repo: Option<&'c str>,
     ) -> Self {
+        let archive_format = if archive_suffix.is_empty() {
+            // Empty archive_suffix means PkgFmt::Bin
+            "bin"
+        } else {
+            debug_assert!(archive_suffix.starts_with('.'), "{archive_suffix}");
+
+            &archive_suffix[1..]
+        };
+
         Self {
             name: &data.name,
             repo,
             target: &data.target,
             version: &data.version,
             format: archive_format,
             archive_format,
+            archive_suffix,
             binary_ext: if data.target.contains("windows") {
                 ".exe"
             } else {
@@ -267,7 +280,7 @@ mod test {
             meta,
         };
 
-        let ctx = Context::from_data(&data, "tgz");
+        let ctx = Context::from_data(&data, ".tgz");
         assert_eq!(
             ctx.render_url(DEFAULT_PKG_URL).unwrap(),
             url("https://github.com/ryankurte/cargo-binstall/releases/download/v1.2.3/cargo-binstall-x86_64-unknown-linux-gnu-v1.2.3.tgz")
@@ -286,7 +299,7 @@ mod test {
             meta,
         };
 
-        let ctx = Context::from_data(&data, "tgz");
+        let ctx = Context::from_data(&data, ".tgz");
         ctx.render_url(data.meta.pkg_url.as_deref().unwrap())
             .unwrap();
     }
@@ -306,7 +319,7 @@ mod test {
             meta,
         };
 
-        let ctx = Context::from_data(&data, "tgz");
+        let ctx = Context::from_data(&data, ".tgz");
         assert_eq!(
             ctx.render_url(data.meta.pkg_url.as_deref().unwrap()).unwrap(),
             url("https://example.com/releases/download/v1.2.3/cargo-binstall-x86_64-unknown-linux-gnu-v1.2.3.tgz")
@@ -330,7 +343,7 @@ mod test {
             meta,
         };
 
-        let ctx = Context::from_data(&data, "tgz");
+        let ctx = Context::from_data(&data, ".tgz");
         assert_eq!(
             ctx.render_url(data.meta.pkg_url.as_deref().unwrap()).unwrap(),
             url("https://github.com/rust-iot/rust-radio-sx128x/releases/download/v0.14.1-alpha.5/sx128x-util-x86_64-unknown-linux-gnu-v0.14.1-alpha.5.tgz")
@@ -352,7 +365,7 @@ mod test {
             meta,
         };
 
-        let ctx = Context::from_data(&data, "tgz");
+        let ctx = Context::from_data(&data, ".tgz");
         assert_eq!(
             ctx.render_url(data.meta.pkg_url.as_deref().unwrap()).unwrap(),
             url("https://github.com/rust-iot/rust-radio-sx128x/releases/download/v0.14.1-alpha.5/sx128x-util-x86_64-unknown-linux-gnu-v0.14.1-alpha.5.tgz")
@@ -378,7 +391,7 @@ mod test {
             meta,
         };
 
-        let ctx = Context::from_data(&data, "txz");
+        let ctx = Context::from_data(&data, ".txz");
         assert_eq!(
             ctx.render_url(data.meta.pkg_url.as_deref().unwrap()).unwrap(),
             url("https://github.com/watchexec/cargo-watch/releases/download/v9.0.0/cargo-watch-v9.0.0-aarch64-apple-darwin.tar.xz")
@@ -401,7 +414,7 @@ mod test {
             meta,
         };
 
-        let ctx = Context::from_data(&data, "bin");
+        let ctx = Context::from_data(&data, ".bin");
         assert_eq!(
             ctx.render_url(data.meta.pkg_url.as_deref().unwrap()).unwrap(),
             url("https://github.com/watchexec/cargo-watch/releases/download/v9.0.0/cargo-watch-v9.0.0-aarch64-pc-windows-msvc.exe")

diff --git a/crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs b/crates/binstalk/src/fetchers/gh_crate_meta/hosting.rs
@@ -14,15 +14,15 @@ pub enum RepositoryHost {
 /// Make sure to update possible_dirs in `bins::infer_bin_dir_template`
 /// if you modified FULL_FILENAMES or NOVERSION_FILENAMES.
 pub const FULL_FILENAMES: &[&str] = &[
-    "{ name }-{ target }-v{ version }.{ archive-format }",
-    "{ name }-{ target }-{ version }.{ archive-format }",
-    "{ name }-{ version }-{ target }.{ archive-format }",
-    "{ name }-v{ version }-{ target }.{ archive-format }",
+    "{ name }-{ target }-v{ version }{ archive-suffix }",
+    "{ name }-{ target }-{ version }{ archive-suffix }",
+    "{ name }-{ version }-{ target }{ archive-suffix }",
+    "{ name }-v{ version }-{ target }{ archive-suffix }",
 ];
 
 pub const NOVERSION_FILENAMES: &[&str] = &[
-    "{ name }-{ target }.{ archive-format }",
-    "{ name }.{ archive-format }",
+    "{ name }-{ target }{ archive-suffix }",
+    "{ name }{ archive-suffix }",
 ];
 
 impl RepositoryHost {

diff --git a/crates/binstalk/src/manifests/cargo_toml_binstall/package_formats.rs b/crates/binstalk/src/manifests/cargo_toml_binstall/package_formats.rs
@@ -44,16 +44,17 @@ impl PkgFmt {
         }
     }
 
-    /// List of possible file extensions for the format.
+    /// List of possible file extensions for the format
+    /// (with prefix `.`).
     pub fn extensions(self) -> &'static [&'static str] {
         match self {
-            PkgFmt::Tar => &["tar"],
-            PkgFmt::Tbz2 => &["tbz2", "tar.bz2"],
-            PkgFmt::Tgz => &["tgz", "tar.gz"],
-            PkgFmt::Txz => &["txz", "tar.xz"],
-            PkgFmt::Tzstd => &["tzstd", "tzst", "tar.zst"],
-            PkgFmt::Bin => &["bin", "exe"],
-            PkgFmt::Zip => &["zip"],
+            PkgFmt::Tar => &[".tar"],
+            PkgFmt::Tbz2 => &[".tbz2", ".tar.bz2"],
+            PkgFmt::Tgz => &[".tgz", ".tar.gz"],
+            PkgFmt::Txz => &[".txz", ".tar.xz"],
+            PkgFmt::Tzstd => &[".tzstd", ".tzst", ".tar.zst"],
+            PkgFmt::Bin => &[".bin", ".exe", ""],
+            PkgFmt::Zip => &[".zip"],
         }
     }
 }