Skip to content

Commit 40a19b7

Browse files
Resolve tilde to home directory (#184)
We need the ability to specify files in the home directory as a `check.path`. For example, `mysql` uses the following config locations. > Default options are read from the following files in the given order: > /etc/my.cnf /etc/mysql/my.cnf /opt/homebrew/etc/my.cnf ~/.my.cnf We use the one in the home directory to add to the defaults placed in the other directories. Here is an example config ```yaml apiVersion: scope.github.com/v1alpha kind: ScopeDoctorGroup metadata: name: home-directory description: Cache on a file in user's home directory spec: include: when-required actions: - name: home-directory check: paths: - ~/.doesnotexist commands: - test -f ~/.doesnotexist fix: commands: - touch ~/.doesnotexist ``` Currently, the `make_absolute` function https://github.com/oscope-dev/scope/blob/0d96462f809d1af5ff08e662d37f4b8813122fd6/scope/src/doctor/check.rs#L574-L580 would receive these parameters ```rust base_dir = /Users/chris.mcclellan/src/scope/examples glob = ~/.doesnotexist ``` and return the following string ```txt /Users/chris.mcclellan/src/scope/examples/~/.doesnotexist ``` Where what we really need is the glob to expand into the user's home directory. For clarity, specifying the absolute path to the user's directory works right now ```yaml paths: - /Users/chris.mcclellan/.doesnotexist ``` but that will obviously be different for every user. f6d0a9f has some tests illustrating current and desired behavior. It's also worth noting that environment variables do not expand either. I did not include that in this PR, but the `shellexpand` library I used for tilde expansion also [allows for environment variable expansion](https://docs.rs/shellexpand/latest/shellexpand/fn.full.html). I was not sure if we want to allow that, but it is a simple change to make if that is desired. While implementing this, I also noticed that `check` and `fix` `commands` are not expanded. https://github.com/oscope-dev/scope/blob/41cb86feea7e913ed3eb647aecfbf96625f6f733/examples/.scope/home-directory.yaml#L14-L27 For a consistent user experience, I figured these should expand as well, removing the need for calling a bash script in the commit above. Testing --- I added unit tests for this feature, but it can also be tested with this command. ```sh cargo run -- doctor run --working-dir examples --only home-directory --cache-dir . --progress=plain ``` - Run it once to verify the check fails and fix works. - Inspect the `cache-file.json` file to verify the file in the home directory is cached - Run it again to verify the check succeeds
1 parent 0d96462 commit 40a19b7

File tree

5 files changed

+271
-6
lines changed

5 files changed

+271
-6
lines changed

Cargo.lock

+19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,4 @@ tonic = "0.11.0"
9191
gethostname = "0.4.3"
9292
normpath = "1.3.0"
9393
fake = "2.10.0"
94+
shellexpand = "3.1.0"

examples/.scope/home-directory.yaml

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
apiVersion: scope.github.com/v1alpha
2+
kind: ScopeDoctorGroup
3+
metadata:
4+
name: home-directory
5+
description: Cache on a file in user's home directory
6+
spec:
7+
include: when-required
8+
actions:
9+
- name: home-directory
10+
check:
11+
paths:
12+
- ~/.doesnotexist
13+
# alternatively, tilde expansion works in commands too
14+
# if you don't want to cache the file
15+
commands:
16+
- test -f ~/.doesnotexist
17+
fix:
18+
commands:
19+
- touch ~/.doesnotexist

scope/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ tonic.workspace = true
8080
gethostname.workspace = true
8181
normpath.workspace = true
8282
fake.workspace = true
83+
shellexpand.workspace = true
8384

8485
[dev-dependencies]
8586
assert_cmd = "2.0.16"

scope/src/doctor/check.rs

+231-6
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ impl DefaultDoctorActionRun {
352352
}
353353

354354
async fn run_single_fix(&self, command: &str) -> Result<ActionTaskReport, RuntimeError> {
355-
let args = vec![command.to_string()];
355+
let args = vec![Self::expand_command(command)];
356356
let capture = self
357357
.exec_runner
358358
.run_command(CaptureOpts {
@@ -467,7 +467,7 @@ impl DefaultDoctorActionRun {
467467
let mut result: Option<CacheStatus> = None;
468468

469469
for command in &action_command.commands {
470-
let args = vec![command.clone()];
470+
let args = vec![Self::expand_command(command)];
471471
let path = format!(
472472
"{}:{}",
473473
self.model.metadata().containing_dir(),
@@ -514,6 +514,16 @@ impl DefaultDoctorActionRun {
514514
output: Some(action_reports),
515515
})
516516
}
517+
518+
/// splits a commands and performs shell expansion its parts
519+
fn expand_command(command: &str) -> String {
520+
command
521+
.split(' ')
522+
//consider doing a full expansion that includes env vars?
523+
.map(|word| shellexpand::tilde(word))
524+
.collect::<Vec<_>>()
525+
.join(" ")
526+
}
517527
}
518528

519529
#[automock]
@@ -572,10 +582,12 @@ impl Default for DefaultGlobWalker {
572582
}
573583

574584
fn make_absolute(base_dir: &Path, glob: &String) -> String {
575-
if glob.starts_with('/') {
576-
glob.to_string()
585+
let expanded_glob = shellexpand::tilde(glob);
586+
let glob_buf = PathBuf::from(expanded_glob.to_string());
587+
if glob_buf.is_absolute() {
588+
glob_buf.display().to_string()
577589
} else {
578-
format!("{}/{}", base_dir.display(), glob)
590+
base_dir.join(glob_buf).display().to_string()
579591
}
580592
}
581593

@@ -638,6 +650,7 @@ pub(crate) mod tests {
638650
use crate::doctor::tests::build_root_model;
639651
use crate::shared::prelude::*;
640652
use anyhow::{anyhow, Result};
653+
use directories::UserDirs;
641654
use predicates::prelude::predicate;
642655
use std::path::{Path, PathBuf};
643656
use std::sync::Arc;
@@ -723,10 +736,11 @@ pub(crate) mod tests {
723736
.withf(move |params| {
724737
params.args[0].eq(command) && params.env_vars.contains_key("SCOPE_BIN_DIR")
725738
})
726-
.returning(move |_| {
739+
.returning(move |capture_opts| {
727740
let resp_code = expected_results[counter];
728741
counter += 1;
729742
Ok(OutputCaptureBuilder::default()
743+
.command(capture_opts.args[0].to_string())
730744
.exit_code(Some(resp_code))
731745
.build()
732746
.unwrap())
@@ -765,6 +779,11 @@ pub(crate) mod tests {
765779
false
766780
}
767781

782+
fn home_dir() -> PathBuf {
783+
let user_dirs = UserDirs::new().expect("Couldn't initialize UserDirs");
784+
user_dirs.home_dir().to_owned()
785+
}
786+
768787
#[tokio::test]
769788
async fn test_only_exec_will_check_passes() -> Result<()> {
770789
let action = build_run_fail_fix_succeed_action();
@@ -1040,4 +1059,210 @@ pub(crate) mod tests {
10401059
.await;
10411060
assert!(res.is_ok());
10421061
}
1062+
1063+
#[tokio::test]
1064+
async fn test_glob_walker_update_path_honors_tilde_paths() {
1065+
let mut file_system = MockFileSystem::new();
1066+
let mut file_cache = MockFileCache::new();
1067+
1068+
let home_dir = home_dir();
1069+
let resolved_path = home_dir.join("path/foo.txt").as_path().to_owned();
1070+
1071+
file_cache
1072+
.expect_update_cache_entry()
1073+
.once()
1074+
.with(
1075+
predicate::eq("group_name".to_string()),
1076+
predicate::eq(resolved_path.clone()),
1077+
)
1078+
.returning(|_, _| Ok(()));
1079+
1080+
file_system
1081+
.expect_find_files()
1082+
.once()
1083+
.withf(move |glob_pattern| {
1084+
home_dir
1085+
.join("path/*.txt")
1086+
.as_path()
1087+
.to_owned()
1088+
.to_str()
1089+
.unwrap()
1090+
== glob_pattern
1091+
})
1092+
.returning(move |_| Ok(vec![resolved_path.clone()]));
1093+
1094+
let walker = DefaultGlobWalker {
1095+
file_system: Box::new(file_system),
1096+
};
1097+
1098+
let res = walker
1099+
.update_cache(
1100+
Path::new("/foo/root"),
1101+
&["~/path/*.txt".to_string()],
1102+
"group_name",
1103+
Arc::new(file_cache),
1104+
)
1105+
.await;
1106+
assert!(res.is_ok());
1107+
}
1108+
1109+
#[tokio::test]
1110+
async fn test_glob_walker_update_path_honors_relative_paths() {
1111+
// I can make an argument that we should toss this test
1112+
// and say that the correct thing to do is use **/path/*.txt
1113+
let mut file_system = MockFileSystem::new();
1114+
let mut file_cache = MockFileCache::new();
1115+
1116+
file_cache
1117+
.expect_update_cache_entry()
1118+
.once()
1119+
.with(
1120+
predicate::eq("group_name".to_string()),
1121+
predicate::eq(Path::new("/foo/path/foo.txt")),
1122+
)
1123+
.returning(|_, _| Ok(()));
1124+
1125+
file_system
1126+
.expect_find_files()
1127+
.once()
1128+
//glob() will error on relative paths! This is wrong!
1129+
.with(predicate::eq("/foo/root/../path/*.txt"))
1130+
// this is the correct expectation
1131+
// .with(predicate::eq("/foo/path/*.txt"))
1132+
.returning(|_| Ok(vec![PathBuf::from("/foo/path/foo.txt")]));
1133+
1134+
let walker = DefaultGlobWalker {
1135+
file_system: Box::new(file_system),
1136+
};
1137+
1138+
let res = walker
1139+
.update_cache(
1140+
Path::new("/foo/root"),
1141+
&["../path/*.txt".to_string()],
1142+
"group_name",
1143+
Arc::new(file_cache),
1144+
)
1145+
.await;
1146+
assert!(res.is_ok());
1147+
}
1148+
1149+
mod test_run_single_fix {
1150+
use super::*;
1151+
1152+
#[tokio::test]
1153+
async fn expands_tilde_to_home_dir() {
1154+
let mut exec_runner = MockExecutionProvider::new();
1155+
exec_runner
1156+
.expect_run_command()
1157+
.once()
1158+
.returning(move |capture_opts| {
1159+
Ok(OutputCaptureBuilder::default()
1160+
.command(capture_opts.args[0].to_string())
1161+
.exit_code(Some(0))
1162+
.build()
1163+
.unwrap())
1164+
});
1165+
1166+
let run = setup_test(
1167+
vec![build_run_fail_fix_succeed_action()],
1168+
exec_runner,
1169+
MockGlobWalker::new(),
1170+
);
1171+
1172+
let result = run.run_single_fix("touch ~/.somefile").await.unwrap();
1173+
1174+
assert_eq!(
1175+
format!("{} {}", "touch", home_dir().join(".somefile").display()),
1176+
result.command
1177+
);
1178+
}
1179+
}
1180+
1181+
mod test_run_check_command {
1182+
use super::*;
1183+
1184+
#[tokio::test]
1185+
async fn expands_tilde_to_home_dir() {
1186+
let mut exec_runner = MockExecutionProvider::new();
1187+
exec_runner
1188+
.expect_run_command()
1189+
.once()
1190+
.returning(move |capture_opts| {
1191+
Ok(OutputCaptureBuilder::default()
1192+
.command(capture_opts.args[0].to_string())
1193+
.exit_code(Some(0))
1194+
.build()
1195+
.unwrap())
1196+
});
1197+
1198+
let run = setup_test(
1199+
vec![build_run_fail_fix_succeed_action()],
1200+
exec_runner,
1201+
MockGlobWalker::new(),
1202+
);
1203+
1204+
let action_commands = DoctorGroupActionCommandBuilder::default()
1205+
.commands(vec!["test -f ~/.somefile".to_string()])
1206+
.build()
1207+
.unwrap();
1208+
1209+
let result = run.run_check_command(&action_commands).await.unwrap();
1210+
let reports = result.output.expect("Expected ActionTaskreports");
1211+
assert_eq!(
1212+
format!("{} {}", "test -f", home_dir().join(".somefile").display()),
1213+
reports[0].command
1214+
);
1215+
}
1216+
}
1217+
1218+
mod test_make_absolute {
1219+
use super::*;
1220+
use crate::doctor::check::make_absolute;
1221+
1222+
#[test]
1223+
fn filename_gets_preprended_with_basepath() {
1224+
let base_dir = Path::new("/Users/first.last/path/to/project");
1225+
let glob = "foo.txt".to_string();
1226+
1227+
let actual = make_absolute(base_dir, &glob);
1228+
assert_eq!("/Users/first.last/path/to/project/foo.txt", &actual)
1229+
}
1230+
1231+
#[test]
1232+
fn wildcard_does_not_get_replaced() {
1233+
let base_dir = Path::new("/Users/first.last/path/to/project");
1234+
let glob = "*.txt".to_string();
1235+
1236+
let actual = make_absolute(base_dir, &glob);
1237+
assert_eq!("/Users/first.last/path/to/project/*.txt", &actual)
1238+
}
1239+
1240+
#[test]
1241+
fn path_from_root_does_not_get_replaced() {
1242+
let base_dir = Path::new("/Users/first.last/path/to/project");
1243+
let glob = "/etc/project/foo.txt".to_string();
1244+
1245+
let actual = make_absolute(base_dir, &glob);
1246+
assert_eq!("/etc/project/foo.txt", &actual)
1247+
}
1248+
1249+
#[test]
1250+
fn relative_paths_are_not_resolved() {
1251+
let base_dir = Path::new("/Users/first.last/path/to/project");
1252+
let glob = "../foo.txt".to_string();
1253+
1254+
let actual = make_absolute(base_dir, &glob);
1255+
assert_eq!("/Users/first.last/path/to/project/../foo.txt", &actual)
1256+
}
1257+
1258+
#[test]
1259+
fn tilde_resolves() {
1260+
let home_dir = home_dir();
1261+
let base_dir = home_dir.join("path/to/project");
1262+
let glob = "~/foo.txt".to_string();
1263+
1264+
let actual = make_absolute(base_dir.as_path(), &glob);
1265+
assert_eq!(home_dir.join("foo.txt").display().to_string(), actual);
1266+
}
1267+
}
10431268
}

0 commit comments

Comments
 (0)