From 7a871c38c8fa31ab3caa38e529dde4f6a8a3a8e3 Mon Sep 17 00:00:00 2001 From: Chuck Pierce Date: Thu, 21 Mar 2019 16:34:53 -0700 Subject: [PATCH 1/3] Remove autoshimming --- crates/notion-core/src/project.rs | 79 +---------------------------- crates/notion-core/src/session.rs | 2 +- crates/notion-core/src/tool/mod.rs | 7 +-- crates/notion-core/src/tool/npm.rs | 17 +------ crates/notion-core/src/tool/yarn.rs | 18 +------ src/command/pin.rs | 9 ---- 6 files changed, 8 insertions(+), 124 deletions(-) diff --git a/crates/notion-core/src/project.rs b/crates/notion-core/src/project.rs index d165853cf..61ec9ccb6 100644 --- a/crates/notion-core/src/project.rs +++ b/crates/notion-core/src/project.rs @@ -14,8 +14,7 @@ use crate::distro::node::{load_default_npm_version, NodeVersion}; use crate::error::ErrorDetails; use crate::manifest::{serial, Manifest}; use crate::platform::PlatformSpec; -use crate::shim; -use notion_fail::{throw, Fallible, NotionError, ResultExt}; +use notion_fail::{throw, Fallible, ResultExt}; fn is_node_root(dir: &Path) -> bool { dir.join("package.json").is_file() @@ -143,26 +142,6 @@ impl Project { Ok(false) } - /// Automatically shim the binaries of all direct dependencies of this project and - /// return a vector of any errors which occurred while doing so. - pub fn autoshim(&self) -> Vec { - let dependent_binaries = self.dependent_binary_names_fault_tolerant(); - let mut errors = Vec::new(); - - for result in dependent_binaries { - match result { - Ok(name) => { - if let Err(error) = shim::create(&name) { - errors.push(error); - } - } - Err(error) => errors.push(error), - } - } - - errors - } - /// Returns a mapping of the names to paths for all the binaries installed /// by direct dependencies of the current project. fn dependent_binaries(&self) -> Fallible> { @@ -185,33 +164,6 @@ impl Project { Ok(dependent_bins) } - /// Gets the names of the binaries of all direct dependencies and returns them along - /// with any errors which occurred while doing so. - fn dependent_binary_names_fault_tolerant(&self) -> Vec> { - let mut results = Vec::new(); - let dependencies = &self.manifest.merged_dependencies(); - let dependency_paths = dependencies - .iter() - .map(|name| self.get_dependency_path(name)); - - for dependency_path in dependency_paths { - match Manifest::for_dir(&dependency_path) { - Ok(dependency) => { - for (name, _path) in dependency.bin { - results.push(Result::Ok(name.clone())) - } - } - Err(error) => { - if !error.to_string().contains("directory does not exist") { - results.push(Result::Err(error)) - } - } - } - } - - results - } - /// Convert dependency names to the path to each project. fn get_dependency_path(&self, name: &String) -> PathBuf { // ISSUE(158): Add support for Yarn Plug'n'Play. @@ -287,7 +239,7 @@ impl Project { #[cfg(test)] pub mod tests { - use std::collections::{HashMap, HashSet}; + use std::collections::HashMap; use std::ffi::OsStr; use std::path::PathBuf; @@ -316,33 +268,6 @@ pub mod tests { assert_eq!(dep_bins, expected_bins); } - #[test] - fn gets_binary_names() { - let project = Project::for_dir(&fixture_path("basic")).unwrap().unwrap(); - let binary_names = project.dependent_binary_names_fault_tolerant(); - let mut expected = HashSet::new(); - - expected.insert("eslint".to_string()); - expected.insert("rsvp".to_string()); - expected.insert("bin-1".to_string()); - expected.insert("bin-2".to_string()); - - let mut iterator = binary_names.iter(); - let mut actual = HashSet::new(); - - while let Some(fallible) = iterator.next() { - match fallible { - Ok(binary_name) => { - actual.insert(binary_name.clone()); - } - - Err(error) => panic!("encountered error {:?}", error), - } - } - - assert_eq!(actual, expected); - } - #[test] fn local_bin_true() { let project_path = fixture_path("basic"); diff --git a/crates/notion-core/src/session.rs b/crates/notion-core/src/session.rs index f29f3411d..c9c8a42ad 100644 --- a/crates/notion-core/src/session.rs +++ b/crates/notion-core/src/session.rs @@ -5,7 +5,7 @@ use std::rc::Rc; use crate::distro::node::NodeVersion; -use crate::distro::package::{self, PackageVersion, UserTool}; +use crate::distro::package::{PackageVersion, UserTool}; use crate::distro::Fetched; use crate::error::ErrorDetails; use crate::hook::{HookConfig, LazyHookConfig, Publish}; diff --git a/crates/notion-core/src/tool/mod.rs b/crates/notion-core/src/tool/mod.rs index 4892e7d47..fea94ed45 100644 --- a/crates/notion-core/src/tool/mod.rs +++ b/crates/notion-core/src/tool/mod.rs @@ -11,9 +11,8 @@ use std::process::{Command, ExitStatus}; use crate::env::UNSAFE_GLOBAL; use crate::error::ErrorDetails; use crate::session::Session; -use crate::style; use crate::version::VersionSpec; -use notion_fail::{Fallible, NotionError, ResultExt}; +use notion_fail::{Fallible, ResultExt}; mod binary; mod node; @@ -27,10 +26,6 @@ use self::npm::Npm; use self::npx::Npx; use self::yarn::Yarn; -fn display_tool_error(err: &NotionError) { - style::display_error(style::ErrorContext::Shim, err); -} - pub enum ToolSpec { Node(VersionSpec), Yarn(VersionSpec), diff --git a/crates/notion-core/src/tool/npm.rs b/crates/notion-core/src/tool/npm.rs index 0eccbd8c8..bbe50de31 100644 --- a/crates/notion-core/src/tool/npm.rs +++ b/crates/notion-core/src/tool/npm.rs @@ -1,9 +1,8 @@ use std::env::{args_os, ArgsOs}; use std::ffi::{OsStr, OsString}; -use std::io; -use std::process::{Command, ExitStatus}; +use std::process::Command; -use super::{command_for, display_tool_error, intercept_global_installs, Tool}; +use super::{command_for, intercept_global_installs, Tool}; use crate::error::ErrorDetails; use crate::session::{ActivityKind, Session}; @@ -46,18 +45,6 @@ impl Tool for Npm { fn command(self) -> Command { self.0 } - - fn finalize(session: &Session, maybe_status: &io::Result) { - if let Ok(_) = maybe_status { - if let Ok(Some(project)) = session.project() { - let errors = project.autoshim(); - - for error in errors { - display_tool_error(&error); - } - } - } - } } fn is_global_npm_install() -> bool { diff --git a/crates/notion-core/src/tool/yarn.rs b/crates/notion-core/src/tool/yarn.rs index db6024ce8..e140e0b01 100644 --- a/crates/notion-core/src/tool/yarn.rs +++ b/crates/notion-core/src/tool/yarn.rs @@ -1,9 +1,8 @@ use std::env::{args_os, ArgsOs}; use std::ffi::OsStr; -use std::io; -use std::process::{Command, ExitStatus}; +use std::process::Command; -use super::{command_for, display_tool_error, intercept_global_installs, Tool}; +use super::{command_for, intercept_global_installs, Tool}; use crate::error::ErrorDetails; use crate::session::{ActivityKind, Session}; @@ -43,19 +42,6 @@ impl Tool for Yarn { fn command(self) -> Command { self.0 } - - /// Perform any tasks which must be run after the tool runs but before exiting. - fn finalize(session: &Session, maybe_status: &io::Result) { - if let Ok(_) = maybe_status { - if let Ok(Some(project)) = session.project() { - let errors = project.autoshim(); - - for error in errors { - display_tool_error(&error); - } - } - } - } } fn is_global_yarn_add() -> bool { diff --git a/src/command/pin.rs b/src/command/pin.rs index b08a2a43b..bd3c7b1c3 100644 --- a/src/command/pin.rs +++ b/src/command/pin.rs @@ -2,7 +2,6 @@ use structopt::StructOpt; use notion_core::error::ErrorDetails; use notion_core::session::{ActivityKind, Session}; -use notion_core::style::{display_error, ErrorContext}; use notion_core::tool::ToolSpec; use notion_core::version::VersionSpec; use notion_fail::{throw, ExitCode, Fallible}; @@ -37,14 +36,6 @@ impl Command for Pin { ToolSpec::Package(_name, _version) => throw!(ErrorDetails::CannotPinPackage), } - if let Some(project) = session.project()? { - let errors = project.autoshim(); - - for error in errors { - display_error(ErrorContext::Notion, &error); - } - } - session.add_event_end(ActivityKind::Pin, ExitCode::Success); Ok(ExitCode::Success) } From 27aa12a4c3deac47ec2c740651439656cf95884f Mon Sep 17 00:00:00 2001 From: Chuck Pierce Date: Thu, 21 Mar 2019 16:38:16 -0700 Subject: [PATCH 2/3] Remove now-unused finalize method --- crates/notion-core/src/tool/mod.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/notion-core/src/tool/mod.rs b/crates/notion-core/src/tool/mod.rs index fea94ed45..f704ef805 100644 --- a/crates/notion-core/src/tool/mod.rs +++ b/crates/notion-core/src/tool/mod.rs @@ -130,14 +130,10 @@ pub trait Tool: Sized { /// Extracts the `Command` from this tool. fn command(self) -> Command; - /// Perform any tasks which must be run after the tool runs but before exiting. - fn finalize(_session: &Session, _maybe_status: &io::Result) {} - /// Delegates the current process to this tool. fn exec(self, session: &Session) -> Fallible { let mut command = self.command(); let status = command.status(); - Self::finalize(session, &status); status.with_context(binary_exec_error) } } From 251fc5d5ac325412e27a18295bc8a914d276ffc3 Mon Sep 17 00:00:00 2001 From: Chuck Pierce Date: Fri, 22 Mar 2019 11:09:38 -0700 Subject: [PATCH 3/3] Remove unused Session parameter in Tool exec method --- crates/notion-core/src/tool/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/notion-core/src/tool/mod.rs b/crates/notion-core/src/tool/mod.rs index f704ef805..bedbeedc3 100644 --- a/crates/notion-core/src/tool/mod.rs +++ b/crates/notion-core/src/tool/mod.rs @@ -102,10 +102,10 @@ pub fn execute_tool(session: &mut Session) -> Fallible { // all the possible `Tool` implementations and fill it dynamically, // as they have different sizes and associated types. match &exe.to_str() { - Some("node") => Node::new(args, session)?.exec(session), - Some("npm") => Npm::new(args, session)?.exec(session), - Some("npx") => Npx::new(args, session)?.exec(session), - Some("yarn") => Yarn::new(args, session)?.exec(session), + Some("node") => Node::new(args, session)?.exec(), + Some("npm") => Npm::new(args, session)?.exec(), + Some("npx") => Npx::new(args, session)?.exec(), + Some("yarn") => Yarn::new(args, session)?.exec(), _ => Binary::new( BinaryArgs { executable: exe, @@ -113,7 +113,7 @@ pub fn execute_tool(session: &mut Session) -> Fallible { }, session, )? - .exec(session), + .exec(), } } @@ -131,7 +131,7 @@ pub trait Tool: Sized { fn command(self) -> Command; /// Delegates the current process to this tool. - fn exec(self, session: &Session) -> Fallible { + fn exec(self) -> Fallible { let mut command = self.command(); let status = command.status(); status.with_context(binary_exec_error)