Skip to content

Commit

Permalink
chore(cli): Deprecates the config arg
Browse files Browse the repository at this point in the history
  • Loading branch information
@mchernicoff @alilleybrinker
mchernicoff authored and alilleybrinker committed Sep 6, 2024
1 parent dc397e0 commit 16023e8
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 45 deletions.
38 changes: 19 additions & 19 deletions hipcheck/src/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,6 @@ struct OutputArgs {
/// Arguments configuring paths for Hipcheck to use.
#[derive(Debug, Default, clap::Args, hc::Update)]
struct PathArgs {
/// Path to the configuration folder.
#[arg(
short = 'c',
long = "config",
global = true,
help_heading = "Path Flags",
long_help = "Path to the configuration folder. Can also be set with the `HC_CONFIG` environment variable"
)]
config: Option<PathBuf>,

/// Path to the data folder.
#[arg(
short = 'd',
Expand Down Expand Up @@ -144,6 +134,10 @@ struct DeprecatedArgs {
#[arg(long = "print-data", hide = true, global = true)]
print_data: Option<bool>,

/// Path to the configuration folder.
#[arg(short = 'c', long = "config", hide = true, global = true)]
config: Option<PathBuf>,

/// Path to the Hipcheck home folder.
#[arg(short = 'H', long = "home", hide = true, global = true)]
home: Option<PathBuf>,
Expand Down Expand Up @@ -218,11 +212,6 @@ impl CliConfig {
}
}

/// Get the path to the configuration directory.
pub fn config(&self) -> Option<&Path> {
self.path_args.config.as_deref()
}

/// Get the path to the data directory.
pub fn data(&self) -> Option<&Path> {
self.path_args.data.as_deref()
Expand All @@ -246,6 +235,11 @@ impl CliConfig {
self.path_args.policy.as_deref()
}

/// Get the path to the configuration directory.
pub fn config(&self) -> Option<&Path> {
self.deprecated_args.config.as_deref()
}

/// Check if the `--print-home` flag was used.
pub fn print_home(&self) -> bool {
self.deprecated_args.print_home.unwrap_or(false)
Expand Down Expand Up @@ -286,13 +280,13 @@ impl CliConfig {
format: hc_env_var_value_enum("format"),
},
path_args: PathArgs {
config: hc_env_var("config"),
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,
},
deprecated_args: DeprecatedArgs {
config: hc_env_var("config"),
home: hc_env_var("home"),
..Default::default()
},
Expand All @@ -308,11 +302,14 @@ impl CliConfig {
CliConfig {
path_args: PathArgs {
cache: platform_cache(),
config: platform_config(),
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,
},
deprecated_args: DeprecatedArgs {
config: platform_config(),
..Default::default()
},
..Default::default()
}
}
Expand All @@ -321,14 +318,17 @@ impl CliConfig {
fn backups() -> CliConfig {
CliConfig {
path_args: PathArgs {
config: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "config"]),
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
// policy: env::current_dir().ok().map(|dir| pathbuf![&dir, "Hipcheck.kdl"]),
policy: None,
},
deprecated_args: DeprecatedArgs {
config: dirs::home_dir().map(|dir| pathbuf![&dir, "hipcheck", "config"]),
..Default::default()
},
..Default::default()
}
}
Expand Down Expand Up @@ -1126,7 +1126,7 @@ mod tests {
temp.update(&CliConfig::from_platform());
temp.update(&CliConfig::from_env());
temp.update(&CliConfig {
path_args: PathArgs {
deprecated_args: DeprecatedArgs {
config: Some(expected.clone()),
..Default::default()
},
Expand Down
23 changes: 1 addition & 22 deletions hipcheck/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ struct ReadyChecks {
hipcheck_version_check: StdResult<String, VersionCheckError>,
git_version_check: StdResult<String, VersionCheckError>,
npm_version_check: StdResult<String, VersionCheckError>,
config_path_check: StdResult<PathBuf, PathCheckError>,
data_path_check: StdResult<PathBuf, PathCheckError>,
cache_path_check: StdResult<PathBuf, PathCheckError>,
policy_path_check: StdResult<PathBuf, PathCheckError>,
Expand All @@ -428,7 +427,6 @@ impl ReadyChecks {
self.hipcheck_version_check.is_ok()
&& self.git_version_check.is_ok()
&& self.npm_version_check.is_ok()
&& self.config_path_check.is_ok()
&& self.data_path_check.is_ok()
&& self.cache_path_check.is_ok()
&& self.policy_path_check.is_ok()
Expand Down Expand Up @@ -548,18 +546,6 @@ fn check_npm_version() -> StdResult<String, VersionCheckError> {
})
}

fn check_config_path(config: &CliConfig) -> StdResult<PathBuf, PathCheckError> {
let path = config.config().ok_or(PathCheckError::PathNotFound)?;

let path = pathbuf![path, HIPCHECK_TOML_FILE];

if path.exists().not() {
return Err(PathCheckError::PathNotFound);
}

Ok(path)
}

fn check_cache_path(config: &CliConfig) -> StdResult<PathBuf, PathCheckError> {
let path = config.cache().ok_or(PathCheckError::PathNotFound)?;

Expand All @@ -582,7 +568,7 @@ fn check_data_path(config: &CliConfig) -> StdResult<PathBuf, PathCheckError> {
}

fn check_policy_path(config: &CliConfig) -> StdResult<PathBuf, PathCheckError> {
let path = config.policy().ok_or(PathCheckError::PathNotFound)?;
let path = config.policy().ok_or(PathCheckError::PolicyNotFound)?;

if path.exists().not() {
return Err(PathCheckError::PolicyNotFound);
Expand Down Expand Up @@ -706,7 +692,6 @@ fn cmd_ready(config: &CliConfig) {
hipcheck_version_check: check_hipcheck_version(),
git_version_check: check_git_version(),
npm_version_check: check_npm_version(),
config_path_check: check_config_path(config),
data_path_check: check_data_path(config),
cache_path_check: check_cache_path(config),
policy_path_check: check_policy_path(config),
Expand All @@ -733,11 +718,6 @@ fn cmd_ready(config: &CliConfig) {
Err(e) => println!("{:<17} {}", "Cache Path:", e),
}

match &ready.config_path_check {
Ok(path) => println!("{:<17} {}", "Config Path:", path.display()),
Err(e) => println!("{:<17} {}", "Config Path:", e),
}

match &ready.data_path_check {
Ok(path) => println!("{:<17} {}", "Data Path:", path.display()),
Err(e) => println!("{:<17} {}", "Data Path:", e),
Expand Down Expand Up @@ -911,7 +891,6 @@ const LANGS_FILE: &str = "Langs.toml";
const BINARY_CONFIG_FILE: &str = "Binary.toml";
const TYPO_FILE: &str = "Typos.toml";
const ORGS_FILE: &str = "Orgs.toml";
const HIPCHECK_TOML_FILE: &str = "Hipcheck.toml";

// Constants for exiting with error codes.
/// Indicates the program failed.
Expand Down
10 changes: 6 additions & 4 deletions hipcheck/src/session/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl Session {
* Loading configuration.
*-----------------------------------------------------------------*/

// Check if a policy file was provided, otherwise convert a deprecated config file to a policy file
// 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()) {
Expand All @@ -177,7 +177,7 @@ impl Session {

// Set github token in salsa
session.set_github_api_token(Some(Rc::new(hc_github_token)));
} else {
} 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()) {
Ok(results) => results,
Expand All @@ -196,6 +196,8 @@ impl Session {

// Set github token in salsa
session.set_github_api_token(Some(Rc::new(hc_github_token)));
} else {
return Err(hc_error!("No policy file or (deprecated) config file found. Please provide a policy file before running Hipcheck."));
}

/*===================================================================
Expand Down Expand Up @@ -256,7 +258,7 @@ fn load_config_and_data(
data_path: Option<&Path>,
) -> Result<(PolicyFile, PathBuf, PathBuf, String)> {
// Start the phase.
let phase = SpinnerPhase::start("loading configuration and data files");
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.
phase.inc();
// Set the spinner phase to tick constantly, 10 times a second.
Expand Down Expand Up @@ -311,7 +313,7 @@ fn load_policy_and_data(

// Load the policy file.
let policy = PolicyFile::load_from(valid_policy_path)
.context("Failed to load policy. Plase make sure the policy file is in the proived location and is formatted correctly.")?;
.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
Expand Down

0 comments on commit 16023e8

Please sign in to comment.