Merge pull request #579 from immunant/kkysen/c2rust-forward: Make `c2…

…rust` just forward args to discovered subcommands

This changes `c2rust` to be a simpler wrapper around the other `c2rust-*` subcommand.

Instead of `c2rust` having to know about all the subcommands and their arguments upfront, `c2rust $subcommand $args` just runs `c2rust-$subcommand $args`.  This allows `c2rust instrument` to work as before (before #554), while also enabling `c2rust analyze` and `c2rust pdg` in the same way.  The `clap` help messages are still preserved for the most part, except for the short help messages for the subcommands.  Otherwise, `c2rust --help` works as before (while also suggesting the new subcommands), and `c2rust $subcommand --help` works by running `c2rust-$subcommand --help` (instead of `clap` intercepting the `--help`).

The way this is implemented is, first the `c2rust` binary's directory is searched for executables named `c2rust-*` to discover subcommands.  This is combined with the simple list of known subcommands (`["transpile", "instrument", "pdg", "analyze"]`) in case they're not discovered properly and we still want to suggest them.  Then we check if the first argument is one of these subcommands.  If it exists, we invoke it.  If it doesn't exist, but is known, we suggest building it, and it doesn't exist and isn't known (or there was no subcommand given), then we run the `clap` parser and let it handle arg parsing and nice error/help messages.  The reason we don't have everything go through `clap` is that I couldn't figure out a way to have `clap` just forward all arguments, even ones like `--metadata` with hyphens (`Arg::allow_hyphen_values` didn't work), without requiring a leading `--` argument.

c2rust/Cargo.toml

@@ -18,10 +18,12 @@ azure-devops = { project = "immunant/c2rust", pipeline = "immunant.c2rust", buil
 
 [dependencies]
 anyhow = "1.0"
-clap = {version = "2.34", features = ["yaml"]}
-log = "0.4"
+camino = "1.0"
+clap = { version = "2.34", features = ["yaml"] }
 env_logger = "0.9"
 git-testament = "0.2.1"
+is_executable = "1.0"
+log = "0.4"
 regex = "1.3"
 shlex = "1.1"
 c2rust-transpile = { version = "0.16.0", path = "../c2rust-transpile" }

c2rust/src/main.rs

@@ -1,56 +1,147 @@
-use clap::{crate_authors, load_yaml, App, AppSettings, SubCommand};
+use anyhow::anyhow;
+use camino::Utf8Path;
+use clap::{crate_authors, App, AppSettings, Arg};
 use git_testament::{git_testament, render_testament};
+use is_executable::IsExecutable;
+use std::borrow::Cow;
+use std::collections::HashMap;
 use std::env;
 use std::ffi::OsStr;
-use std::process::{exit, Command};
+use std::path::{Path, PathBuf};
+use std::process;
+use std::process::Command;
+use std::str;
 
 git_testament!(TESTAMENT);
 
-fn main() {
-    let subcommand_yamls = [load_yaml!("transpile.yaml")];
-    let matches = App::new("C2Rust")
-        .version(&*render_testament!(TESTAMENT))
-        .author(crate_authors!(", "))
-        .setting(AppSettings::SubcommandRequiredElseHelp)
-        .subcommands(
-            subcommand_yamls
-                .iter()
-                .map(|yaml| SubCommand::from_yaml(yaml)),
-        )
-        .get_matches();
+/// A `c2rust` sub-command.
+struct SubCommand {
+    /// The path to the [`SubCommand`]'s executable,
+    /// if it was found (see [`Self::find_all`]).
+    /// Otherwise [`None`] if it is a known [`SubCommand`] (see [`Self::known`]).
+    path: Option<PathBuf>,
+    /// The name of the [`SubCommand`], i.e. in `c2rust-{name}`.
+    name: Cow<'static, str>,
+}
 
-    let mut os_args = env::args_os();
-    os_args.next(); // Skip executable name
-    let arg_name = os_args.next().and_then(|name| name.into_string().ok());
-    match (&arg_name, matches.subcommand_name()) {
-        (Some(arg_name), Some(subcommand)) if arg_name == subcommand => {
-            invoke_subcommand(subcommand, os_args);
-        }
-        _ => {
-            eprintln!("{:?}", arg_name);
-            panic!("Could not match subcommand");
+impl SubCommand {
+    /// Find all [`SubCommand`]s adjacent to the current (`c2rust`) executable.
+    /// They are of the form `c2rust-{name}`.
+    pub fn find_all() -> anyhow::Result<Vec<Self>> {
+        let c2rust = env::current_exe()?;
+        let c2rust_name: &Utf8Path = c2rust
+            .file_name()
+            .map(Path::new)
+            .ok_or_else(|| anyhow!("no file name: {}", c2rust.display()))?
+            .try_into()?;
+        let c2rust_name = c2rust_name.as_str();
+        let dir = c2rust
+            .parent()
+            .ok_or_else(|| anyhow!("no directory: {}", c2rust.display()))?;
+        let mut sub_commands = Vec::new();
+        for entry in dir.read_dir()? {
+            let entry = entry?;
+            let file_type = entry.file_type()?;
+            let path = entry.path();
+            let name = path
+                .file_name()
+                .and_then(|name| name.to_str())
+                .and_then(|name| name.strip_prefix(c2rust_name))
+                .and_then(|name| name.strip_prefix('-'))
+                .map(|name| name.to_owned())
+                .map(Cow::from)
+                .filter(|_| file_type.is_file() || file_type.is_symlink())
+                .filter(|_| path.is_executable());
+            if let Some(name) = name {
+                sub_commands.push(Self {
+                    path: Some(path),
+                    name,
+                });
+            }
         }
-    };
+        Ok(sub_commands)
+    }
+
+    /// Get all known [`SubCommand`]s.  These have no [`SubCommand::path`].
+    /// Even if the subcommand executables aren't there, we can still suggest them.
+    pub fn known() -> impl Iterator<Item = Self> {
+        ["transpile", "instrument", "pdg", "analyze"]
+            .into_iter()
+            .map(|name| Self {
+                path: None,
+                name: name.into(),
+            })
+    }
+
+    /// Get all known ([`Self::known`]) and actual, found ([`Self::find_all`]) subcommands,
+    /// putting the known ones first so that the found ones overwrite them and take precedence.
+    pub fn all() -> anyhow::Result<impl Iterator<Item = Self>> {
+        Ok(Self::known().chain(Self::find_all()?))
+    }
+
+    pub fn invoke<I, S>(&self, args: I) -> anyhow::Result<()>
+    where
+        I: IntoIterator<Item = S>,
+        S: AsRef<OsStr>,
+    {
+        let path = self.path.as_ref().ok_or_else(|| {
+            anyhow!(
+                "known subcommand not found (probably not built): {}",
+                self.name
+            )
+        })?;
+        let status = Command::new(&path).args(args).status()?;
+        process::exit(status.code().unwrap_or(1));
+    }
 }
 
-fn invoke_subcommand<I, S>(subcommand: &str, args: I)
-where
-    I: IntoIterator<Item = S>,
-    S: AsRef<OsStr>,
-{
-    // Assumes the subcommand executable is in the same directory as this driver
-    // program.
-    let cmd_path = std::env::current_exe().expect("Cannot get current executable path");
-    let mut cmd_path = cmd_path.as_path().canonicalize().unwrap();
-    cmd_path.pop(); // remove current executable
-    cmd_path.push(format!("c2rust-{}", subcommand));
-    assert!(cmd_path.exists(), "{:?} is missing", cmd_path);
-    exit(
-        Command::new(cmd_path.into_os_string())
-            .args(args)
-            .status()
-            .expect("SubCommand failed to start")
-            .code()
-            .unwrap_or(-1),
-    );
+fn main() -> anyhow::Result<()> {
+    let sub_commands = SubCommand::all()?.collect::<Vec<_>>();
+    let sub_commands = sub_commands
+        .iter()
+        .map(|cmd| (cmd.name.as_ref(), cmd))
+        .collect::<HashMap<_, _>>();
+
+    // If the subcommand matches, don't use `clap` at all.
+    //
+    // I can't seem to get `clap` to pass through all arguments as is,
+    // like the ones with hyphens like `--metadata`,
+    // even though I've set [`Arg::allow_hyphen_values`].
+    // This is faster anyways.
+    // I also tried a single "subcommand" argument with [`Arg::possible_values`],
+    // but that had the same problem passing through all arguments as well.
+    //
+    // Furthermore, doing it this way correctly forwards `--help` through to the subcommand
+    // instead of `clap` intercepting it and displaying the top-level `--help`.
+    let mut args = env::args_os();
+    let sub_command = args.nth(1);
+    let sub_command = sub_command
+        .as_ref()
+        .and_then(|arg| arg.to_str())
+        .and_then(|name| sub_commands.get(name));
+
+    if let Some(sub_command) = sub_command {
+        return sub_command.invoke(args);
+    }
+
+    // If we didn't get a subcommand, then use `clap` for parsing and error/help messages.
+    let matches = App::new("C2Rust")
+        .version(&*render_testament!(TESTAMENT))
+        .author(crate_authors!(", "))
+        .settings(&[
+            AppSettings::SubcommandRequiredElseHelp,
+            AppSettings::AllowExternalSubcommands,
+        ])
+        .subcommands(sub_commands.keys().map(|name| {
+            clap::SubCommand::with_name(name).arg(
+                Arg::with_name("args")
+                    .multiple(true)
+                    .allow_hyphen_values(true),
+            )
+        }))
+        .get_matches();
+    let sub_command_name = matches
+        .subcommand_name()
+        .ok_or_else(|| anyhow!("no subcommand"))?;
+    sub_commands[sub_command_name].invoke(args)
 }