Fixup clap args (Jake-Shadle#38)

* Clap 3 by default doesn't add author/version/about metadata

* Change option to `--manifest-version`

* Update CHANGELOG

* Add snapshot testing for CLI

* Actually run CLI tests in CI

* Update snapshots

* Force terminal width

.github/workflows/rust-ci.yml

@@ -79,6 +79,21 @@ jobs:
       - name: cargo test
         run: cargo test --release verify_deterministic
 
+  check_cli:
+    name: Verify CLI
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          override: true
+      - run: cargo fetch
+      - name: cargo test build
+        run: cargo build --tests
+      - name: cargo test
+        run: cargo test cli_help
+
   deny-check:
     name: cargo-deny
     runs-on: ubuntu-20.04

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Changed
+- [PR#37](https://github.com/Jake-Shadle/xwin/pull/37) changed from structopt to clap v3 for arguments parsing. Thanks [@messense](https://github.com/messense)!
+- [PR#38](https://github.com/Jake-Shadle/xwin/pull/38) fixed up the clap arguments to include metadata to be closer to the original structopt output with eg. `xwin -V`, however this exposed a problem that clap couldn't handle the old `--version <MANIFEST_VERSION>` flag since it clashed with `-V, --version`, so the flag has been renamed to `--manifest-version`. This is unfortunately a breaking change for the CLI.
+
 ## [0.1.10] - 2022-02-28
 ### Fixed
 - [PR#34](https://github.com/Jake-Shadle/xwin/pull/34) changed some code so that it is possible to compile and run for `x86_64-pc-windows-msvc`, though this target is not explicitly support. Thanks [@messense](https://github.com/messense)!

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "xwin"
 version = "0.1.10"
-description = "Allows downloading and repacking the MSVC and Windows SDK for cross compilation"
+description = "Allows downloading and repacking the MSVC CRT and Windows SDK for cross compilation"
 authors = ["Jake Shadle <jake.shadle@embark-studios.com>"]
 edition = "2018"
 license = "Apache-2.0 OR MIT"
@@ -70,5 +70,6 @@ twox-hash = "1.6"
 zip = { version = "0.5", default-features = false, features = ["deflate"] }
 
 [dev-dependencies]
+insta = "1.12"
 similar-asserts = "1.1"
 walkdir = "2.3"

src/main.rs

@@ -92,6 +92,7 @@ fn parse_level(s: &str) -> Result<LevelFilter, Error> {
 }
 
 #[derive(Parser)]
+#[clap(author, version, about, long_about = None)]
 pub struct Args {
     /// Doesn't display the prompt to accept the license
     #[clap(long, env = "XWIN_ACCEPT_LICENSE")]
@@ -119,12 +120,12 @@ pub struct Args {
     cache_dir: Option<PathBuf>,
     /// Specifies a VS manifest to use from a file, rather than downloading it
     /// from the Microsoft site.
-    #[clap(long, conflicts_with_all = &["version", "channel"])]
+    #[clap(long, conflicts_with_all = &["manifest-version", "channel"])]
     manifest: Option<PathBuf>,
     /// The version to retrieve, can either be a major version of 15 or 16, or
     /// a "<major>.<minor>" version.
     #[clap(long, default_value = "16")]
-    version: String,
+    manifest_version: String,
     /// The product channel to use.
     #[clap(long, default_value = "release")]
     channel: String,
@@ -347,13 +348,101 @@ fn load_manifest(
             serde_json::from_str(&manifest_content)
                 .with_context(|| format!("failed to deserialize manifest in '{}'", manifest_path))?
         }
-        None => {
-            xwin::manifest::get_manifest(ctx, &args.version, &args.channel, manifest_pb.clone())?
-        }
+        None => xwin::manifest::get_manifest(
+            ctx,
+            &args.manifest_version,
+            &args.channel,
+            manifest_pb.clone(),
+        )?,
     };
 
     let pkg_manifest = xwin::manifest::get_package_manifest(ctx, &manifest, manifest_pb.clone())?;
 
     manifest_pb.finish_with_message("📥 downloaded");
     Ok(pkg_manifest)
 }
+
+#[cfg(test)]
+mod test {
+    #[test]
+    fn cli_help() {
+        use clap::CommandFactory;
+
+        let snapshot_path = format!("{}/tests/snapshots", env!("CARGO_MANIFEST_DIR"));
+
+        insta::with_settings!({
+            prepend_module_to_snapshot => false,
+            snapshot_path => snapshot_path,
+        }, {
+            // the tests here will force maps to sort
+            snapshot_test_cli_command(super::Args::command().name(env!("CARGO_PKG_NAME")), "xwin".to_owned(), &SnapshotTestDesc {
+                manifest_path: env!("CARGO_MANIFEST_DIR"),
+                module_path: module_path!(),
+                file: file!(),
+                line: line!(),
+            });
+        });
+    }
+
+    use clap::{ColorChoice, Command};
+
+    pub struct SnapshotTestDesc {
+        pub manifest_path: &'static str,
+        pub module_path: &'static str,
+        pub file: &'static str,
+        pub line: u32,
+    }
+
+    fn snapshot_test_cli_command(app: Command<'_>, cmd_name: String, desc: &SnapshotTestDesc) {
+        let mut app = app
+            .clone()
+            // we do not want ASCII colors in our snapshot test output
+            .color(ColorChoice::Never)
+            // override versions to not have to update test when changing versions
+            .version("0.0.0")
+            .long_version("0.0.0")
+            .term_width(80);
+
+        // don't show current env vars as that will make snapshot test output diff depending on environment run in
+        let arg_names = app
+            .get_arguments()
+            .map(|a| a.get_id())
+            .filter(|a| *a != "version" && *a != "help")
+            .collect::<Vec<_>>();
+        for arg_name in arg_names {
+            app = app.mut_arg(arg_name, |arg| arg.hide_env_values(true));
+        }
+
+        // get the long help text for the command
+        let mut buffer = Vec::new();
+        app.write_long_help(&mut buffer).unwrap();
+        let help_text = std::str::from_utf8(&buffer).unwrap();
+
+        // use internal `insta` function instead of the macro so we can pass in the
+        // right module information from the crate and to gather up the errors instead of panicking directly on failures
+        insta::_macro_support::assert_snapshot(
+            cmd_name.clone().into(),
+            help_text,
+            desc.manifest_path,
+            "cli-cmd",
+            desc.module_path,
+            desc.file,
+            desc.line,
+            "help_text",
+        )
+        .unwrap();
+
+        // recursively test all subcommands
+        for app in app.get_subcommands() {
+            if app.get_name() == "help" {
+                continue;
+            }
+
+            snapshot_test_cli_command(
+                app.clone(),
+                format!("{}-{}", cmd_name, app.get_name()),
+                desc,
+            );
+        }
+    }
+}

tests/snapshots/xwin-download.snap

@@ -0,0 +1,20 @@
+---
+source: src/main.rs
+assertion_line: 382
+expression: help_text
+
+---
+download 0.0.0
+Downloads all the selected packages that aren't already present in the download
+cache
+
+USAGE:
+    download
+
+OPTIONS:
+    -h, --help
+            Print help information
+
+    -V, --version
+            Print version information
+

tests/snapshots/xwin-list.snap

@@ -0,0 +1,23 @@
+---
+source: src/main.rs
+assertion_line: 382
+expression: help_text
+
+---
+list 0.0.0
+Displays a summary of the packages that would be downloaded.
+
+Note that this is not a full list as the SDK uses MSI files for many packages,
+so they would need to be downloaded and inspected to determine which CAB files
+must also be downloaded to get the content needed.
+
+USAGE:
+    list
+
+OPTIONS:
+    -h, --help
+            Print help information
+
+    -V, --version
+            Print version information
+

tests/snapshots/xwin-splat.snap

@@ -0,0 +1,52 @@
+---
+source: src/main.rs
+assertion_line: 382
+expression: help_text
+
+---
+splat 0.0.0
+Fixes the packages to prune unneeded files and adds symlinks to address file
+casing issues and then spalts the final artifacts into directories
+
+USAGE:
+    splat [OPTIONS]
+
+OPTIONS:
+        --copy
+            Copies files from the unpack directory to the splat directory
+            instead of moving them, which preserves the original unpack
+            directories but increases overall time and disk usage
+
+        --disable-symlinks
+            By default, symlinks are added to both the CRT and WindowsSDK to
+            address casing issues in general usage. For example, if you are
+            compiling C/C++ code that does `#include <windows.h>`, it will break
+            on a case-sensitive file system, as the actual path in the
+            WindowsSDK is `Windows.h`. This also applies even if the C/C++ you
+            are compiling uses correct casing for all CRT/SDK includes, as the
+            internal headers also use incorrect casing in most cases
+
+    -h, --help
+            Print help information
+
+        --include-debug-libs
+            The MSVCRT includes (non-redistributable) debug versions of the
+            various libs that are generally uninteresting to keep for most usage
+
+        --include-debug-symbols
+            The MSVCRT includes PDB (debug symbols) files for several of the
+            libraries that are generally uninteresting to keep for most usage
+
+        --output <OUTPUT>
+            The root output directory. Defaults to `./.xwin-cache/splat` if not
+            specified
+
+        --preserve-ms-arch-notation
+            By default, we convert the MS specific `x64`, `arm`, and `arm64`
+            target architectures to the more canonical `x86_64`, `aarch`, and
+            `aarch64` of LLVM etc when creating directories/names. Passing this
+            flag will preserve the MS names for those targets
+
+    -V, --version
+            Print version information
+

tests/snapshots/xwin-unpack.snap

@@ -0,0 +1,19 @@
+---
+source: src/main.rs
+assertion_line: 382
+expression: help_text
+
+---
+unpack 0.0.0
+Unpacks all of the downloaded packages to disk
+
+USAGE:
+    unpack
+
+OPTIONS:
+    -h, --help
+            Print help information
+
+    -V, --version
+            Print version information
+