diff --git a/.github/workflows/hipcheck.yml b/.github/workflows/hipcheck.yml index 8f881c1a..968e862b 100644 --- a/.github/workflows/hipcheck.yml +++ b/.github/workflows/hipcheck.yml @@ -17,7 +17,6 @@ on: - "config/**" - "hipcheck/**" - "plugins/**" - - "scripts/**" - "xtask/**" pull_request: branches: [main] @@ -25,7 +24,6 @@ on: - "config/**" - "hipcheck/**" - "plugins/**" - - "scripts/**" - "xtask/**" permissions: diff --git a/Cargo.toml b/Cargo.toml index 3be3506f..9cb2fe19 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ pr-run-mode = "plan" # Whether to install an updater program install-updater = true # Extra static files to include in each App (path relative to this Cargo.toml's dir) -include = ["config/", "scripts/"] +include = ["config/"] # Path that installers should place binaries in install-path = "CARGO_HOME" diff --git a/Containerfile b/Containerfile index 12334ebc..9fed4c14 100644 --- a/Containerfile +++ b/Containerfile @@ -26,7 +26,6 @@ WORKDIR /app COPY --from=builder /build/target/release/hc ./hc COPY config/ config/ -COPY scripts/ scripts/ RUN set -eux && \ apt-get update && \ @@ -39,7 +38,6 @@ RUN set -eux && \ USER hc_user ENV HC_CONFIG=./config -ENV HC_DATA=./scripts ENTRYPOINT ["./hc"] diff --git a/hipcheck/src/cli.rs b/hipcheck/src/cli.rs index b5986569..91614027 100644 --- a/hipcheck/src/cli.rs +++ b/hipcheck/src/cli.rs @@ -81,16 +81,6 @@ struct OutputArgs { /// Arguments configuring paths for Hipcheck to use. #[derive(Debug, Default, clap::Args, hc::Update)] struct PathArgs { - /// Path to the data folder. - #[arg( - short = 'd', - long = "data", - global = true, - help_heading = "Path Flags", - long_help = "Path to the data folder. Can also be set with the `HC_DATA` environment variable" - )] - data: Option, - /// Path to the cache folder. #[arg( short = 'C', @@ -131,10 +121,6 @@ struct DeprecatedArgs { #[arg(long = "print-config", hide = true, global = true)] print_config: Option, - /// Print the data folder path for Hipcheck. - #[arg(long = "print-data", hide = true, global = true)] - print_data: Option, - /// Path to the configuration folder. #[arg(short = 'c', long = "config", hide = true, global = true)] config: Option, @@ -164,10 +150,6 @@ impl CliConfig { /// Get the selected subcommand, if any. pub fn subcommand(&self) -> Option { - if self.print_data() { - return Some(FullCommands::PrintData); - } - if self.print_home() { return Some(FullCommands::PrintCache); } @@ -212,11 +194,6 @@ impl CliConfig { } } - /// Get the path to the data directory. - pub fn data(&self) -> Option<&Path> { - self.path_args.data.as_deref() - } - /// Get the path to the cache directory. pub fn cache(&self) -> Option<&Path> { match (&self.path_args.cache, &self.deprecated_args.home) { @@ -250,11 +227,6 @@ impl CliConfig { self.deprecated_args.print_config.unwrap_or(false) } - /// Check if the `--print-data` flag was used. - pub fn print_data(&self) -> bool { - self.deprecated_args.print_data.unwrap_or(false) - } - /// Get an empty configuration object with nothing set. /// /// This is just an alias for `default()`. @@ -280,7 +252,6 @@ impl CliConfig { format: hc_env_var_value_enum("format"), }, path_args: PathArgs { - data: hc_env_var("data"), cache: hc_env_var("cache"), // For now, we do not get this from the environment, so pass a None to never update this field policy: None, @@ -302,7 +273,6 @@ impl CliConfig { CliConfig { path_args: PathArgs { cache: platform_cache(), - data: platform_data(), // There is no central per-user or per-system location for the policy file, so pass a None to never update this field policy: None, }, @@ -318,7 +288,6 @@ impl CliConfig { fn backups() -> CliConfig { CliConfig { path_args: PathArgs { - data: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "data"]), cache: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "cache"]), // TODO: currently if this is set, then when running `hc check`, it errors out // because policy files are not yet supported @@ -347,7 +316,7 @@ fn platform_cache() -> Option { fn platform_config() -> Option { let base = dirs::config_dir().map(|dir| pathbuf![&dir, "hipcheck"]); - // Config and data paths aren't differentiated on MacOS or Windows, + // Config and (now unused) data paths aren't differentiated on MacOS or Windows, // so on those platforms we differentiate them ourselves. if cfg!(target_os = "macos") || cfg!(target_os = "windows") { base.map(|dir| pathbuf![&dir, "config"]) @@ -356,21 +325,6 @@ fn platform_config() -> Option { } } -/// Get the platform data directory. -/// -/// See: https://docs.rs/dirs/latest/dirs/fn.data_dir.html -fn platform_data() -> Option { - let base = dirs::data_dir().map(|dir| pathbuf![&dir, "hipcheck"]); - - // Config and data paths aren't differentiated on MacOS or Windows, - // so on those platforms we differentiate them ourselves. - if cfg!(target_os = "macos") || cfg!(target_os = "windows") { - base.map(|dir| pathbuf![&dir, "data"]) - } else { - base - } -} - /// Get a Hipcheck configuration environment variable. /// /// This is generic in the return type, to automatically handle @@ -400,7 +354,6 @@ pub enum FullCommands { Cache(CacheArgs), Plugin(PluginArgs), PrintConfig, - PrintData, PrintCache, Scoring, } @@ -428,13 +381,13 @@ pub enum Commands { Schema(SchemaArgs), /// Initialize Hipcheck config file and script file locations. /// - /// The "destination" directories for configuration and data files + /// The "destination" directories for configuration files /// Hipcheck needs are determined with the following methods, in /// increasing precedence: /// /// 1. Platform-specific defaults - /// 2. `HC_CONFIG` and `HC_DATA` environment variables - /// 3. `--config` and `--data` command line flags + /// 2. `HC_CONFIG` environment variable + /// 3. `--config` command line flag Setup(SetupArgs), /// Check if Hipcheck is ready to run. Ready, @@ -1158,82 +1111,6 @@ mod tests { }); } - #[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] - #[test] - fn resolve_data_with_platform() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HOME", Some(tempdir.path().to_str().unwrap())), - ("XDG_DATA_HOME", None), - ("HC_DATA", None), - ]; - - with_env_vars(vars, || { - let config = { - let mut temp = CliConfig::empty(); - temp.update(&CliConfig::from_platform()); - temp.update(&CliConfig::from_env()); - temp - }; - - assert_eq!(config.data().unwrap(), platform_data().unwrap()); - }); - } - - #[test] - fn resolve_data_with_env_var() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HOME", None), - ("XDG_DATA_HOME", None), - ("HC_DATA", Some(tempdir.path().to_str().unwrap())), - ]; - - with_env_vars(vars, || { - let config = { - let mut temp = CliConfig::empty(); - temp.update(&CliConfig::from_platform()); - temp.update(&CliConfig::from_env()); - temp - }; - - assert_eq!(config.data().unwrap(), tempdir.path()); - }); - } - - #[test] - fn resolve_data_with_flag() { - let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); - - let vars = vec![ - ("HOME", Some(tempdir.path().to_str().unwrap())), - ("XDG_DATA_HOME", None), - ("HC_DATA", None), - ]; - - with_env_vars(vars, || { - let expected = pathbuf![tempdir.path(), "hipcheck"]; - - let config = { - let mut temp = CliConfig::empty(); - temp.update(&CliConfig::from_platform()); - temp.update(&CliConfig::from_env()); - temp.update(&CliConfig { - path_args: PathArgs { - data: Some(expected.clone()), - ..Default::default() - }, - ..Default::default() - }); - temp - }; - - assert_eq!(config.data().unwrap(), expected); - }); - } - #[test] fn resolve_policy_with_flag() { let tempdir = TempDir::with_prefix(TEMPDIR_PREFIX).unwrap(); diff --git a/hipcheck/src/data/git/mod.rs b/hipcheck/src/data/git/mod.rs index 661ff109..c00ffedd 100644 --- a/hipcheck/src/data/git/mod.rs +++ b/hipcheck/src/data/git/mod.rs @@ -75,19 +75,6 @@ pub fn get_diffs(repo: &Path) -> Result> { git_diff(&output) } -pub fn get_commits_for_file(repo_path: &Path, file: &str) -> Result { - log::debug!("getting commits for file [file = '{}']", file); - - let output = GitCommand::for_repo( - repo_path, - ["log", "--follow", "--pretty=tformat:%H%n", "--", file], - )? - .output() - .context("git log hash command failed")?; - - Ok(output) -} - #[cfg(test)] mod test { use super::*; diff --git a/hipcheck/src/data/mod.rs b/hipcheck/src/data/mod.rs index 1bbaa2f1..2d36a4e7 100644 --- a/hipcheck/src/data/mod.rs +++ b/hipcheck/src/data/mod.rs @@ -10,22 +10,16 @@ mod code_quality; mod es_lint; mod github; mod hash; -mod modules; mod query; pub use query::*; -use crate::{ - error::{Context, Error, Result}, - hc_error, -}; -use git::{get_commits_for_file, Commit, CommitContributor, Contributor, Diff}; +use crate::error::{Context, Error, Result}; +use git::{Commit, CommitContributor, Contributor, Diff}; use github::*; -use modules::RawModule; use pathbuf::pathbuf; -use petgraph::{visit::Dfs, Graph}; use serde::Serialize; -use std::{collections::HashSet, path::Path, sync::Arc}; +use std::{path::Path, sync::Arc}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct Dependencies { @@ -202,122 +196,3 @@ pub fn get_single_pull_request_review_from_github( Ok(result) } - -// Module structs/enums - -#[derive(Debug, PartialEq, Eq, Copy, Clone, Serialize)] -pub enum Relationship { - Child, -} - -#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize)] -pub struct Module { - pub file: String, -} - -#[derive(Clone, Debug, Serialize)] -pub struct ModuleGraph { - pub connections: Graph, -} - -// For a given ModuleGraph representation of repository files, associate each module with the file's commit hashes -pub fn associate_modules_and_commits( - repo_path: &Path, - module_graph: Arc, - commits: Arc>>, -) -> Result { - // Vector containing pairings between module and associated commits - let mut modules_commits: Vec<(Arc, Arc)> = Vec::new(); - - // Graph traversal, depth-first - let start = module_graph - .connections - .node_indices() - .next() - .ok_or_else(|| hc_error!("no nodes in the module graph"))?; - let mut dfs = Dfs::new(&module_graph.connections, start); - - // Loop through adjoining nodes in graph - while let Some(visited) = dfs.next(&module_graph.connections) { - let hashes_raw = get_commits_for_file(repo_path, &module_graph.connections[visited].file)?; - - // Get hashes associated with this file - let hashes = hashes_raw.lines().collect::>(); - - // Get all commits referencing the hashes for current module in loop - let commit_vec = commits - .iter() - .filter_map(|commit| { - if hashes.contains(&commit.hash.as_ref()) { - Some(Arc::clone(commit)) - } else { - None - } - }) - .collect::>(); - - // Add entry to vec for each commit e.g. - for commit in commit_vec { - modules_commits.push(( - Arc::new(module_graph.connections[visited].clone()), - Arc::clone(&commit), - )); - } - } - - Ok(Arc::new(modules_commits)) -} - -impl ModuleGraph { - // Calls node library module-deps, converts json to graph model of modules for analysis - pub fn get_module_graph_from_repo(repo: &Path, module_deps: &Path) -> Result { - let raw_modules = modules::generate_module_model(repo, module_deps) - .context("Unable to generate module model")?; - module_graph_from_raw(raw_modules) - } -} - -// Implement a crude Eq and PartialEq trait (should revisit this as we evolve the module functionality) -impl Eq for ModuleGraph {} - -impl PartialEq for ModuleGraph { - fn eq(&self, _other: &Self) -> bool { - false - } -} - -fn module_graph_from_raw(raw_modules: Vec) -> Result { - let mut graph = Graph::new(); - let mut node_idxs = Vec::new(); - - for raw_module in raw_modules { - let node = Module { - file: raw_module.file, - }; - - let node_idx = graph.add_node(node); - - // Record the node index. - node_idxs.push(node_idx); - - for (_, dep) in raw_module.deps { - // Check if `dep` is a node already. Add if it isn't, get - // index to it. - let node = Module { file: dep }; - - let mut known_idx = None; - - for idx in &node_idxs { - if graph[*idx] == node { - known_idx = Some(*idx); - } - } - - let dep_idx = known_idx.unwrap_or_else(|| graph.add_node(node)); - - graph.add_edge(node_idx, dep_idx, Relationship::Child); - } - } - - Ok(ModuleGraph { connections: graph }) -} diff --git a/hipcheck/src/data/modules.rs b/hipcheck/src/data/modules.rs deleted file mode 100644 index b51fe353..00000000 --- a/hipcheck/src/data/modules.rs +++ /dev/null @@ -1,154 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -use crate::{ - error::{Context as _, Error, Result}, - hc_error, - util::command::{log_args, DependentProgram}, -}; -use pathbuf::pathbuf; -use serde::Deserialize; -use serde_json::Value as JsonValue; -use std::{ - collections::HashMap, - convert::AsRef, - ffi::OsStr, - fs, - iter::IntoIterator, - ops::Not as _, - path::{Path, PathBuf}, - process::Command, -}; - -pub fn generate_module_model(repo_dir: &Path, module_deps: &Path) -> Result> { - let root = detect_npm_package_root(&pathbuf![repo_dir, "package.json"])?; - if !pathbuf![repo_dir, &root].exists() { - return Err(Error::msg( - "Unable to identify module structure of repository code", - )); - } - let output = ModuleDepsCommand::for_path(repo_dir, module_deps, &[root])?.output()?; - - let raw_modules: Vec = - serde_json::from_str(&output).context("failed to parse module-deps output")?; - - Ok(raw_modules) -} - -fn detect_npm_package_root(pkg_file: &Path) -> Result { - let file_contents = fs::read_to_string(pkg_file); - match file_contents { - Ok(contents) => { - let json: JsonValue = serde_json::from_str(&contents)?; - let path = json - .pointer("/exports") - .or_else(|| json.pointer("/main")) - .or_else(|| json.pointer("/browser")) - .and_then(JsonValue::as_str) - .unwrap_or("index.js"); - - Ok(PathBuf::from(path)) - } - Err(..) => Err(hc_error!( - "package.json file not found; file missing or repo does not use Node.js modules" - )), - } -} - -#[derive(Debug, Deserialize)] -pub struct RawModule { - pub file: String, - pub deps: HashMap, - // These two fields are part of the raw module data, so we include them here, - // but they're not used anywhere in the code. - #[allow(unused)] - pub entry: bool, - #[allow(unused)] - pub expose: Option, -} - -#[derive(Debug)] -pub struct ModuleDepsCommand { - command: Command, -} - -impl ModuleDepsCommand { - pub fn for_path( - pkg_path: &Path, - module_deps_path: &Path, - args: I, - ) -> Result - where - I: IntoIterator + Copy, - S: AsRef, - { - ModuleDepsCommand::internal(Some(pkg_path), module_deps_path, args) - } - - fn internal( - pkg_path: Option<&Path>, - module_deps_path: &Path, - args: I, - ) -> Result - where - I: IntoIterator + Copy, - S: AsRef, - { - let path = module_deps_path.display().to_string(); - log_args(&path, args, DependentProgram::ModuleDeps); - - let mut command = Command::new(module_deps_path); - command.args(args); - - // Set the path if necessary - if let Some(pkg_path) = pkg_path { - command.current_dir(pkg_path); - } - - Ok(ModuleDepsCommand { command }) - } - - fn output(&mut self) -> Result { - let output = self.command.output()?; - - if output.status.success() { - let output_text = String::from_utf8_lossy(&output.stdout).to_string(); - return Ok(output_text); - } - - match String::from_utf8(output.stderr) { - Ok(msg) if msg.is_empty().not() => Err(hc_error!( - "(from module-deps) {} [{}]", - msg.trim(), - output.status - )), - _ => Err(hc_error!("module-deps failed [{}]", output.status)), - } - } -} - -// #[cfg(test)] -// mod tests { -// use super::*; -// use std::env::current_exe; -// use std::io::Result as IoResult; -// use std::path::PathBuf; - -// #[test] -// fn can_run_module_deps() { -// let path = get_testpkg_path().unwrap(); -// if let Ok(mut command) = ModuleDepsCommand::for_path(&path, ,&["main.js"]) { -// let _ = command.output().unwrap(); -// } -// } - -// // Return the absolute path to testpkg -// fn get_testpkg_path() -> IoResult { -// let mut path = current_exe()?; -// for _ in 0..4 { -// path.pop(); -// } -// path.push("libs/hc_data/src/modules/testpkg"); - -// Ok(path) -// } -// } diff --git a/hipcheck/src/data/query/mod.rs b/hipcheck/src/data/query/mod.rs index f187cf8b..68ea8e52 100644 --- a/hipcheck/src/data/query/mod.rs +++ b/hipcheck/src/data/query/mod.rs @@ -7,12 +7,10 @@ mod code_quality; mod dependencies; mod fuzz; mod github; -mod module; mod pr_review; pub use code_quality::CodeQualityProviderStorage; pub use dependencies::{DependenciesProvider, DependenciesProviderStorage}; pub use fuzz::{FuzzProvider, FuzzProviderStorage}; pub use github::{GitHubProvider, GitHubProviderStorage}; -pub use module::{ModuleCommitMap, ModuleProvider, ModuleProviderStorage}; pub use pr_review::{PullRequestReviewProvider, PullRequestReviewProviderStorage}; diff --git a/hipcheck/src/data/query/module.rs b/hipcheck/src/data/query/module.rs deleted file mode 100644 index 0a156c5b..00000000 --- a/hipcheck/src/data/query/module.rs +++ /dev/null @@ -1,96 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -//! Query group for module information. - -use crate::{ - data::{ - associate_modules_and_commits, - git::{Commit, GitProvider}, - Module, ModuleGraph, - }, - error::{Error, Result}, -}; -use pathbuf::pathbuf; -use std::{path::PathBuf, sync::Arc}; - -/// A module and an associated commit -pub type ModuleCommitMap = Arc, Arc)>>; - -/// Queries about modules -#[salsa::query_group(ModuleProviderStorage)] -pub trait ModuleProvider: GitProvider { - /// Returns output of module analysis on the source code. - #[salsa::dependencies] - fn get_module_graph(&self) -> Result>; - - /// Returns an association list of modules and commits - fn commits_for_modules(&self) -> Result; - - /// Returns the commits associated with a particular module - fn commits_for_module(&self, module: Arc) -> Result>>>; - - /// Returns the modules associated with a particular commit - fn modules_for_commit(&self, commit: Arc) -> Result>>>; - - /// Returns the directory containing the data files - #[salsa::input] - fn data_dir(&self) -> Arc; - - /// Returns the location of the module-deps.js file - fn module_deps(&self) -> Result>; -} - -/// Derived query implementations. Return values are wrapped in an -/// `Rc` to keep cloning cheap. - -fn get_module_graph(db: &dyn ModuleProvider) -> Result> { - let module_deps = db.module_deps()?; - ModuleGraph::get_module_graph_from_repo(&db.local(), module_deps.as_ref()).map(Arc::new) -} - -fn commits_for_modules(db: &dyn ModuleProvider) -> Result { - let repo_path = db.local(); - let commits = db.commits()?; - let modules = db.get_module_graph()?; - associate_modules_and_commits(repo_path.as_ref(), modules, commits) -} - -fn commits_for_module( - db: &dyn ModuleProvider, - module: impl AsRef, -) -> Result>>> { - let commits = db - .commits_for_modules()? - .iter() - .filter(|(m, _)| **m == *module.as_ref()) - .map(|(_, c)| Arc::clone(c)) - .collect(); - - Ok(Arc::new(commits)) -} - -fn modules_for_commit( - db: &dyn ModuleProvider, - commit: impl AsRef, -) -> Result>>> { - let modules = db - .commits_for_modules()? - .iter() - .filter(|(_, c)| **c == *commit.as_ref()) - .map(|(m, _)| Arc::clone(m)) - .collect(); - - Ok(Arc::new(modules)) -} - -fn module_deps(db: &dyn ModuleProvider) -> Result> { - let data_path = db.data_dir(); - let module_deps_path = pathbuf![data_path.as_ref(), "module-deps.js"]; - if module_deps_path.exists() { - Ok(Arc::new(module_deps_path)) - } else { - Err(Error::msg( - "module-deps.js missing from Hipcheck data folder", - )) - } -} diff --git a/hipcheck/src/main.rs b/hipcheck/src/main.rs index 65888851..9af6e77a 100644 --- a/hipcheck/src/main.rs +++ b/hipcheck/src/main.rs @@ -99,7 +99,6 @@ fn main() -> ExitCode { Some(FullCommands::Cache(args)) => return cmd_cache(args, &config), Some(FullCommands::Plugin(args)) => cmd_plugin(args), Some(FullCommands::PrintConfig) => cmd_print_config(config.config()), - Some(FullCommands::PrintData) => cmd_print_data(config.data()), Some(FullCommands::PrintCache) => cmd_print_home(config.cache()), Some(FullCommands::Scoring) => { return cmd_print_weights(&config) @@ -130,7 +129,6 @@ fn cmd_check(args: &CheckArgs, config: &CliConfig) -> ExitCode { let report = run( target, config.config().map(ToOwned::to_owned), - config.data().map(ToOwned::to_owned), config.cache().map(ToOwned::to_owned), config.policy().map(ToOwned::to_owned), config.format(), @@ -176,7 +174,6 @@ fn cmd_print_weights(config: &CliConfig) -> Result<()> { refspec: Some("HEAD".to_owned()), }, config.config().map(ToOwned::to_owned), - config.data().map(ToOwned::to_owned), config.cache().map(ToOwned::to_owned), config.policy().map(ToOwned::to_owned), config.format(), @@ -300,12 +297,9 @@ fn cmd_setup(args: &SetupArgs, config: &CliConfig) -> ExitCode { Ok(x) => x, }; - // Derive the config/scripts paths from the source path - let (src_conf_path, src_data_path) = match &source.path { - SourceType::Dir(p) => ( - pathbuf![p.as_path(), "config"], - pathbuf![p.as_path(), "scripts"], - ), + // Derive the config path from the source path + let src_conf_path = match &source.path { + SourceType::Dir(p) => pathbuf![p.as_path(), "config"], _ => { Shell::print_error( &hc_error!("expected source to be a directory"), @@ -337,25 +331,6 @@ fn cmd_setup(args: &SetupArgs, config: &CliConfig) -> ExitCode { return ExitCode::FAILURE; }; - // Make data dir if not exist - let Some(tgt_data_path) = config.data() else { - Shell::print_error(&hc_error!("target data dir not specified"), Format::Human); - return ExitCode::FAILURE; - }; - if !tgt_data_path.exists() && create_dir_all(tgt_data_path).is_err() { - Shell::print_error( - &hc_error!("failed to create missing target data dir"), - Format::Human, - ); - } - let Ok(abs_data_path) = tgt_data_path.canonicalize() else { - Shell::print_error( - &hc_error!("failed to canonicalize HC_DATA path"), - Format::Human, - ); - return ExitCode::FAILURE; - }; - // Copy local config/data dirs to target locations if let Err(e) = copy_dir_contents(src_conf_path, &abs_conf_path) { Shell::print_error( @@ -364,13 +339,6 @@ fn cmd_setup(args: &SetupArgs, config: &CliConfig) -> ExitCode { ); return ExitCode::FAILURE; } - if let Err(e) = copy_dir_contents(src_data_path, &abs_data_path) { - Shell::print_error( - &hc_error!("failed to copy data dir contents: {}", e), - Format::Human, - ); - return ExitCode::FAILURE; - } println!("Hipcheck setup completed successfully."); @@ -386,7 +354,6 @@ fn cmd_setup(args: &SetupArgs, config: &CliConfig) -> ExitCode { shell_profile ); println!("\texport HC_CONFIG={:?}", abs_conf_path); - println!("\texport HC_DATA={:?}", abs_data_path); println!("Run `hc help` to get started"); @@ -400,7 +367,6 @@ struct ReadyChecks { hipcheck_version_check: StdResult, git_version_check: StdResult, npm_version_check: StdResult, - data_path_check: StdResult, cache_path_check: StdResult, policy_path_check: StdResult, github_token_check: StdResult<(), EnvVarCheckError>, @@ -414,7 +380,6 @@ impl ReadyChecks { self.hipcheck_version_check.is_ok() && self.git_version_check.is_ok() && self.npm_version_check.is_ok() - && self.data_path_check.is_ok() && self.cache_path_check.is_ok() && self.policy_path_check.is_ok() } @@ -544,16 +509,6 @@ fn check_cache_path(config: &CliConfig) -> StdResult { Ok(path.to_owned()) } -fn check_data_path(config: &CliConfig) -> StdResult { - let path = config.data().ok_or(PathCheckError::PathNotFound)?; - - if path.exists().not() { - return Err(PathCheckError::PathNotFound); - } - - Ok(path.to_owned()) -} - fn check_policy_path(config: &CliConfig) -> StdResult { let path = config.policy().ok_or(PathCheckError::PolicyNotFound)?; @@ -679,7 +634,6 @@ fn cmd_ready(config: &CliConfig) { hipcheck_version_check: check_hipcheck_version(), git_version_check: check_git_version(), npm_version_check: check_npm_version(), - data_path_check: check_data_path(config), cache_path_check: check_cache_path(config), policy_path_check: check_policy_path(config), github_token_check: check_github_token(), @@ -705,11 +659,6 @@ fn cmd_ready(config: &CliConfig) { Err(e) => println!("{:<17} {}", "Cache Path:", e), } - match &ready.data_path_check { - Ok(path) => println!("{:<17} {}", "Data Path:", path.display()), - Err(e) => println!("{:<17} {}", "Data Path:", e), - } - match &ready.policy_path_check { Ok(path) => println!("{:<17} {}", "Policy Path:", path.display()), Err(e) => println!("{:<17} {}", "Policy Path:", e), @@ -829,20 +778,6 @@ fn cmd_print_config(config_path: Option<&Path>) { } } -/// Print the current data folder path for Hipcheck. -/// -/// Exits `Ok` if config path is specified, `Err` otherwise. -fn cmd_print_data(data_path: Option<&Path>) { - match data_path.ok_or_else(|| hc_error!("can't find data directory")) { - Ok(path_buffer) => { - println!("{}", path_buffer.display()); - } - Err(err) => { - Shell::print_error(&err, Format::Human); - } - } -} - /// Print the JSON schema of the report. fn print_report_schema() { let schema = schema_for!(Report); @@ -913,20 +848,12 @@ impl CheckKind { fn run( target: TargetSeed, config_path: Option, - data_path: Option, home_dir: Option, policy_path: Option, format: Format, ) -> Result { // Initialize the session. - let session = match Session::new( - &target, - config_path, - data_path, - home_dir, - policy_path, - format, - ) { + let session = match Session::new(&target, config_path, home_dir, policy_path, format) { Ok(session) => session, Err(err) => return Err(err), }; diff --git a/hipcheck/src/metric/mod.rs b/hipcheck/src/metric/mod.rs index e1955d3c..da2d1d40 100644 --- a/hipcheck/src/metric/mod.rs +++ b/hipcheck/src/metric/mod.rs @@ -12,23 +12,18 @@ pub mod fuzz; pub mod identity; pub mod linguist; mod math; -pub mod module; pub mod review; pub mod typo; use crate::{ config::{AttacksConfigQuery, CommitConfigQuery}, - data::{ - git::GitProvider, DependenciesProvider, FuzzProvider, ModuleProvider, - PullRequestReviewProvider, - }, + data::{git::GitProvider, DependenciesProvider, FuzzProvider, PullRequestReviewProvider}, error::Result, metric::{ activity::ActivityOutput, affiliation::AffiliationOutput, binary::BinaryOutput, binary_detector::BinaryFile, churn::ChurnOutput, commit_trust::CommitTrustOutput, contributor_trust::ContributorTrustOutput, entropy::EntropyOutput, fuzz::FuzzOutput, - identity::IdentityOutput, linguist::Linguist, module::ModuleOutput, review::ReviewOutput, - typo::TypoOutput, + identity::IdentityOutput, linguist::Linguist, review::ReviewOutput, typo::TypoOutput, }, }; use std::sync::Arc; @@ -42,7 +37,6 @@ pub trait MetricProvider: + DependenciesProvider + GitProvider + Linguist - + ModuleProvider + FuzzProvider + PullRequestReviewProvider { @@ -78,10 +72,6 @@ pub trait MetricProvider: #[salsa::invoke(identity::identity_metric)] fn identity_metric(&self) -> Result>; - /// Returns result of module analysis. - #[salsa::invoke(module::module_analysis)] - fn module_analysis(&self) -> Result>; - /// Returns result of fuzz metric #[salsa::invoke(fuzz::fuzz_metric)] fn fuzz_metric(&self) -> Result>; diff --git a/hipcheck/src/metric/module.rs b/hipcheck/src/metric/module.rs deleted file mode 100644 index de5d5d2f..00000000 --- a/hipcheck/src/metric/module.rs +++ /dev/null @@ -1,34 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -use crate::{ - data::ModuleGraph, - error::{Context as _, Result}, - metric::MetricProvider, -}; -use serde::Serialize; -use std::sync::Arc; - -#[derive(Debug, Eq, PartialEq, Serialize)] -pub struct ModuleOutput { - pub module_graph: Arc, - pub is_modular: bool, -} - -pub fn module_analysis(db: &dyn MetricProvider) -> Result> { - log::debug!("running module analysis"); - - let module_graph = db - .get_module_graph() - .context("failed to get module graph")?; - - log::trace!("got module graph [pr='{:#?}']", module_graph); - - let modules = ModuleOutput { - module_graph, - is_modular: true, - }; - - log::info!("completed module analysis"); - - Ok(Arc::new(modules)) -} diff --git a/hipcheck/src/session/mod.rs b/hipcheck/src/session/mod.rs index e3531e1c..78c7fbd4 100644 --- a/hipcheck/src/session/mod.rs +++ b/hipcheck/src/session/mod.rs @@ -17,8 +17,7 @@ use crate::{ git::{get_git_version, GitProviderStorage}, npm::get_npm_version, CodeQualityProviderStorage, DependenciesProviderStorage, FuzzProviderStorage, - GitHubProviderStorage, ModuleProvider, ModuleProviderStorage, - PullRequestReviewProviderStorage, + GitHubProviderStorage, PullRequestReviewProviderStorage, }, engine::{start_plugins, HcEngine, HcEngineStorage}, error::{Context as _, Error, Result}, @@ -67,7 +66,6 @@ use url::Url; LanguagesConfigQueryStorage, LinguistStorage, MetricProviderStorage, - ModuleProviderStorage, FuzzProviderStorage, PracticesConfigQueryStorage, PullRequestReviewProviderStorage, @@ -111,7 +109,6 @@ impl Session { pub fn new( target: &TargetSeed, config_path: Option, - data_path: Option, home_dir: Option, policy_path: Option, format: Format, @@ -150,8 +147,8 @@ impl Session { // Check if a policy file was provided, otherwise convert a deprecated config file to a policy file. If neither was provided, error out. if policy_path.is_some() { - let (policy, policy_path, data_dir, hc_github_token) = - match load_policy_and_data(policy_path.as_deref(), data_path.as_deref()) { + let (policy, policy_path, hc_github_token) = + match load_policy_and_data(policy_path.as_deref()) { Ok(results) => results, Err(err) => return Err(err), }; @@ -163,14 +160,11 @@ impl Session { session.set_policy(Rc::new(policy)); session.set_policy_path(Some(Rc::new(policy_path))); - // Set data folder location for module analysis - session.set_data_dir(Arc::new(data_dir)); - // Set github token in salsa session.set_github_api_token(Some(Rc::new(hc_github_token))); } else if config_path.is_some() { - let (policy, config_dir, data_dir, hc_github_token) = - match load_config_and_data(config_path.as_deref(), data_path.as_deref()) { + let (policy, config_dir, hc_github_token) = + match load_config_and_data(config_path.as_deref()) { Ok(results) => results, Err(err) => return Err(err), }; @@ -182,9 +176,6 @@ impl Session { session.set_policy(Rc::new(policy)); session.set_policy_path(None); - // Set data folder location for module analysis - session.set_data_dir(Arc::new(data_dir)); - // Set github token in salsa session.set_github_api_token(Some(Rc::new(hc_github_token))); } else { @@ -262,10 +253,7 @@ fn load_software_versions() -> Result<(String, String)> { Ok((git_version, npm_version)) } -fn load_config_and_data( - config_path: Option<&Path>, - data_path: Option<&Path>, -) -> Result<(PolicyFile, PathBuf, PathBuf, String)> { +fn load_config_and_data(config_path: Option<&Path>) -> Result<(PolicyFile, PathBuf, String)> { // Start the phase. let phase = SpinnerPhase::start("Loading configuration and data files from config file. Note: The use of a config TOML file is deprecated. Please consider using a policy KDL file in the future."); // Increment the phase into the "running" stage. @@ -284,28 +272,15 @@ fn load_config_and_data( // Convert the Config struct to a PolicyFile struct let policy = config_to_policy(config)?; - // Get the directory the data file is in. - let data_dir = data_path - .ok_or_else(|| hc_error!("Failed to load data files. Please make sure the path set by the hc_data env variable exists."))? - .to_owned(); - // Resolve the github token file. let hc_github_token = resolve_token()?; phase.finish_successful(); - Ok(( - policy, - valid_config_path.to_path_buf(), - data_dir, - hc_github_token, - )) + Ok((policy, valid_config_path.to_path_buf(), hc_github_token)) } -fn load_policy_and_data( - policy_path: Option<&Path>, - data_path: Option<&Path>, -) -> Result<(PolicyFile, PathBuf, PathBuf, String)> { +fn load_policy_and_data(policy_path: Option<&Path>) -> Result<(PolicyFile, PathBuf, String)> { // Start the phase. let phase = SpinnerPhase::start("loading policy and data files"); // Increment the phase into the "running" stage. @@ -324,22 +299,12 @@ fn load_policy_and_data( let policy = PolicyFile::load_from(valid_policy_path) .context("Failed to load policy. Plase make sure the policy file is in the proidved location and is formatted correctly.")?; - // Get the directory the data file is in. - let data_dir = data_path - .ok_or_else(|| hc_error!("Failed to load data files. Please make sure the path set by the hc_data env variable exists."))? - .to_owned(); - // Resolve the github token file. let hc_github_token = resolve_token()?; phase.finish_successful(); - Ok(( - policy, - valid_policy_path.to_path_buf(), - data_dir, - hc_github_token, - )) + Ok((policy, valid_policy_path.to_path_buf(), hc_github_token)) } fn load_target(seed: &TargetSeed, home: &Path) -> Result { diff --git a/scripts/module-deps.js b/scripts/module-deps.js deleted file mode 100644 index 661483da..00000000 --- a/scripts/module-deps.js +++ /dev/null @@ -1,347 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -/** - * `module-deps.js` - * - * This file is a helper script that Hipcheck uses to enable us to leverage - * the "module-deps" NPM package when trying to do module-level analyses of - * NPM packages. - * - * The `module-deps` package isn't really built to be used as a CLI, so this - * helper script lets us do that to make consuming the data it produces - * from Rust easier. - * - * This script also ensures `module-deps` doesn't error out when it encounters - * imports of packages provided by Node.js. - * - * Portions of this script are vendored in from different NPM packages. The - * appropriate license notices are included with them below. - */ - -let process = require("process"); -let Stream = require("stream"); -let mdeps = require("module-deps"); - -const getBuiltins = function () { - // Making this function memoized ensures the set of builtins is - // only generated once, and can then be reused on each call to builtins() - // in the main function below. - let cached = null; - - return () => { - if (cached !== null) { - return cached; - } - - // This is the list of built-in packages provided by Node.js. - const pkgs = [ - "assert", - "async_hooks", - "buffer", - "child_process", - "cluster", - "console", - "constants", - "crypto", - "dgram", - "dns", - "domain", - "events", - "fs", - "http", - "http2", - "https", - "inspector", - "module", - "net", - "os", - "path", - "perf_hooks", - "process", - "punycode", - "querystring", - "readline", - "repl", - "stream", - "string_decoder", - "sys", - "timers", - "tls", - "trace_events", - "tty", - "url", - "util", - "v8", - "vm", - "wasi", - "worker_threads", - "zlib", - ]; - - let builtins = new Set(); - - for (const pkg of pkgs) { - builtins.add(pkg); - } - - cached = builtins; - - return builtins; - }; -}; - -const processArgs = function () { - let args = process.argv.slice(2); - - if (args.length < 1) { - process.stderr.write("error: missing entrypoint name"); - process.exit(1); - } else if (args.length > 1) { - process.stderr.write("error: only one entrypoint accepted"); - process.exit(1); - } - - return args; -}; - -/* - * This function is adapted from the 'JSONStream' NPM library. - * - * Copyright (c) 2011 Dominic Tarr - * - * Permission is hereby granted, free of charge, - * to any person obtaining a copy of this software and - * associated documentation files (the "Software"), to - * deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, - * merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom - * the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice - * shall be included in all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES - * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. - * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR - * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, - * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE - * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -const stringify = function (op, sep, cl, indent) { - indent = indent || 0; - - if (op === false) { - op = ""; - sep = "\n"; - cl = ""; - } else if (op == null) { - op = "[\n"; - sep = "\n,\n"; - cl = "\n]\n"; - } - - var stream; - var first = true; - var anyData = false; - - var write = function (data) { - anyData = true; - - try { - var json = JSON.stringify(data, null, indent); - } catch (err) { - return stream.emit("error", err); - } - - if (first) { - first = false; - stream.queue(op + json); - } else { - stream.queue(sep + json); - } - }; - - var end = function (_data) { - if (!anyData) { - stream.queue(op); - } - - stream.queue(cl); - stream.queue(null); - }; - - stream = through(write, end); - return stream; -}; - -/* - * This function is adapted from the 'through' NPM library. - * - * Copyright (c) 2011 Dominic Tarr - * - * Permission is hereby granted, free of charge, - * to any person obtaining a copy of this software and - * associated documentation files (the "Software"), to - * deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, - * merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom - * the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice - * shall be included in all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES - * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. - * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR - * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, - * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE - * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -const through = function (write, end, opts) { - write = - write || - function (data) { - this.queue(data); - }; - end = - end || - function () { - this.queue(null); - }; - - var ended = false; - var destroyed = false; - var buffer = []; - var _ended = false; - - var stream = new Stream(); - stream.readable = stream.writable = true; - stream.paused = false; - stream.autoDestroy = !(opts && opts.autoDestroy === false); - - stream.write = function (data) { - write.call(this, data); - return !stream.paused; - }; - - function drain() { - while (buffer.length && !stream.paused) { - var data = buffer.shift(); - - if (null === data) { - return stream.emit("end"); - } else { - stream.emit("data", data); - } - } - } - - stream.queue = stream.push = function (data) { - if (_ended) { - return stream; - } - - if (data === null) { - _ended = true; - } - - buffer.push(data); - drain(); - return stream; - }; - - stream.on("end", function () { - stream.readable = false; - - if (!stream.writable && stream.autoDestroy) - process.nextTick(function () { - stream.destroy(); - }); - }); - - function _end() { - stream.writable = false; - end.call(stream); - - if (!stream.readable && stream.autoDestroy) { - stream.destroy(); - } - } - - stream.end = function (data) { - if (ended) { - return; - } - - ended = true; - - if (arguments.length) { - stream.write(data); - } - - _end(); - return stream; - }; - - stream.destroy = function () { - if (destroyed) { - return; - } - - destroyed = true; - ended = true; - buffer.length = 0; - stream.writable = stream.readable = false; - stream.emit("close"); - return stream; - }; - - stream.pause = function () { - if (stream.paused) { - return; - } - - stream.paused = true; - return stream; - }; - - stream.resume = function () { - if (stream.paused) { - stream.paused = false; - stream.emit("resume"); - } - - drain(); - - if (!stream.paused) { - stream.emit("drain"); - } - - return stream; - }; - - return stream; -}; - -const main = function () { - let args = processArgs(); - - // Get a memoized function that returns the builtins. - let builtins = getBuiltins(); - - const filterBuiltins = function (name) { - return !builtins().has(name); - }; - - let md = mdeps({ filter: filterBuiltins }); - md.pipe(stringify()).pipe(process.stdout); - md.end({ file: args[0] }); -}; - -main();