Breaking: move existing functionality to 'remodel run'

CHANGELOG.md

@@ -1,12 +1,15 @@
 # Remodel Changelog
 
 ## Unreleased Changes
+* **Breaking**: Moved script execution to `remodel run` to make room for new subcommands.
+	* If you previously used `remodel foo.lua`, use `remodel run foo.lua` now.
 * Added `json.fromString` and `json.toString` for encoding/decoding JSON
 * Added `remodel.isFile` and `remodel.isDir`.
-* Added (experimental) support for building Rojo projects through `rojo.buildProject`.
 * Added support for reading the auth cookie through the `REMODEL_AUTH` environment variable.
 * Added support for Remodel looking for scripts in the `.remodel` folder of a project
-	* `remodel foo` will look for `foo` as well as `.remodel/foo.lua` now!
+	* `remodel run foo` will now run `.remodel/foo.lua` if it exists.
+* Added (experimental) support for building Rojo projects through `rojo.buildProject`.
+	* This is behind the `unstable_rojo_api` Cargo feature and is not enabled by default.
 
 ## 0.6.1 (2019-12-11)
 * Upgraded reflection database and dependencies.

Cargo.toml

@@ -24,7 +24,7 @@ rbx_xml = "0.11.2"
 rbx_reflection = "3.2.399"
 reqwest = "0.9.20"
 rlua = "0.17.0"
-structopt = "0.3.1"
+structopt = "0.3.11"
 serde = "1.0.104"
 serde_json = "1.0.44"
 rojo = { git = "https://github.com/rojo-rbx/rojo.git", rev = "build-api", optional = true }

README.md

@@ -10,23 +10,24 @@ Remodel is still in early development, but much of its API is already fairly sta
 
 ## Installation
 
+### With [Foreman](https://github.com/rojo-rbx/foreman)
+Remodel can be installed with Foreman, a toolchain manager for Roblox projects:
+
+```toml
+[tools]
+remodel = { source = "rojo-rbx/remodel", version = "0.6.1" }
+```
+
 ### From GitHub Releases
-You can download pre-built Windows and macOS binaries from [Remodel's GitHub Releases page](https://github.com/rojo-rbx/remodel/releases).
+You can download pre-built binaries from [Remodel's GitHub Releases page](https://github.com/rojo-rbx/remodel/releases).
 
 ### From crates.io
-You'll need Rust 1.37+.
+You'll need Rust 1.37.0 or newer.
 
 ```bash
 cargo install remodel
 ```
 
-### Latest development changes (unstable!!)
-You'll need Rust 1.37+.
-
-```bash
-cargo install --git https://github.com/rojo-rbx/remodel
-```
-
 ## Quick Start
 Most of Remodel's interface is its Lua API. Users write Lua 5.3 scripts that Remodel runs, providing them with a special set of APIs.
 
@@ -309,14 +310,16 @@ _|WARNING:-DO-NOT-SHARE-THIS.--Sharing-this-will-allow-someone-to-log-in-as-you-
 
 **Auth cookies are very sensitive information! If you're using Remodel on a remote server like Travis CI or GitHub Actions, you should create a throwaway account with limited permissions in a group to ensure your valuable accounts are not compromised!**
 
-If you're on Windows, Remodel will attempt to use the cookie from a logged in Roblox Studio session to authenticate all requests.
+On Windows, Remodel will attempt to use the cookie from a logged in Roblox Studio session to authenticate all requests.
 
 To give a different auth cookie to Remodel, use the `--auth` argument:
 
 ```
 remodel foo.lua --auth "$MY_AUTH_COOKIE"
 ```
 
+You can also define the `REMODEL_AUTH` environment variable to avoid passing `--auth` as an argument.
+
 ## Remodel vs rbxmk
 Remodel is similar to [rbxmk](https://github.com/Anaminus/rbxmk):
 * Both Remodel and rbxmk use Lua

bin/test.sh

@@ -12,7 +12,7 @@ mkdir temp
 
 for script in test-scripts/*.lua; do
 	echo "Running $(basename "$script" .lua)"
-	./target/debug/remodel "$script"
+	./target/debug/remodel run "$script"
 done
 
 if [ ! -z "${REMODEL_AUTH_TESTS}" ]; then
@@ -22,6 +22,6 @@ if [ ! -z "${REMODEL_AUTH_TESTS}" ]; then
 
 	for script in test-scripts-extra/*.lua; do
 		echo "Running $(basename "$script" .lua)"
-		./target/debug/remodel "$script"
+		./target/debug/remodel run "$script"
 	done
 fi

src/main.rs

@@ -27,106 +27,135 @@ use crate::{
     about = env!("CARGO_PKG_DESCRIPTION"),
     author = env!("CARGO_PKG_AUTHORS"),
 )]
-struct Options {
-    /// The Lua 5.3 script to run. Pass `-` to read from stdin.
-    #[structopt(parse(from_os_str))]
-    script: PathBuf,
-
-    /// Arguments to pass to the script as a list of strings.
-    script_arguments: Vec<String>,
+pub struct Options {
+    #[structopt(subcommand)]
+    subcommand: Subcommand,
 
     /// The .ROBLOSECURITY cookie to use for authenticating to the Roblox API.
-    #[structopt(long = "auth", env = "REMODEL_AUTH", hide_env_values = true)]
+    ///
+    /// Remodel will attempt to use an existing session from Roblox Studio on
+    /// Windows if it is installed and you are logged in.
+    ///
+    /// Can also be passed via the REMODEL_AUTH environment variable.
+    #[structopt(
+        long = "auth",
+        env = "REMODEL_AUTH",
+        hide_env_values = true,
+        global = true
+    )]
     auth_cookie: Option<String>,
 }
 
+#[derive(Debug, StructOpt)]
+enum Subcommand {
+    /// Run a Lua 5.3 script by path or defined in a .remodel directory.
+    ///
+    /// Additional arguments are passed to the script being run.
+    Run {
+        /// Name of .remodel script or path to a script to run.
+        ///
+        /// Pass `-` to read a script from stdin.
+        script: String,
+
+        /// Arguments to pass to the script as a list of strings.
+        args: Vec<String>,
+    },
+}
+
 fn start() -> Result<(), Box<dyn Error>> {
     env_logger::from_env(env_logger::Env::default().default_filter_or("warn")).init();
 
     let opt = Options::from_args();
-    let (contents, chunk_name) = load_script(&opt.script)?;
-
-    let lua = Lua::new();
+    let auth_cookie = opt.auth_cookie.or_else(get_auth_cookie);
 
-    lua.context(move |context| {
-        let lua_args = opt
-            .script_arguments
-            .into_iter()
-            .map(|value| value.to_lua(context))
-            .collect::<Result<Vec<_>, _>>()?;
+    match opt.subcommand {
+        Subcommand::Run { script, args } => {
+            let (contents, chunk_name) = load_script(&script)?;
+            let lua = Lua::new();
 
-        let auth_cookie = opt.auth_cookie.or_else(get_auth_cookie);
+            lua.context(move |context| {
+                let lua_args = args
+                    .into_iter()
+                    .skip(1)
+                    .map(|value| value.to_lua(context))
+                    .collect::<Result<Vec<_>, _>>()?;
 
-        RemodelContext::new(auth_cookie).inject(context)?;
+                RemodelContext::new(auth_cookie).inject(context)?;
 
-        RemodelApi::inject(context)?;
-        RobloxApi::inject(context)?;
+                RemodelApi::inject(context)?;
+                RobloxApi::inject(context)?;
 
-        #[cfg(feature = "unstable_rojo_api")]
-        {
-            rojo_api::RojoApi::inject(context)?;
-        }
+                #[cfg(feature = "unstable_rojo_api")]
+                {
+                    rojo_api::RojoApi::inject(context)?;
+                }
 
-        let chunk = context.load(&contents).set_name(&chunk_name)?;
-        chunk.call(MultiValue::from_vec(lua_args))
-    })?;
+                let chunk = context.load(&contents).set_name(&chunk_name)?;
+                chunk.call(MultiValue::from_vec(lua_args))
+            })?;
 
-    Ok(())
+            Ok(())
+        }
+    }
 }
 
 /// Load the script from the given CLI-supplied path.
 ///
 /// Returns the contents of the script followed by its chunk name that should be
 /// given to Lua.
-fn load_script(script: &Path) -> io::Result<(String, String)> {
+fn load_script(script: &str) -> io::Result<(String, String)> {
     // Passing `-` indicates that the script should be read from stdin.
-    if script.as_os_str() == "-" {
+    if script == "-" {
         let mut contents = String::new();
         io::stdin().read_to_string(&mut contents)?;
 
         return Ok((contents, "stdin".to_owned()));
     }
 
-    log::trace!("Reading script from {}", script.display());
+    log::trace!("Reading script from {}", script);
 
     match fs::read_to_string(script) {
         // If the input is an exact file name that exists, we'll run that
         // script.
         Ok(contents) => {
-            let file_name = script.file_name().unwrap().to_string_lossy().into_owned();
+            let file_name = Path::new(script)
+                .file_name()
+                .unwrap()
+                .to_string_lossy()
+                .into_owned();
 
             Ok((contents, file_name))
         }
 
-        Err(err) => {
-            // If we couldn't find the exact name we're looking for, but a
-            // script with that name inside a folder named `.remodel/` exists,
-            // we'll run that!
-            if script.is_relative() {
-                if let Some(command_name) = script.file_name().and_then(|name| name.to_str()) {
-                    let mut command_path = PathBuf::from(".remodel");
-                    command_path.push(format!("{}.lua", command_name));
+        Err(full_path_err) => {
+            // If the given script was not a file that exists, we'll also try to
+            // search for it in `.remodel/<script>.lua`.
+            if full_path_err.kind() == io::ErrorKind::NotFound {
+                // If the script contains path-like components, the user
+                // definitely meant it as a path. To avoid path traversal
+                // issues, we won't try to check `.remodel/`.
+                if script.contains('/') || script.contains('\\') {
+                    return Err(full_path_err);
+                }
 
-                    log::trace!("Reading script from {}", command_path.display());
+                let mut remodel_path = PathBuf::from(".remodel");
+                remodel_path.push(format!("{}.lua", script));
 
-                    match fs::read_to_string(command_path) {
-                        Ok(contents) => {
-                            return Ok((contents, command_name.to_owned()));
-                        }
+                log::trace!("Reading script from {}", remodel_path.display());
 
-                        // If we got any IO errors that weren't 'file not
-                        // found', we'll report those. Otherwise, reporting an
-                        // error with this file path might confuse the user.
-                        Err(err) => {
-                            if err.kind() != io::ErrorKind::NotFound {
-                                return Err(err);
-                            }
+                match fs::read_to_string(remodel_path) {
+                    Ok(contents) => Ok((contents, script.to_owned())),
+                    Err(remodel_err) => {
+                        if remodel_err.kind() == io::ErrorKind::NotFound {
+                            Err(full_path_err)
+                        } else {
+                            Err(remodel_err)
                         }
                     }
                 }
+            } else {
+                Err(full_path_err)
             }
-
-            Err(err)
         }
     }
 }