Add -V, clean up errors and help text (#259)

Co-authored-by: Jiahao XU <Jiahao_XU@outlook.com>

src/binstall/resolve.rs

@@ -85,7 +85,7 @@ pub async fn resolve(
     let mut version = match (&crate_name.version, &opts.version) {
         (Some(version), None) => version.to_string(),
         (None, Some(version)) => version.to_string(),
-        (Some(_), Some(_)) => Err(BinstallError::DuplicateVersionReq)?,
+        (Some(_), Some(_)) => Err(BinstallError::SuperfluousVersionOption)?,
         (None, None) => "*".to_string(),
     };
 
@@ -103,7 +103,7 @@ pub async fn resolve(
     // TODO: work out which of these to do based on `opts.name`
     // TODO: support git-based fetches (whole repo name rather than just crate name)
     let manifest = match opts.manifest_path.clone() {
-        Some(manifest_path) => load_manifest_path(manifest_path.join("Cargo.toml"))?,
+        Some(manifest_path) => load_manifest_path(manifest_path)?,
         None => {
             fetch_crate_cratesio(&client, &crates_io_api_client, &crate_name.name, &version).await?
         }

src/errors.rs

@@ -5,11 +5,19 @@ use miette::{Diagnostic, Report};
 use thiserror::Error;
 use tokio::task;
 
-/// Errors emitted by the library portion of cargo-binstall.
+/// Errors emitted by cargo-binstall.
 #[derive(Error, Diagnostic, Debug)]
 #[diagnostic(url(docsrs))]
 #[non_exhaustive]
 pub enum BinstallError {
+    /// Internal: a task could not be joined.
+    ///
+    /// - Code: `binstall::internal::task_join`
+    /// - Exit: 17
+    #[error(transparent)]
+    #[diagnostic(severity(error), code(binstall::internal::task_join))]
+    TaskJoinError(#[from] task::JoinError),
+
     /// The installation was cancelled by a user at a confirmation prompt.
     ///
     /// - Code: `binstall::user_abort`
@@ -96,6 +104,14 @@ pub enum BinstallError {
         err: crates_io_api::Error,
     },
 
+    /// The override path to the cargo manifest is invalid or cannot be resolved.
+    ///
+    /// - Code: `binstall::cargo_manifest_path`
+    /// - Exit: 77
+    #[error("the --manifest-path is invalid or cannot be resolved")]
+    #[diagnostic(severity(error), code(binstall::cargo_manifest_path))]
+    CargoManifestPath,
+
     /// A parsing or validation error in a cargo manifest.
     ///
     /// This should be rare, as manifests are generally fetched from crates.io, which does its own
@@ -167,29 +183,41 @@ pub enum BinstallError {
         v: semver::Version,
     },
 
-    /// This occurs when you specified `--version` while also using
+    /// The crate@version syntax was used at the same time as the --version option.
-    /// form `$crate_name@$ver` tp specify version requirements.
+    ///
-    #[error("duplicate version requirements")]
+    /// You can't do that as it's ambiguous which should apply.
+    ///
+    /// - Code: `binstall::conflict::version`
+    /// - Exit: 84
+    #[error("superfluous version specification")]
     #[diagnostic(
         severity(error),
-        code(binstall::version::requirement),
+        code(binstall::conflict::version),
-        help("Remove the `--version req` or simply use `$crate_name`")
+        help("You cannot use both crate@version and the --version option. Remove one.")
     )]
-    DuplicateVersionReq,
+    SuperfluousVersionOption,
 
-    /// This occurs when you specified `--manifest-path` while also
+    /// An override option is used when multiple packages are to be installed.
-    /// specifing multiple crates to install.
+    ///
-    #[error("If you use --manifest-path, then you can only specify one crate to install")]
+    /// This is raised when more than one package name is provided and any of:
+    ///
+    /// - `--version`
+    /// - `--manifest-path`
+    /// - `--bin-dir`
+    /// - `--pkg-fmt`
+    /// - `--pkg-url`
+    ///
+    /// is provided.
+    ///
+    /// - Code: `binstall::conflict::overrides`
+    /// - Exit: 85
+    #[error("override option used with multi package syntax")]
     #[diagnostic(
         severity(error),
-        code(binstall::manifest_path),
+        code(binstall::conflict::overrides),
-        help("Remove the `--manifest-path` or only specify one `$crate_name`")
+        help("You cannot use --{option} and specify multiple packages at the same time. Do one or the other.")
     )]
-    ManifestPathConflictedWithBatchInstallation,
+    OverrideOptionUsedWithMultiInstall { option: &'static str },
-
-    #[error("Failed to join tokio::task::JoinHandle")]
-    #[diagnostic(severity(error), code(binstall::join_error))]
-    TaskJoinError(#[from] task::JoinError),
 }
 
 impl BinstallError {
@@ -203,6 +231,7 @@ impl BinstallError {
     pub fn exit_code(&self) -> ExitCode {
         use BinstallError::*;
         let code: u8 = match self {
+            TaskJoinError(_) => 17,
             UserAbort => 32,
             UrlParse(_) => 65,
             Unzip(_) => 66,
@@ -211,14 +240,14 @@ impl BinstallError {
             Http { .. } => 69,
             Io(_) => 74,
             CratesIoApi { .. } => 76,
+            CargoManifestPath => 77,
             CargoManifest { .. } => 78,
             VersionParse { .. } => 80,
             VersionReq { .. } => 81,
             VersionMismatch { .. } => 82,
             VersionUnavailable { .. } => 83,
-            DuplicateVersionReq => 84,
+            SuperfluousVersionOption => 84,
-            ManifestPathConflictedWithBatchInstallation => 85,
+            OverrideOptionUsedWithMultiInstall { .. } => 85,
-            TaskJoinError(_) => 17,
         };
 
         // reserved codes

src/helpers.rs

@@ -88,7 +88,19 @@ pub fn load_manifest_path<P: AsRef<Path>>(
     manifest_path: P,
 ) -> Result<Manifest<Meta>, BinstallError> {
     block_in_place(|| {
-        debug!("Reading manifest: {}", manifest_path.as_ref().display());
+        let manifest_path = manifest_path.as_ref();
+        let manifest_path = if manifest_path.is_dir() {
+            manifest_path.join("Cargo.toml")
+        } else if manifest_path.is_file() {
+            manifest_path.into()
+        } else {
+            return Err(BinstallError::CargoManifestPath);
+        };
+
+        debug!(
+            "Reading manifest at local path: {}",
+            manifest_path.display()
+        );
 
         // Load and parse manifest (this checks file system for binary output names)
         let manifest = Manifest::<Meta>::from_path_with_metadata(manifest_path)?;

src/main.rs

@@ -22,18 +22,27 @@ static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;
 #[derive(Debug, Parser)]
 #[clap(version, about = "Install a Rust binary... from binaries!", setting = AppSettings::ArgRequiredElseHelp)]
 struct Options {
-    /// Package name for installation.
+    /// Packages to install.
     ///
-    /// This must be a crates.io package name.
+    /// Syntax: crate[@version]
-    #[clap(value_name = "crate")]
+    ///
+    /// Each value is either a crate name alone, or a crate name followed by @ and the version to
+    /// install. The version syntax is as with the --version option.
+    ///
+    /// When multiple names are provided, the --version option and any override options are
+    /// unavailable due to ambiguity.
+    #[clap(help_heading = "Package selection", value_name = "crate[@version]")]
     crate_names: Vec<CrateName>,
 
-    /// Semver filter to select the package version to install.
+    /// Package version to install.
     ///
-    /// This is in Cargo.toml dependencies format: `--version 1.2.3` is equivalent to
+    /// Takes either an exact semver version or a semver version requirement expression, which will
-    /// `--version "^1.2.3"`. Use `=1.2.3` to install a specific version.
+    /// be resolved to the highest matching version available.
-    #[clap(long)]
+    ///
-    version: Option<String>,
+    /// Cannot be used when multiple packages are installed at once, use the attached version
+    /// syntax in that case.
+    #[clap(help_heading = "Package selection", long = "version")]
+    version_req: Option<String>,
 
     /// Override binary target set.
     ///
@@ -48,18 +57,32 @@ struct Options {
     ///
     /// If falling back to installing from source, the first target will be used.
     #[clap(
-        help_heading = "OVERRIDES",
+        help_heading = "Package selection",
         alias = "target",
         long,
         value_name = "TRIPLE"
     )]
     targets: Option<String>,
 
-    /// Override install path for downloaded binary.
+    /// Override Cargo.toml package manifest path.
     ///
-    /// Defaults to `$HOME/.cargo/bin`
+    /// This skips searching crates.io for a manifest and uses the specified path directly, useful
-    #[clap(help_heading = "OVERRIDES", long)]
+    /// for debugging and when adding Binstall support. This may be either the path to the folder
-    install_path: Option<String>,
+    /// containing a Cargo.toml file, or the Cargo.toml file itself.
+    #[clap(help_heading = "Overrides", long)]
+    manifest_path: Option<PathBuf>,
+
+    /// Override Cargo.toml package manifest bin-dir.
+    #[clap(help_heading = "Overrides", long)]
+    bin_dir: Option<String>,
+
+    /// Override Cargo.toml package manifest pkg-fmt.
+    #[clap(help_heading = "Overrides", long)]
+    pkg_fmt: Option<PkgFmt>,
+
+    /// Override Cargo.toml package manifest pkg-url.
+    #[clap(help_heading = "Overrides", long)]
+    pkg_url: Option<String>,
 
     /// Disable symlinking / versioned updates.
     ///
@@ -68,21 +91,30 @@ struct Options {
     /// possible to have multiple versions of the same binary, for example for testing or rollback.
     ///
     /// Pass this flag to disable this behavior.
-    #[clap(long)]
+    #[clap(help_heading = "Options", long)]
     no_symlinks: bool,
 
     /// Dry run, fetch and show changes without installing binaries.
-    #[clap(long)]
+    #[clap(help_heading = "Options", long)]
     dry_run: bool,
 
     /// Disable interactive mode / confirmation prompts.
-    #[clap(long)]
+    #[clap(help_heading = "Options", long)]
     no_confirm: bool,
 
     /// Do not cleanup temporary files.
-    #[clap(long)]
+    #[clap(help_heading = "Options", long)]
     no_cleanup: bool,
 
+    /// Install binaries in a custom location.
+    ///
+    /// By default, binaries are installed to the global location `$CARGO_HOME/bin`, and global
+    /// metadata files are updated with the package information. Specifying another path here
+    /// switches over to a "local" install, where binaries are installed at the path given, and the
+    /// global metadata files are not updated.
+    #[clap(help_heading = "Options", long)]
+    install_path: Option<String>,
+
     /// Enforce downloads over secure transports only.
     ///
     /// Insecure HTTP downloads will be removed completely in the future; in the meantime this
@@ -91,41 +123,34 @@ struct Options {
     /// Without this option, plain HTTP will warn.
     ///
     /// Implies `--min-tls-version=1.2`.
-    #[clap(long)]
+    #[clap(help_heading = "Options", long)]
     secure: bool,
 
     /// Require a minimum TLS version from remote endpoints.
     ///
     /// The default is not to require any minimum TLS version, and use the negotiated highest
     /// version available to both this client and the remote server.
-    #[clap(long, arg_enum, value_name = "VERSION")]
+    #[clap(help_heading = "Options", long, arg_enum, value_name = "VERSION")]
     min_tls_version: Option<TLSVersion>,
 
-    /// Override manifest source.
+    /// Print help information
-    ///
+    #[clap(help_heading = "Meta", short, long)]
-    /// This skips searching crates.io for a manifest and uses the specified path directly, useful
+    help: bool,
-    /// for debugging and when adding Binstall support. This must be the path to the folder
+
-    /// containing a Cargo.toml file, not the Cargo.toml file itself.
+    /// Print version information
-    #[clap(help_heading = "OVERRIDES", long)]
+    #[clap(help_heading = "Meta", short = 'V')]
-    manifest_path: Option<PathBuf>,
+    version: bool,
 
     /// Utility log level
     ///
     /// Set to `debug` when submitting a bug report.
-    #[clap(long, default_value = "info", value_name = "LEVEL")]
+    #[clap(
+        help_heading = "Meta",
+        long,
+        default_value = "info",
+        value_name = "LEVEL"
+    )]
     log_level: LevelFilter,
-
-    /// Override Cargo.toml package manifest bin-dir.
-    #[clap(help_heading = "OVERRIDES", long)]
-    bin_dir: Option<String>,
-
-    /// Override Cargo.toml package manifest pkg-fmt.
-    #[clap(help_heading = "OVERRIDES", long)]
-    pkg_fmt: Option<PkgFmt>,
-
-    /// Override Cargo.toml package manifest pkg-url.
-    #[clap(help_heading = "OVERRIDES", long)]
-    pkg_url: Option<String>,
 }
 
 enum MainExit {
@@ -199,15 +224,33 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
 
     // Load options
     let mut opts = Options::parse_from(args);
+
+    let crate_names = take(&mut opts.crate_names);
+    if crate_names.len() > 1 {
+        let option = if opts.version_req.is_some() {
+            "version"
+        } else if opts.manifest_path.is_some() {
+            "manifest-path"
+        } else if opts.bin_dir.is_some() {
+            "bin-dir"
+        } else if opts.pkg_fmt.is_some() {
+            "pkg-fmt"
+        } else if opts.pkg_url.is_some() {
+            "pkg-url"
+        } else {
+            ""
+        };
+
+        if option != "" {
+            return Err(BinstallError::OverrideOptionUsedWithMultiInstall { option }.into());
+        }
+    }
+
     let cli_overrides = PkgOverride {
         pkg_url: opts.pkg_url.take(),
         pkg_fmt: opts.pkg_fmt.take(),
         bin_dir: opts.bin_dir.take(),
     };
-    let crate_names = take(&mut opts.crate_names);
-    if crate_names.len() > 1 && opts.manifest_path.is_some() {
-        return Err(BinstallError::ManifestPathConflictedWithBatchInstallation.into());
-    }
 
     // Initialize reqwest client
     let client = create_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?;
@@ -264,7 +307,7 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
     let binstall_opts = Arc::new(binstall::Options {
         no_symlinks: opts.no_symlinks,
         dry_run: opts.dry_run,
-        version: opts.version.take(),
+        version: opts.version_req.take(),
         manifest_path: opts.manifest_path.take(),
         cli_overrides,
         desired_targets,