Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layered config support #111

Merged
merged 7 commits into from
May 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/cmd.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::BTreeMap;
use std::ffi::OsStr;
use std::path::Path;
use std::process::Command;

Expand All @@ -7,7 +8,7 @@ use crate::error::FatalError;
fn do_call(
command: Vec<&str>,
path: Option<&Path>,
envs: Option<BTreeMap<&str, &str>>,
envs: Option<BTreeMap<&OsStr, &OsStr>>,
dry_run: bool,
) -> Result<bool, FatalError> {
if dry_run {
Expand All @@ -30,9 +31,7 @@ fn do_call(
}

if let Some(e) = envs {
for (key, val) in e.iter() {
cmd.env(key, val);
}
cmd.envs(e.iter());
}

for arg in iter {
Expand All @@ -57,7 +56,7 @@ pub fn call_on_path(command: Vec<&str>, path: &Path, dry_run: bool) -> Result<bo

pub fn call_with_env(
command: Vec<&str>,
envs: BTreeMap<&str, &str>,
envs: BTreeMap<&OsStr, &OsStr>,
path: &Path,
dry_run: bool,
) -> Result<bool, FatalError> {
Expand Down
57 changes: 32 additions & 25 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,41 +446,48 @@ pub fn resolve_custom_config(file_path: &Path) -> Result<Option<Config>, FatalEr

/// Try to resolve configuration source.
///
/// This tries the following sources in order, short-circuiting on the first one found:
/// 1. $(pwd)/release.toml
/// 2. $(pwd)/Cargo.toml `package.metadata.release` (with deprecation warning)
/// 3. $HOME/.release.toml
/// This tries the following sources in order, merging the results:
/// 1. $HOME/.release.toml
/// 2. $(workspace)/release.toml
/// 3. $(crate)/Cargo.toml `package.metadata.release` (with deprecation warning)
/// 4. $(crate)/release.toml
///
pub fn resolve_config(manifest_path: &Path) -> Result<Option<Config>, FatalError> {
// Project release file.
let default_config = manifest_path
.parent()
.unwrap_or_else(|| Path::new("."))
.join("release.toml");
let current_dir_config = get_config_from_file(&default_config)?;
if let Some(cfg) = current_dir_config {
let mut config = Config::default();
config.update(&cfg);
return Ok(Some(config));
pub fn resolve_config(workspace_root: &Path, manifest_path: &Path) -> Result<Config, FatalError> {
let mut config = Config::default();

// User-local configuration from home directory.
let home_dir = dirs::home_dir();
if let Some(mut home) = home_dir {
home.push(".release.toml");
if let Some(cfg) = get_config_from_file(&home)? {
config.update(&cfg);
}
};

let crate_root = manifest_path.parent().unwrap_or_else(|| Path::new("."));

if crate_root != workspace_root {
let default_config = workspace_root.join("release.toml");
let current_dir_config = get_config_from_file(&default_config)?;
if let Some(cfg) = current_dir_config {
config.update(&cfg);
};
}

// Crate manifest.
let current_dir_config = get_config_from_manifest(manifest_path)?;
if let Some(cfg) = current_dir_config {
let mut config = Config::default();
config.update(&cfg);
return Ok(Some(config));
};

// User-local configuration from home directory.
let home_dir = dirs::home_dir();
if let Some(mut home) = home_dir {
home.push(".release.toml");
return resolve_custom_config(home.as_path());
// Project release file.
let default_config = crate_root.join("release.toml");
let current_dir_config = get_config_from_file(&default_config)?;
if let Some(cfg) = current_dir_config {
config.update(&cfg);
};

// No project-wide configuration.
Ok(None)
Ok(config)
}

#[cfg(test)]
Expand All @@ -492,7 +499,7 @@ mod test {

#[test]
fn doesnt_panic() {
let release_config = resolve_config(Path::new("Cargo.toml")).unwrap().unwrap();
let release_config = resolve_config(Path::new("."), Path::new("Cargo.toml")).unwrap();
assert!(release_config.sign_commit());
}
}
Expand Down
30 changes: 21 additions & 9 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use structopt;
#[cfg(test)]
extern crate assert_fs;

use std::ffi::OsStr;
use std::path::Path;
use std::process::exit;

Expand Down Expand Up @@ -79,13 +80,17 @@ fn execute(args: &ReleaseOpt) -> Result<i32, error::FatalError> {
let cargo_file = cargo::parse_cargo_config(&manifest_path)?;
let custom_config_path_option = args.config.as_ref();
let release_config = {
let mut release_config = if let Some(custom_config_path) = custom_config_path_option {
// when calling with -c option
config::resolve_custom_config(Path::new(custom_config_path))?
} else {
config::resolve_config(&manifest_path)?
let mut release_config = config::Config::default();
if !args.isolated {
let cfg = config::resolve_config(&ws_meta.workspace_root, &manifest_path)?;
release_config.update(&cfg);
}
.unwrap_or_default();
if let Some(custom_config_path) = custom_config_path_option {
// when calling with -c option
let cfg =
config::resolve_custom_config(Path::new(custom_config_path))?.unwrap_or_default();
release_config.update(&cfg);
};
release_config.update(args);
release_config
};
Expand Down Expand Up @@ -270,9 +275,12 @@ fn execute(args: &ReleaseOpt) -> Result<i32, error::FatalError> {
if let Some(pre_rel_hook) = pre_release_hook {
shell::log_info(&format!("Calling pre-release hook: {:?}", pre_rel_hook));
let envs = btreemap! {
"PREV_VERSION" => prev_version_string.as_ref(),
"NEW_VERSION" => new_version_string.as_ref(),
"DRY_RUN" => if dry_run { "true" } else { "false" }
OsStr::new("PREV_VERSION") => prev_version_string.as_ref(),
epage marked this conversation as resolved.
Show resolved Hide resolved
OsStr::new("NEW_VERSION") => new_version_string.as_ref(),
OsStr::new("DRY_RUN") => OsStr::new(if dry_run { "true" } else { "false" }),
OsStr::new("CRATE_NAME") => OsStr::new(crate_name),
OsStr::new("WORKSPACE_ROOT") => ws_meta.workspace_root.as_os_str(),
OsStr::new("CRATE_ROOT") => manifest_path.parent().unwrap_or_else(|| Path::new(".")).as_os_str(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to add some tests to make sure this actually is useful for the various use cases.

I suspect it won't be and that we'll need to do replacements on the hook to allow it to be shared.

I decided against tracking the directory that defines the hook and setting the CWD to simplify the merging process.

};
// we use dry_run environmental variable to run the script
// so here we set dry_run=false and always execute the command.
Expand Down Expand Up @@ -410,6 +418,10 @@ struct ReleaseOpt {
/// Custom config file
config: Option<String>,
epage marked this conversation as resolved.
Show resolved Hide resolved

#[structopt(long = "isolated")]
/// Ignore implicit configuration files.
isolated: bool,

#[structopt(short = "m")]
/// Semver metadata
metadata: Option<String>,
Expand Down