Split {format} and allow use of {binary-ext} in pkg-url (#95)

This from feedback in #19:

> wrt. bin-dir and bin-path, this appears to be a typo / should all be called bin-dir

This is only a readme fix afaict, I changed all occurences of `bin-path` in there to `bin-dir`.

> wrt. format, those are actually two (unfortunately named) different concepts, the first
refers to the archive format (eg. .tgz), the second to the binary format (which needs a .exe
appended for windows).

This introduces two new substitutions:
- `binary-ext` is the old "`format` in `bin-dir`"
- `archive-format` is the old "`format` in `pkg-url`"

Contents are unchanged: `binary-ext` includes the dot, `archive-format` doesn't. That
makes it easy to upgrade and also personally I slightly prefer it that way.

The old contextual `format` is still available, "soft deprecated": it will be accepted silently
so everything will work, but all documentation will use the new syntax. In the future we
could move to a "hard deprecated" model where installing a package that uses `format`
will warn the user / tell them to report that to the maintainer. I don't think we'll ever really
be able to remove it but that should be good enough.

A cool new feature is that `binary-ext` is now usable in `pkg-url`, which will be useful for raw binary downloads:

```toml
pkg_url = "{ repo }/releases/download/v{ version }/{ name }-v{ version }-{ target }{ binary-ext }"
```

I've also added a bunch of tests to GhCrateMeta around the templating for `pkg-url`.

Cargo.toml

@@ -10,8 +10,8 @@ license = "GPL-3.0"
 
 
 [package.metadata.binstall]
-pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }.{ format }"
+pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }.{ archive-format }"
-bin-dir = "{ bin }{ format }"
+bin-dir = "{ bin }{ binary-ext }"
 
 [package.metadata.binstall.overrides.x86_64-pc-windows-msvc]
 pkg-fmt = "zip"

README.md

@@ -86,27 +86,30 @@ To get started, add a `[package.metadata.binstall]` section to your `Cargo.toml`
 
 ```toml
 [package.metadata.binstall]
-pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }-v{ version }.{ format }"
+pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }-v{ version }.{ archive-format }"
-bin-dir = "{ name }-{ target }-v{ version }/{ bin }{ format }"
+bin-dir = "{ name }-{ target }-v{ version }/{ bin }{ binary-ext }"
 pkg-fmt = "tgz"
 ```
 
 With the following configuration keys:
 
 - `pkg-url` specifies the package download URL for a given target/version, templated
-- `bin-path` specifies the binary path within the package, templated (with an `.exe` suffix on windows)
+- `bin-dir` specifies the binary path within the package, templated (with an `.exe` suffix on windows)
 - `pkg-fmt` overrides the package format for download/extraction (defaults to: `tgz`)
 
 
-`pkg-url` and `bin-path` are templated to support different names for different versions / architectures / etc.
+`pkg-url` and `bin-dir` are templated to support different names for different versions / architectures / etc.
 Template variables use the format `{ VAR }` where `VAR` is the name of the variable, with the following variables available:
 - `name` is the name of the crate / package
 - `version` is the crate version (per `--version` and the crate manifest)
 - `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().
+- `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
+- `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 for `binary-ext` in `bin-dir`; in the future this may warn at install time.
 
-`pkg-url`, `pkg-fmt` and `bin-path` can be overridden on a per-target basis if required, for example, if your `x86_64-pc-windows-msvc` builds use `zip` archives this could be set via:
+`pkg-url`, `pkg-fmt` and `bin-dir` can be overridden on a per-target basis if required, for example, if your `x86_64-pc-windows-msvc` builds use `zip` archives this could be set via:
 
 ```
 [package.metadata.binstall.overrides.x86_64-pc-windows-msvc]
@@ -117,13 +120,13 @@ pkg-fmt = "zip"
 
 By default `binstall` is setup to work with github releases, and expects to find:
 
-- an archive named `{ name }-{ target }-v{ version }.{ format }`
+- an archive named `{ name }-{ target }-v{ version }.{ archive-format }`
   - so that this does not overwrite different targets or versions when manually downloaded
 - located at `{ repo }/releases/download/v{ version }/`
   - compatible with github tags / releases
 - containing a folder named `{ name }-{ target }-v{ version }`
   - so that prior binary files are not overwritten when manually executing `tar -xvf ...`
-- containing binary files in the form `{ bin }{ format }` (where `bin` is the cargo binary name and `format` is `.exe` on windows and empty on other platforms)
+- containing binary files in the form `{ bin }{ binary-ext }` (where `bin` is the cargo binary name and `binary-ext` is `.exe` on windows and empty on other platforms)
 
 If your package already uses this approach, you shouldn't need to set anything.
 
@@ -146,7 +149,7 @@ As is common with libraries / utilities (and the `radio-sx128x` example), this c
 
 ```toml
 [package.metadata.binstall]
-pkg-url = "{ repo }/releases/download/v{ version }/sx128x-util-{ target }-v{ version }.{ format }"
+pkg-url = "{ repo }/releases/download/v{ version }/sx128x-util-{ target }-v{ version }.{ archive-format }"
 ```
 
 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`
@@ -158,7 +161,7 @@ Were the package to contain binaries in the form `name-target[.exe]`, this could
 
 ```toml
 [package.metadata.binstall]
-bin-dir = "{ bin }-{ target }{ format }"
+bin-dir = "{ bin }-{ target }{ binary-ext }"
 ```
 
 Which provides a binary path of: `sx128x-util-x86_64-unknown-linux-gnu[.exe]`. It is worth noting that binary names are inferred from the crate, so long as cargo builds them this _should_ just work.

src/bins.rs

@@ -16,18 +16,20 @@ impl BinFile {
     pub fn from_product(data: &Data, product: &Product) -> Result<Self, anyhow::Error> {
         let base_name = product.name.clone().unwrap();
 
-        // Generate binary path via interpolation
+        let binary_ext = if data.target.contains("windows") {
+            ".exe"
+        } else {
+            ""
+        };
+
         let ctx = Context {
             name: &data.name,
             repo: data.repo.as_ref().map(|s| &s[..]),
             target: &data.target,
             version: &data.version,
-            format: if data.target.contains("windows") {
-                ".exe"
-            } else {
-                ""
-            },
             bin: &base_name,
+            format: binary_ext,
+            binary_ext,
         };
 
         // Generate install paths
@@ -39,8 +41,8 @@ impl BinFile {
             data.bin_path.join(&source_file_path)
         };
 
-        // Destination path is the install dir + base-name-version{.format}
+        // Destination path is the install dir + base-name-version{.extension}
-        let dest_file_path = ctx.render("{ bin }-v{ version }{ format }")?;
+        let dest_file_path = ctx.render("{ bin }-v{ version }{ binary-ext }")?;
         let dest = data.install_path.join(dest_file_path);
 
         // Link at install dir + base name
@@ -118,8 +120,14 @@ struct Context<'c> {
     pub repo: Option<&'c str>,
     pub target: &'c str,
     pub version: &'c str,
-    pub format: &'c str,
     pub bin: &'c str,
+
+    /// Soft-deprecated alias for binary-ext
+    pub format: &'c str,
+
+    /// Filename extension on the binary, i.e. .exe on Windows, nothing otherwise
+    #[serde(rename = "binary-ext")]
+    pub binary_ext: &'c str,
 }
 
 impl<'c> Template for Context<'c> {}

src/fetchers/gh_crate_meta.rs

@@ -16,18 +16,11 @@ pub struct GhCrateMeta {
 #[async_trait::async_trait]
 impl super::Fetcher for GhCrateMeta {
     async fn new(data: &Data) -> Result<Box<Self>, anyhow::Error> {
-        // Generate context for URL interpolation
+        let ctx = Context::from_data(data);
-        let ctx = Context {
-            name: &data.name,
-            repo: data.repo.as_ref().map(|s| &s[..]),
-            target: &data.target,
-            version: &data.version,
-            format: data.meta.pkg_fmt.to_string(),
-        };
         debug!("Using context: {:?}", ctx);
 
         Ok(Box::new(Self {
-            url: Url::parse(&ctx.render(&data.meta.pkg_url)?)?,
+            url: ctx.render_url(&data.meta.pkg_url)?,
             pkg_fmt: data.meta.pkg_fmt,
         }))
     }
@@ -68,7 +61,184 @@ struct Context<'c> {
     pub repo: Option<&'c str>,
     pub target: &'c str,
     pub version: &'c str,
+
+    /// Soft-deprecated alias for archive-format
     pub format: String,
+
+    /// Archive format e.g. tar.gz, zip
+    #[serde(rename = "archive-format")]
+    pub archive_format: String,
+
+    /// Filename extension on the binary, i.e. .exe on Windows, nothing otherwise
+    #[serde(rename = "binary-ext")]
+    pub binary_ext: &'c str,
 }
 
 impl<'c> Template for Context<'c> {}
+
+impl<'c> Context<'c> {
+    pub(self) fn from_data(data: &'c Data) -> Self {
+        let pkg_fmt = data.meta.pkg_fmt.to_string();
+        Self {
+            name: &data.name,
+            repo: data.repo.as_ref().map(|s| &s[..]),
+            target: &data.target,
+            version: &data.version,
+            format: pkg_fmt.clone(),
+            archive_format: pkg_fmt,
+            binary_ext: if data.target.contains("windows") {
+                ".exe"
+            } else {
+                ""
+            },
+        }
+    }
+
+    pub(self) fn render_url(&self, template: &str) -> Result<Url, anyhow::Error> {
+        Ok(Url::parse(&self.render(template)?)?)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::{super::Data, Context};
+    use crate::{PkgFmt, PkgMeta};
+    use url::Url;
+
+    fn url(s: &str) -> Url {
+        Url::parse(s).unwrap()
+    }
+
+    #[test]
+    fn defaults() {
+        let meta = PkgMeta::default();
+        let data = Data {
+            name: "cargo-binstall".to_string(),
+            target: "x86_64-unknown-linux-gnu".to_string(),
+            version: "1.2.3".to_string(),
+            repo: Some("https://github.com/ryankurte/cargo-binstall".to_string()),
+            meta,
+        };
+
+        let ctx = Context::from_data(&data);
+        assert_eq!(
+            ctx.render_url(&data.meta.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")
+        );
+    }
+
+    #[test]
+    #[should_panic]
+    fn no_repo() {
+        let meta = PkgMeta::default();
+        let data = Data {
+            name: "cargo-binstall".to_string(),
+            target: "x86_64-unknown-linux-gnu".to_string(),
+            version: "1.2.3".to_string(),
+            repo: None,
+            meta,
+        };
+
+        let ctx = Context::from_data(&data);
+        ctx.render_url(&data.meta.pkg_url).unwrap();
+    }
+
+    #[test]
+    fn no_repo_but_full_url() {
+        let mut meta = PkgMeta::default();
+        meta.pkg_url = format!("https://example.com{}", meta.pkg_url);
+        let data = Data {
+            name: "cargo-binstall".to_string(),
+            target: "x86_64-unknown-linux-gnu".to_string(),
+            version: "1.2.3".to_string(),
+            repo: None,
+            meta,
+        };
+
+        let ctx = Context::from_data(&data);
+        assert_eq!(
+            ctx.render_url(&data.meta.pkg_url).unwrap(),
+            url("https://example.com/releases/download/v1.2.3/cargo-binstall-x86_64-unknown-linux-gnu-v1.2.3.tgz")
+        );
+    }
+
+    #[test]
+    fn different_url() {
+        let mut meta = PkgMeta::default();
+        meta.pkg_url = "{ repo }/releases/download/v{ version }/sx128x-util-{ target }-v{ version }.{ archive-format }".to_string();
+        let data = Data {
+            name: "radio-sx128x".to_string(),
+            target: "x86_64-unknown-linux-gnu".to_string(),
+            version: "0.14.1-alpha.5".to_string(),
+            repo: Some("https://github.com/rust-iot/rust-radio-sx128x".to_string()),
+            meta,
+        };
+
+        let ctx = Context::from_data(&data);
+        assert_eq!(
+            ctx.render_url(&data.meta.pkg_url).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")
+        );
+    }
+
+    #[test]
+    fn deprecated_format() {
+        let mut meta = PkgMeta::default();
+        meta.pkg_url = "{ repo }/releases/download/v{ version }/sx128x-util-{ target }-v{ version }.{ format }".to_string();
+        let data = Data {
+            name: "radio-sx128x".to_string(),
+            target: "x86_64-unknown-linux-gnu".to_string(),
+            version: "0.14.1-alpha.5".to_string(),
+            repo: Some("https://github.com/rust-iot/rust-radio-sx128x".to_string()),
+            meta,
+        };
+
+        let ctx = Context::from_data(&data);
+        assert_eq!(
+            ctx.render_url(&data.meta.pkg_url).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")
+        );
+    }
+
+    #[test]
+    fn different_ext() {
+        let mut meta = PkgMeta::default();
+        meta.pkg_url =
+            "{ repo }/releases/download/v{ version }/{ name }-v{ version }-{ target }.tar.xz"
+                .to_string();
+        meta.pkg_fmt = PkgFmt::Txz;
+        let data = Data {
+            name: "cargo-watch".to_string(),
+            target: "aarch64-apple-darwin".to_string(),
+            version: "9.0.0".to_string(),
+            repo: Some("https://github.com/watchexec/cargo-watch".to_string()),
+            meta,
+        };
+
+        let ctx = Context::from_data(&data);
+        assert_eq!(
+            ctx.render_url(&data.meta.pkg_url).unwrap(),
+            url("https://github.com/watchexec/cargo-watch/releases/download/v9.0.0/cargo-watch-v9.0.0-aarch64-apple-darwin.tar.xz")
+        );
+    }
+
+    #[test]
+    fn no_archive() {
+        let mut meta = PkgMeta::default();
+        meta.pkg_url = "{ repo }/releases/download/v{ version }/{ name }-v{ version }-{ target }{ binary-ext }".to_string();
+        meta.pkg_fmt = PkgFmt::Bin;
+        let data = Data {
+            name: "cargo-watch".to_string(),
+            target: "aarch64-pc-windows-msvc".to_string(),
+            version: "9.0.0".to_string(),
+            repo: Some("https://github.com/watchexec/cargo-watch".to_string()),
+            meta,
+        };
+
+        let ctx = Context::from_data(&data);
+        assert_eq!(
+            ctx.render_url(&data.meta.pkg_url).unwrap(),
+            url("https://github.com/watchexec/cargo-watch/releases/download/v9.0.0/cargo-watch-v9.0.0-aarch64-pc-windows-msvc.exe")
+        );
+    }
+}

src/lib.rs

@@ -17,10 +17,10 @@ pub const TARGET: &'static str = env!("TARGET");
 
 /// Default package path template (may be overridden in package Cargo.toml)
 pub const DEFAULT_PKG_URL: &'static str =
-    "{ repo }/releases/download/v{ version }/{ name }-{ target }-v{ version }.{ format }";
+    "{ repo }/releases/download/v{ version }/{ name }-{ target }-v{ version }.{ archive-format }";
 
 /// Default binary name template (may be overridden in package Cargo.toml)
-pub const DEFAULT_BIN_PATH: &'static str = "{ name }-{ target }-v{ version }/{ bin }{ format }";
+pub const DEFAULT_BIN_DIR: &'static str = "{ name }-{ target }-v{ version }/{ bin }{ binary-ext }";
 
 /// Binary format enumeration
 #[derive(
@@ -83,7 +83,7 @@ impl Default for PkgMeta {
         Self {
             pkg_url: DEFAULT_PKG_URL.to_string(),
             pkg_fmt: PkgFmt::default(),
-            bin_dir: DEFAULT_BIN_PATH.to_string(),
+            bin_dir: DEFAULT_BIN_DIR.to_string(),
             pub_key: None,
             overrides: HashMap::new(),
         }
@@ -165,7 +165,7 @@ mod test {
 
         assert_eq!(
             &meta.pkg_url,
-            "{ repo }/releases/download/v{ version }/{ name }-{ target }.{ format }"
+            "{ repo }/releases/download/v{ version }/{ name }-{ target }.{ archive-format }"
         );
 
         assert_eq!(