Fix #61, Fix #66: Generate valid .cmd and allow override of script ty…

…pe from CLI
rcook · Feb 21, 2024 · f3e01a9 · f3e01a9
1 parent 7963a03
commit f3e01a9
Show file tree

Hide file tree

Showing 11 changed files with 157 additions and 42 deletions.
diff --git a/isopy-go/src/go_plugin_factory.rs b/isopy-go/src/go_plugin_factory.rs
@@ -23,8 +23,8 @@ use crate::constants::{DOWNLOADS_URL, PLUGIN_NAME};
 use crate::go_plugin::GoPlugin;
 use crate::go_version::GoVersion;
 use isopy_lib::{
-    other_error as isopy_lib_other_error, Descriptor, EnvInfo, IsopyLibResult, Plugin,
-    PluginFactory,
+    other_error as isopy_lib_other_error, Descriptor, EnvInfo, IsopyLibResult, Platform, Plugin,
+    PluginFactory, Shell,
 };
 use reqwest::Url;
 use serde_json::Value;
@@ -67,7 +67,12 @@ impl PluginFactory for GoPluginFactory {
         todo!();
     }
 
-    fn make_script_command(&self, _script_path: &Path) -> IsopyLibResult<Option<OsString>> {
+    fn make_script_command(
+        &self,
+        _script_path: &Path,
+        _platform: Platform,
+        _shell: Shell,
+    ) -> IsopyLibResult<Option<OsString>> {
         todo!();
     }
 

diff --git a/isopy-java/src/java_plugin_factory.rs b/isopy-java/src/java_plugin_factory.rs
@@ -26,8 +26,8 @@ use crate::java_plugin::JavaPlugin;
 use crate::serialization::{EnvConfigRec, ProjectConfigRec};
 use anyhow::anyhow;
 use isopy_lib::{
-    other_error as isopy_lib_other_error, Descriptor, EnvInfo, IsopyLibResult, Plugin,
-    PluginFactory,
+    other_error as isopy_lib_other_error, Descriptor, EnvInfo, IsopyLibResult, Platform, Plugin,
+    PluginFactory, Shell,
 };
 use serde_json::Value;
 use std::ffi::OsString;
@@ -114,7 +114,12 @@ impl PluginFactory for JavaPluginFactory {
         })
     }
 
-    fn make_script_command(&self, _script_path: &Path) -> IsopyLibResult<Option<OsString>> {
+    fn make_script_command(
+        &self,
+        _script_path: &Path,
+        _platform: Platform,
+        _shell: Shell,
+    ) -> IsopyLibResult<Option<OsString>> {
         Ok(None)
     }
 

diff --git a/isopy-lib/src/lib.rs b/isopy-lib/src/lib.rs
@@ -59,7 +59,7 @@ pub use self::link_header::LinkHeader;
 pub use self::macros::TryToString;
 pub use self::package::Package;
 pub use self::plugin::Plugin;
-pub use self::plugin_factory::PluginFactory;
+pub use self::plugin_factory::{Platform, PluginFactory, Shell};
 pub use self::reqwest_response::ReqwestResponse;
 pub use self::response::{ContentLength, Response, Stream};
 pub use self::result::IsopyLibResult;

diff --git a/isopy-lib/src/plugin_factory.rs b/isopy-lib/src/plugin_factory.rs
@@ -26,6 +26,18 @@ use serde_json::Value;
 use std::ffi::OsString;
 use std::path::Path;
 
+#[derive(Clone, Copy, Debug)]
+pub enum Platform {
+    Unix,
+    Windows,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub enum Shell {
+    Bash,
+    Cmd,
+}
+
 pub trait PluginFactory: Send + Sync {
     fn name(&self) -> &str;
     fn source_url(&self) -> &Url;
@@ -37,6 +49,11 @@ pub trait PluginFactory: Send + Sync {
         props: &Value,
         base_dir: Option<&Path>,
     ) -> IsopyLibResult<EnvInfo>;
-    fn make_script_command(&self, script_path: &Path) -> IsopyLibResult<Option<OsString>>;
+    fn make_script_command(
+        &self,
+        script_path: &Path,
+        platform: Platform,
+        shell: Shell,
+    ) -> IsopyLibResult<Option<OsString>>;
     fn make_plugin(&self, offline: bool, dir: &Path) -> Box<dyn Plugin>;
 }
diff --git a/isopy-python/src/python_plugin_factory.rs b/isopy-python/src/python_plugin_factory.rs
@@ -24,8 +24,8 @@ use crate::python_descriptor::PythonDescriptor;
 use crate::python_plugin::PythonPlugin;
 use crate::serialization::{EnvConfigRec, ProjectConfigRec};
 use isopy_lib::{
-    other_error as isopy_lib_other_error, Descriptor, EnvInfo, IsopyLibResult, Plugin,
-    PluginFactory,
+    other_error as isopy_lib_other_error, Descriptor, EnvInfo, IsopyLibResult, Platform, Plugin,
+    PluginFactory, Shell,
 };
 use serde_json::Value;
 use std::ffi::{OsStr, OsString};
@@ -103,13 +103,24 @@ impl PluginFactory for PythonPluginFactory {
         Ok(EnvInfo { path_dirs, vars })
     }
 
-    fn make_script_command(&self, script_path: &Path) -> IsopyLibResult<Option<OsString>> {
-        fn make_command(script_path: &Path) -> OsString {
+    fn make_script_command(
+        &self,
+        script_path: &Path,
+        _platform: Platform,
+        shell: Shell,
+    ) -> IsopyLibResult<Option<OsString>> {
+        fn make_command(script_path: &Path, shell: Shell) -> OsString {
+            let delimiter: &str = match shell {
+                Shell::Bash => "'",
+                Shell::Cmd => "\"",
+            };
+
             let mut s = OsString::new();
             s.push(PYTHON_BIN_FILE_NAME.as_os_str());
-            s.push(" '");
+            s.push(" ");
+            s.push(delimiter);
             s.push(script_path);
-            s.push("'");
+            s.push(delimiter);
             s
         }
 
@@ -119,7 +130,7 @@ impl PluginFactory for PythonPluginFactory {
             .as_ref()
             == Some(&*PYTHON_SCRIPT_EXT)
         {
-            Ok(Some(make_command(script_path)))
+            Ok(Some(make_command(script_path, shell)))
         } else {
             Ok(None)
         }

diff --git a/isopy/src/args.rs b/isopy/src/args.rs
@@ -23,7 +23,8 @@ use crate::env::{ISOPY_CACHE_DIR_ENV_NAME, ISOPY_LOG_LEVEL_ENV_NAME, ISOPY_OFFLI
 use crate::package_id::PackageId;
 use crate::wrapper_file_name::WrapperFileName;
 use clap::{ArgAction, Args as ClapArgs, Parser, Subcommand, ValueEnum};
-use clap_complete::Shell;
+use clap_complete::Shell as ClapCompleteShell;
+use isopy_lib::Shell as IsopyLibShell;
 use joat_repo::MetaId;
 use log::LevelFilter;
 use path_absolutize::Absolutize;
@@ -111,7 +112,7 @@ pub enum Command {
     #[command(name = "completions", about = "Generate shell completions")]
     Completions {
         #[arg(help = "Shell", long = "shell", value_enum)]
-        shell: Option<Shell>,
+        shell: Option<ClapCompleteShell>,
     },
 
     #[command(name = "env", about = "Environment commands")]
@@ -188,6 +189,9 @@ pub enum Command {
         #[arg(help = "Base directory", value_parser = parse_absolute_path)]
         base_dir: PathBuf,
 
+        #[arg(help = "Shell script type", short = 's', long = "shell")]
+        shell: Option<Shell>,
+
         // Reference: https://jwodder.github.io/kbits/posts/clap-bool-negate/
         #[arg(
             help = "Force overwrite of output file",
@@ -197,7 +201,7 @@ pub enum Command {
         )]
         force: bool,
 
-        #[arg(help = "Dot not force overwrite of output file", long = "no-force")]
+        #[arg(help = "Do not force overwrite of output file", long = "no-force")]
         _no_force: bool,
     },
 }
@@ -353,6 +357,24 @@ impl From<LogLevel> for LevelFilter {
     }
 }
 
+#[derive(Clone, Debug, ValueEnum)]
+pub enum Shell {
+    #[clap(name = "bash")]
+    Bash,
+
+    #[clap(name = "cmd")]
+    Cmd,
+}
+
+impl From<Shell> for IsopyLibShell {
+    fn from(value: Shell) -> Self {
+        match value {
+            Shell::Bash => Self::Bash,
+            Shell::Cmd => Self::Cmd,
+        }
+    }
+}
+
 fn parse_absolute_path(s: &str) -> Result<PathBuf, String> {
     PathBuf::from(s)
         .absolutize()

diff --git a/isopy/src/commands/wrap.rs b/isopy/src/commands/wrap.rs
@@ -25,6 +25,7 @@ use crate::fs::{ensure_file_executable_mode, is_executable_file};
 use crate::status::{return_success, return_user_error, Status};
 use crate::wrapper_file_name::WrapperFileName;
 use anyhow::{anyhow, bail, Result};
+use isopy_lib::{Platform, Shell};
 use joat_repo::DirInfo;
 use joatmon::safe_write_file;
 use log::info;
@@ -34,21 +35,25 @@ use std::ffi::OsString;
 use std::path::{Path, PathBuf};
 use tinytemplate::{format_unescaped, TinyTemplate};
 
-#[cfg(any(target_os = "linux", target_os = "macos"))]
-const WRAPPER_TEMPLATE: &str = r#"#!/bin/bash
+const BASH_WRAPPER_TEMPLATE: &str = r#"#!/bin/bash
 set -euo pipefail
 {path_env} \
 {vars}exec {command} "$@"
 "#;
 
-#[cfg(target_os = "windows")]
-const WRAPPER_TEMPLATE: &str = r#"@echo off
+const CMD_WRAPPER_TEMPLATE: &str = r#"@echo off
 setlocal
 {path_env}
 {vars}
-"{command}" %*
+{command} %*
 "#;
 
+#[cfg(any(target_os = "linux", target_os = "macos"))]
+const DEFAULT_SHELL: Shell = Shell::Bash;
+
+#[cfg(target_os = "windows")]
+const DEFAULT_SHELL: Shell = Shell::Cmd;
+
 #[derive(Serialize)]
 struct TemplateContext {
     path_env: String,
@@ -61,6 +66,7 @@ pub fn wrap(
     wrapper_file_name: &WrapperFileName,
     script_path: &Path,
     base_dir: &Path,
+    shell: Option<Shell>,
     force: bool,
 ) -> Result<Status> {
     let Some(dir_info) = app.find_dir_info(None)? else {
@@ -74,9 +80,16 @@ pub fn wrap(
         return_user_error!("could not get environment info");
     };
 
+    let shell = shell.unwrap_or(DEFAULT_SHELL);
+
+    let wrapper_template = match shell {
+        Shell::Bash => BASH_WRAPPER_TEMPLATE,
+        Shell::Cmd => CMD_WRAPPER_TEMPLATE,
+    };
+
     let mut template = TinyTemplate::new();
     template.set_default_formatter(&format_unescaped);
-    template.add_template("WRAPPER", WRAPPER_TEMPLATE)?;
+    template.add_template("WRAPPER", wrapper_template)?;
 
     let path_env = String::from(
         make_path_env(&env_info.path_dirs)?
@@ -91,7 +104,7 @@ pub fn wrap(
         .join("bin")
         .join(wrapper_file_name.as_os_str());
 
-    let command = make_script_command(&dir_info, script_path)?;
+    let command = make_script_command(&dir_info, script_path, Platform::Unix, shell)?;
 
     let s = template.render(
         "WRAPPER",
@@ -147,8 +160,13 @@ fn make_vars(vars: &[(String, String)]) -> String {
     inner(vars)
 }
 
-fn make_script_command(dir_info: &DirInfo, script_path: &Path) -> Result<String> {
-    if let Some(s) = dir_info.make_script_command(script_path)? {
+fn make_script_command(
+    dir_info: &DirInfo,
+    script_path: &Path,
+    platform: Platform,
+    shell: Shell,
+) -> Result<String> {
+    if let Some(s) = dir_info.make_script_command(script_path, platform, shell)? {
         return Ok(String::from(
             s.to_str()
                 .ok_or_else(|| anyhow!("cannot convert OS string"))?,

diff --git a/isopy/src/dir_info_ext.rs b/isopy/src/dir_info_ext.rs
@@ -23,7 +23,7 @@ use crate::constants::ENV_CONFIG_FILE_NAME;
 use crate::registry::Registry;
 use crate::serialization::EnvRec;
 use anyhow::Result;
-use isopy_lib::EnvInfo;
+use isopy_lib::{EnvInfo, Platform, Shell};
 use joat_repo::{DirInfo, Manifest};
 use joatmon::{read_yaml_file, safe_write_file};
 use std::ffi::OsString;
@@ -33,7 +33,12 @@ pub trait DirInfoExt {
     fn read_env_config(&self) -> Result<EnvRec>;
     fn write_env_config(&self, env_rec: &EnvRec, overwrite: bool) -> Result<()>;
     fn make_env_info(&self, base_dir: Option<&Path>) -> Result<Option<EnvInfo>>;
-    fn make_script_command(&self, script_path: &Path) -> Result<Option<OsString>>;
+    fn make_script_command(
+        &self,
+        script_path: &Path,
+        platform: Platform,
+        shell: Shell,
+    ) -> Result<Option<OsString>>;
 }
 
 impl DirInfoExt for DirInfo {
@@ -49,8 +54,13 @@ impl DirInfoExt for DirInfo {
         make_env_info(self.data_dir(), base_dir)
     }
 
-    fn make_script_command(&self, script_path: &Path) -> Result<Option<OsString>> {
-        make_script_command(self.data_dir(), script_path)
+    fn make_script_command(
+        &self,
+        script_path: &Path,
+        platform: Platform,
+        shell: Shell,
+    ) -> Result<Option<OsString>> {
+        make_script_command(self.data_dir(), script_path, platform, shell)
     }
 }
 
@@ -67,8 +77,13 @@ impl DirInfoExt for Manifest {
         make_env_info(self.data_dir(), base_dir)
     }
 
-    fn make_script_command(&self, script_path: &Path) -> Result<Option<OsString>> {
-        make_script_command(self.data_dir(), script_path)
+    fn make_script_command(
+        &self,
+        script_path: &Path,
+        platform: Platform,
+        shell: Shell,
+    ) -> Result<Option<OsString>> {
+        make_script_command(self.data_dir(), script_path, platform, shell)
     }
 }
 
@@ -110,11 +125,17 @@ fn make_env_info(data_dir: &Path, base_dir: Option<&Path>) -> Result<Option<EnvI
     Ok(Some(all_env_info))
 }
 
-fn make_script_command(data_dir: &Path, script_path: &Path) -> Result<Option<OsString>> {
+fn make_script_command(
+    data_dir: &Path,
+    script_path: &Path,
+    platform: Platform,
+    shell: Shell,
+) -> Result<Option<OsString>> {
     let env_rec = read_env_config(data_dir)?;
 
     for package_rec in &env_rec.packages {
-        let result = Registry::global().make_script_command(package_rec, script_path)?;
+        let result =
+            Registry::global().make_script_command(package_rec, script_path, platform, shell)?;
         if result.is_some() {
             return Ok(result);
         }

diff --git a/isopy/src/plugin_host.rs b/isopy/src/plugin_host.rs
@@ -19,7 +19,7 @@
 // OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 //
-use isopy_lib::{Descriptor, EnvInfo, IsopyLibResult, Plugin, PluginFactory};
+use isopy_lib::{Descriptor, EnvInfo, IsopyLibResult, Platform, Plugin, PluginFactory, Shell};
 use serde_json::Value;
 use std::ffi::OsString;
 use std::path::Path;
@@ -72,8 +72,14 @@ impl PluginFactory for PluginHost {
         self.plugin_factory.make_env_info(data_dir, props, base_dir)
     }
 
-    fn make_script_command(&self, script_path: &Path) -> IsopyLibResult<Option<OsString>> {
-        self.plugin_factory.make_script_command(script_path)
+    fn make_script_command(
+        &self,
+        script_path: &Path,
+        platform: Platform,
+        shell: Shell,
+    ) -> IsopyLibResult<Option<OsString>> {
+        self.plugin_factory
+            .make_script_command(script_path, platform, shell)
     }
 
     fn make_plugin(&self, offline: bool, dir: &Path) -> Box<dyn Plugin> {