Merge pull request #77 from fornwall/cleanup-remove

Small cleanups

Author: fornwall

src/arguments.rs

@@ -164,7 +164,7 @@ impl Args {
                 // "channel"
                 .short('c')
                 .num_args(1)
-                // FIXME: remove if benchmarking is stabilized
+                // Benchmarking currently requires nightly:
                 .conflicts_with("bench")
             );
 

src/main.rs

@@ -111,7 +111,7 @@ fn try_main() -> MainResult<i32> {
     let input = match (args.script.clone().unwrap(), args.expr, args.loop_) {
         (script, false, false) => {
             let (path, mut file) =
-                find_script(&script).ok_or(format!("could not find script: {}", script))?;
+                find_script(script.as_ref()).ok_or(format!("could not find script: {}", script))?;
 
             let script_name = path
                 .file_stem()
@@ -316,8 +316,8 @@ struct InputAction {
     */
     toolchain_version: Option<String>,
 
-    /// The package metadata structure for the current invocation.
-    metadata: PackageMetadata,
+    /// If script should be built in debug mode.
+    debug: bool,
 
     /// The package manifest contents.
     manifest: String,
@@ -342,7 +342,7 @@ impl InputAction {
     }
 
     fn cargo(&self, script_args: &[String]) -> MainResult<Command> {
-        let release_mode = !self.metadata.debug && !matches!(self.build_kind, BuildKind::Bench);
+        let release_mode = !self.debug && !matches!(self.build_kind, BuildKind::Bench);
 
         let built_binary_path = platform::binary_cache_path()
             .join(if release_mode { "release" } else { "debug" })
@@ -426,27 +426,6 @@ impl InputAction {
     }
 }
 
-/**
-The metadata here serves two purposes:
-
-1. It records everything necessary for compilation and execution of a package.
-2. It records everything that must be exactly the same in order for a cached executable to still be valid, in addition to the content hash.
-*/
-#[derive(Clone, Debug, Eq, PartialEq)]
-struct PackageMetadata {
-    /// Path to the script file.
-    path: Option<String>,
-
-    /// Was the script compiled in debug mode?
-    debug: bool,
-
-    /// Sorted list of dependencies.
-    deps: Vec<(String, String)>,
-
-    /// Sorted list of injected prelude items.
-    prelude: Vec<String>,
-}
-
 /**
 For the given input, this constructs the package metadata and checks the cache to see what should be done.
 */
@@ -460,12 +439,11 @@ fn decide_action_for(
         let deps_iter = deps.iter().map(|(n, v)| (n as &str, v as &str));
         input.compute_id(deps_iter)
     };
+    info!("id: {:?}", input_id);
 
     let pkg_name = input.package_name();
     let bin_name = format!("{}_{}", &*pkg_name, input_id.to_str().unwrap());
 
-    info!("id: {:?}", input_id);
-
     let (pkg_path, using_cache) = args
         .pkg_path
         .as_ref()
@@ -480,26 +458,12 @@ fn decide_action_for(
     let (mani_str, script_str) = manifest::split_input(input, &deps, &prelude, &bin_name)?;
 
     // Forcibly override some flags based on build kind.
-    let (debug, force) = match args.build_kind {
-        BuildKind::Normal => (args.debug, args.force),
-        BuildKind::Test => (true, false),
-        BuildKind::Bench => (false, false),
+    let debug = match args.build_kind {
+        BuildKind::Normal => args.debug,
+        BuildKind::Test => true,
+        BuildKind::Bench => false,
     };
 
-    let input_meta = {
-        let path = match input {
-            Input::File(_, path, _) => Some(path.to_string_lossy().into_owned()),
-            _ => None,
-        };
-        PackageMetadata {
-            path,
-            debug,
-            deps,
-            prelude,
-        }
-    };
-    info!("input_meta: {:?}", input_meta);
-
     let toolchain_version = args
         .toolchain_version
         .clone()
@@ -508,46 +472,23 @@ fn decide_action_for(
             _ => None,
         });
 
-    let mut action = InputAction {
+    Ok(InputAction {
         cargo_output: args.cargo_output,
-        force_compile: force,
-        execute: true,
+        force_compile: args.force,
+        execute: !args.gen_pkg_only,
         pkg_path,
         using_cache,
         toolchain_version,
-        metadata: input_meta,
+        debug,
         manifest: mani_str,
         script: script_str,
         build_kind: args.build_kind,
         bin_name,
-    };
-
-    // If we were told to only generate the package, we need to stop *now*
-    if args.gen_pkg_only {
-        action.execute = false;
-        return Ok(action);
-    }
-
-    // If we're not doing a regular build, stop.
-    match action.build_kind {
-        BuildKind::Normal => (),
-        BuildKind::Test | BuildKind::Bench => {
-            info!("not recompiling because: user asked for test/bench");
-            action.force_compile = false;
-            return Ok(action);
-        }
-    }
-
-    Ok(action)
+    })
 }
 
 /// Attempts to locate the script specified by the given path.
-fn find_script<P>(path: P) -> Option<(PathBuf, fs::File)>
-where
-    P: AsRef<Path>,
-{
-    let path = path.as_ref();
-
+fn find_script(path: &Path) -> Option<(PathBuf, fs::File)> {
     if let Ok(file) = fs::File::open(path) {
         return Some((path.into(), file));
     }

src/manifest.rs

@@ -52,7 +52,6 @@ pub fn split_input(
             || line.starts_with("pub async fn main(")
     }
 
-    let template_buf;
     let (part_mani, source, template, sub_prelude) = match input {
         Input::File(_, _, content) => {
             assert_eq!(prelude_items.len(), 0);
@@ -68,10 +67,10 @@ pub fn split_input(
             (manifest, source, templates::get_template("file")?, false)
         }
         Input::Expr(content) => {
-            template_buf = templates::get_template("expr")?;
-            let (manifest, template_src) = find_embedded_manifest(&template_buf)
-                .unwrap_or((Manifest::Toml(""), &template_buf));
-            (manifest, content.to_string(), template_src.into(), true)
+            let template_buf = templates::get_template("expr")?;
+            let (manifest, template_src) =
+                find_embedded_manifest(template_buf).unwrap_or((Manifest::Toml(""), template_buf));
+            (manifest, content.to_string(), template_src, true)
         }
         Input::Loop(content, count) => {
             let templ = if *count { "loop-count" } else { "loop" };
@@ -99,13 +98,10 @@ pub fn split_input(
         subs.insert(consts::SCRIPT_PRELUDE_SUB, &prelude_str[..]);
     }
 
-    let source = templates::expand(&template, &subs)?;
-
-    info!("part_mani: {:?}", part_mani);
-    info!("source: {:?}", source);
-
+    let source = templates::expand(template, &subs)?;
     let part_mani = part_mani.into_toml()?;
     info!("part_mani: {:?}", part_mani);
+    info!("source: {:?}", source);
 
     // It's-a mergin' time!
     let def_mani = default_manifest(input, bin_name)?;

src/templates.rs

@@ -5,7 +5,6 @@ use crate::consts;
 use crate::error::{MainError, MainResult};
 use lazy_static::lazy_static;
 use regex::Regex;
-use std::borrow::Cow;
 use std::collections::HashMap;
 
 lazy_static! {
@@ -49,19 +48,12 @@ pub fn expand(src: &str, subs: &HashMap<&str, &str>) -> MainResult<String> {
 /**
 Attempts to locate and load the contents of the specified template.
 */
-pub fn get_template(name: &str) -> MainResult<Cow<'static, str>> {
-    if let Some(text) = builtin_template(name) {
-        return Ok(text.into());
-    }
-    panic!("No such template: {name}");
-}
-
-fn builtin_template(name: &str) -> Option<&'static str> {
-    Some(match name {
+pub fn get_template(name: &str) -> MainResult<&'static str> {
+    Ok(match name {
         "expr" => consts::EXPR_TEMPLATE,
         "file" => consts::FILE_TEMPLATE,
         "loop" => consts::LOOP_TEMPLATE,
         "loop-count" => consts::LOOP_COUNT_TEMPLATE,
-        _ => return None,
+        _ => panic!("No such template: {name}"),
     })
 }