Skip to content

Commit

Permalink
fix: improve hook-env
Browse files Browse the repository at this point in the history
* check config filetime modification in millis so scripts work better
* only warn if files are untrusted
  • Loading branch information
jdx committed Dec 11, 2024
1 parent 816d6ee commit 7ad6a2b
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 56 deletions.
14 changes: 14 additions & 0 deletions e2e/cli/test_activate_aggressive
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash

ORIG_PATH="$PATH"
eval "$(mise activate bash)"
export PATH="/added:$PATH"

assert "mise hook-env -s bash --trace" "" # checking early exit functions
mkdir -p "$MISE_DATA_DIR/installs/dummy/1.0.0/bin"
echo "#!/usr/bin/env bash" >"$MISE_DATA_DIR/installs/dummy/1.0.0/bin/dummy"
chmod +x "$MISE_DATA_DIR/installs/dummy/1.0.0/bin/dummy"
echo "tools.dummy = '1'" >mise.toml
assert_contains "mise hook-env -s bash --trace" "export PATH='/added:$MISE_DATA_DIR/installs/dummy/1.0.0/bin:"
export MISE_ACTIVATE_AGGRESSIVE=1
assert_contains "mise hook-env -s bash --trace" "export PATH='$MISE_DATA_DIR/installs/dummy/1.0.0/bin:/added:"
20 changes: 18 additions & 2 deletions src/cli/activate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use crate::env::PATH_KEY;
use crate::file::touch_dir;
use crate::path_env::PathEnv;
use crate::shell::{get_shell, Shell, ShellType};
use crate::shell::{get_shell, ActivateOptions, Shell, ShellType};
use crate::{dirs, env};
use eyre::Result;
use itertools::Itertools;
Expand Down Expand Up @@ -49,6 +49,15 @@ pub struct Activate {
/// Suppress non-error messages
#[clap(long, short)]
quiet: bool,

/// Do not automatically call hook-env
///
/// This can be helpful for debugging mise. If you run `eval "$(mise activate --no-hook-env)"`, then
/// you can call `mise hook-env` manually which will output the env vars to stdout without actually
/// modifying the environment. That way you can do things like `mise hook-env --trace` to get more
/// information or just see the values that hook-env is outputting.
#[clap(long)]
no_hook_env: bool,
}

impl Activate {
Expand Down Expand Up @@ -91,7 +100,14 @@ impl Activate {
flags.push(" --status");
}
miseprint!("{}", self.prepend_path(shell, exe_dir))?;
miseprint!("{}", shell.activate(mise_bin, flags.join("")))?;
miseprint!(
"{}",
shell.activate(ActivateOptions {
exe: mise_bin.to_path_buf(),
flags: flags.join(""),
no_hook_env: self.no_hook_env,
})
)?;
Ok(())
}

Expand Down
19 changes: 9 additions & 10 deletions src/cli/hook_env.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::env::{join_paths, split_paths, PATH_ENV_SEP};
use console::truncate_str;
use eyre::Result;
use itertools::Itertools;
use std::env::{join_paths, split_paths};
use std::ops::Deref;
use std::path::PathBuf;

Expand Down Expand Up @@ -137,19 +137,18 @@ impl HookEnv {
to_remove: &Vec<PathBuf>,
) -> Result<Vec<EnvDiffOperation>> {
let full = join_paths(&*env::PATH)?.to_string_lossy().to_string();
dbg!(&*env::__MISE_ORIG_PATH);
let (pre, post) = match &*env::__MISE_ORIG_PATH {
Some(orig_path) => {
match full.split_once(&format!(
"{}{orig_path}",
if cfg!(windows) { ';' } else { ':' }
)) {
Some((pre, post)) if !settings.activate_aggressive => (
Some(orig_path) => match full.split_once(&format!("{PATH_ENV_SEP}{orig_path}")) {
Some((pre, post)) if !settings.activate_aggressive => {
dbg!(pre, post);
(
split_paths(pre).collect_vec(),
split_paths(&format!("{orig_path}{post}")).collect_vec(),
),
_ => (vec![], split_paths(&full).collect_vec()),
)
}
}
_ => (vec![], split_paths(&full).collect_vec()),
},
None => (vec![], split_paths(&full).collect_vec()),
};

Expand Down
2 changes: 1 addition & 1 deletion src/config/config_file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ pub fn parse_or_init(path: &Path) -> eyre::Result<Box<dyn ConfigFile>> {
Ok(cf)
}

pub fn parse(path: &Path) -> eyre::Result<Box<dyn ConfigFile>> {
pub fn parse(path: &Path) -> Result<Box<dyn ConfigFile>> {
if let Ok(settings) = Settings::try_get() {
if settings.paranoid {
trust_check(path)?;
Expand Down
22 changes: 15 additions & 7 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,19 +900,27 @@ fn load_all_config_files(
.collect_vec()
.into_par_iter()
.map(|f| {
let cf = parse_config_file(f, idiomatic_filenames).wrap_err_with(|| {
format!(
"error parsing config file: {}",
style::ebold(display_path(f))
)
})?;
let cf = match parse_config_file(f, idiomatic_filenames) {
Ok(cfg) => cfg,
Err(err) => {
if err.to_string().contains("are not trusted.") {
warn!("{err}");
return Ok(None);
}
return Err(err.wrap_err(format!(
"error parsing config file: {}",
style::ebold(display_path(f))
)));
}
};
if let Err(err) = Tracker::track(f) {
warn!("tracking config: {err:#}");
}
Ok((f.clone(), cf))
Ok(Some((f.clone(), cf)))
})
.collect::<Result<Vec<_>>>()?
.into_iter()
.flatten()
.collect())
}

Expand Down
8 changes: 7 additions & 1 deletion src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ pub static __MISE_DIR: Lazy<Option<PathBuf>> = Lazy::new(|| {
pub static __MISE_ORIG_PATH: Lazy<Option<String>> = Lazy::new(|| var("__MISE_ORIG_PATH").ok());
pub static __MISE_WATCH: Lazy<Option<HookEnvWatches>> = Lazy::new(|| match var("__MISE_WATCH") {
Ok(raw) => deserialize_watches(raw)
.map_err(|e| warn!("Failed to deserialize __MISE_WATCH {e}"))
// TODO: enable this later when the bigint change goes out
.map_err(|e| debug!("Failed to deserialize __MISE_WATCH {e}"))
.ok(),
_ => None,
});
Expand Down Expand Up @@ -294,6 +295,11 @@ pub static NVM_DIR: Lazy<PathBuf> =
pub static NODENV_ROOT: Lazy<PathBuf> =
Lazy::new(|| var_path("NODENV_ROOT").unwrap_or_else(|| HOME.join(".nodenv")));

#[cfg(unix)]
pub const PATH_ENV_SEP: char = ':';
#[cfg(windows)]
pub const PATH_ENV_SEP: char = ';';

fn get_env_diff() -> EnvDiff {
let env = vars().collect::<HashMap<_, _>>();
match env.get("__MISE_DIFF") {
Expand Down
8 changes: 4 additions & 4 deletions src/hook_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn have_files_been_modified(watches: &HookEnvWatches, watch_files: BTreeSet<Path
let modtime = modtime
.duration_since(SystemTime::UNIX_EPOCH)
.unwrap()
.as_secs();
.as_millis();
if modtime > watches.latest_update {
trace!("file modified: {:?}", fp);
modified = true;
Expand All @@ -100,7 +100,7 @@ fn have_files_been_modified(watches: &HookEnvWatches, watch_files: BTreeSet<Path
}
}
if !modified {
trace!("config files unmodified");
trace!("watch files unmodified");
}
modified
}
Expand All @@ -114,7 +114,7 @@ fn have_mise_env_vars_been_modified(watches: &HookEnvWatches) -> bool {

#[derive(Debug, Default, Serialize, Deserialize)]
pub struct HookEnvWatches {
latest_update: u64,
latest_update: u128,
env_var_hash: String,
}

Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn build_watches(
latest_update: max_modtime
.duration_since(SystemTime::UNIX_EPOCH)
.unwrap()
.as_secs(),
.as_millis(),
})
}

Expand Down
27 changes: 20 additions & 7 deletions src/shell/bash.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
use std::fmt::Display;
use std::path::Path;

use indoc::formatdoc;

use crate::config::Settings;
use crate::shell::Shell;
use crate::shell::{ActivateOptions, Shell};

#[derive(Default)]
pub struct Bash {}

impl Shell for Bash {
fn activate(&self, exe: &Path, flags: String) -> String {
fn activate(&self, opts: ActivateOptions) -> String {
let exe = opts.exe;
let flags = opts.flags;
let settings = Settings::get();
let exe = exe.to_string_lossy();
let mut out = formatdoc! {r#"
Expand Down Expand Up @@ -43,14 +44,21 @@ impl Shell for Bash {
eval "$(mise hook-env{flags} -s bash)";
return $previous_exit_status;
}};
"#};
if !opts.no_hook_env {
out.push_str(&formatdoc! {r#"
if [[ ";${{PROMPT_COMMAND:-}};" != *";_mise_hook;"* ]]; then
PROMPT_COMMAND="_mise_hook${{PROMPT_COMMAND:+;$PROMPT_COMMAND}}"
fi
{}
{}
{chpwd_functions}
{chpwd_load}
chpwd_functions+=(_mise_hook)
_mise_hook
"#, include_str!("../assets/bash_zsh_support/chpwd/function.sh"), include_str!("../assets/bash_zsh_support/chpwd/load.sh")};
"#,
chpwd_functions = include_str!("../assets/bash_zsh_support/chpwd/function.sh"),
chpwd_load = include_str!("../assets/bash_zsh_support/chpwd/load.sh")
});
}
if settings.not_found_auto_install {
out.push_str(&formatdoc! {r#"
if [ -z "${{_mise_cmd_not_found:-}}" ]; then
Expand Down Expand Up @@ -124,7 +132,12 @@ mod tests {
fn test_activate() {
let bash = Bash::default();
let exe = Path::new("/some/dir/mise");
assert_snapshot!(bash.activate(exe, " --status".into()));
let opts = ActivateOptions {
exe: exe.to_path_buf(),
flags: " --status".into(),
no_hook_env: false,
};
assert_snapshot!(bash.activate(opts));
}

#[test]
Expand Down
18 changes: 12 additions & 6 deletions src/shell/elvish.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use std::fmt::Display;
use std::path::Path;

use crate::shell::Shell;
use crate::shell::{ActivateOptions, Shell};
use indoc::formatdoc;

#[derive(Default)]
pub struct Elvish {}

impl Shell for Elvish {
fn activate(&self, exe: &Path, flags: String) -> String {
fn activate(&self, opts: ActivateOptions) -> String {
let exe = opts.exe;
let flags = opts.flags;
let exe = exe.to_string_lossy();

formatdoc! {r#"
Expand All @@ -25,7 +26,7 @@ impl Shell for Elvish {
fn activate {{
set-env MISE_SHELL elvish
set hook-enabled = $true
set hook-enabled = ${hook_enabled}
hook-env
}}
Expand Down Expand Up @@ -55,7 +56,7 @@ impl Shell for Elvish {
}}
{exe} $@a
}}
"#}
"#, hook_enabled = !opts.no_hook_env}
}

fn deactivate(&self) -> String {
Expand Down Expand Up @@ -103,7 +104,12 @@ mod tests {
fn test_hook_init() {
let elvish = Elvish::default();
let exe = Path::new("/some/dir/mise");
assert_snapshot!(elvish.activate(exe, " --status".into()));
let opts = ActivateOptions {
exe: exe.to_path_buf(),
flags: " --status".into(),
no_hook_env: false,
};
assert_snapshot!(elvish.activate(opts));
}

#[test]
Expand Down
18 changes: 14 additions & 4 deletions src/shell/fish.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
use std::fmt::{Display, Formatter};
use std::path::Path;

use crate::config::Settings;
use crate::shell::Shell;
use crate::shell::{ActivateOptions, Shell};
use indoc::formatdoc;
use shell_escape::unix::escape;

#[derive(Default)]
pub struct Fish {}

impl Shell for Fish {
fn activate(&self, exe: &Path, flags: String) -> String {
fn activate(&self, opts: ActivateOptions) -> String {
let exe = opts.exe;
let flags = opts.flags;
let exe = exe.to_string_lossy();
let description = "'Update mise environment when changing directories'";
let mut out = String::new();
Expand Down Expand Up @@ -49,7 +50,10 @@ impl Shell for Fish {
command {exe} "$command" $argv
end
end
"#});

if !opts.no_hook_env {
out.push_str(&formatdoc! {r#"
function __mise_env_eval --on-event fish_prompt --description {description};
{exe} hook-env{flags} -s fish | source;
Expand All @@ -76,6 +80,7 @@ impl Shell for Fish {
__mise_env_eval
"#});
}
if Settings::get().not_found_auto_install {
out.push_str(&formatdoc! {r#"
if functions -q fish_command_not_found; and not functions -q __mise_fish_command_not_found
Expand Down Expand Up @@ -146,7 +151,12 @@ mod tests {
fn test_activate() {
let fish = Fish::default();
let exe = Path::new("/some/dir/mise");
assert_snapshot!(fish.activate(exe, " --status".into()));
let opts = ActivateOptions {
exe: exe.to_path_buf(),
flags: " --status".into(),
no_hook_env: false,
};
assert_snapshot!(fish.activate(opts));
}

#[test]
Expand Down
10 changes: 8 additions & 2 deletions src/shell/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::env;
use std::fmt::{Display, Formatter};
use std::path::Path;
use std::path::PathBuf;
use std::str::FromStr;

mod bash;
Expand Down Expand Up @@ -73,13 +73,19 @@ impl FromStr for ShellType {
}

pub trait Shell: Display {
fn activate(&self, exe: &Path, flags: String) -> String;
fn activate(&self, opts: ActivateOptions) -> String;
fn deactivate(&self) -> String;
fn set_env(&self, k: &str, v: &str) -> String;
fn prepend_env(&self, k: &str, v: &str) -> String;
fn unset_env(&self, k: &str) -> String;
}

pub struct ActivateOptions {
pub exe: PathBuf,
pub flags: String,
pub no_hook_env: bool,
}

pub fn get_shell(shell: Option<ShellType>) -> Option<Box<dyn Shell>> {
shell.or_else(ShellType::load).map(|st| st.as_shell())
}
Loading

0 comments on commit 7ad6a2b

Please sign in to comment.