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

fix: improve hook-env #3466

Merged
merged 2 commits into from
Dec 11, 2024
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
8 changes: 7 additions & 1 deletion docs/cli/activate.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# `mise activate`

- **Usage**: `mise activate [--shims] [-q --quiet] [SHELL_TYPE]`
- **Usage**: `mise activate [FLAGS] [SHELL_TYPE]`
- **Source code**: [`src/cli/activate.rs`](https://github.com/jdx/mise/blob/main/src/cli/activate.rs)

Initializes mise in the current shell session
Expand Down Expand Up @@ -48,6 +48,12 @@ Effectively the same as:

Suppress non-error messages

### `--no-hook-env`

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.

Examples:

eval "$(mise activate bash)"
Expand Down
2 changes: 1 addition & 1 deletion docs/cli/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Answer yes to all confirmation prompts

## Subcommands

- [`mise activate [--shims] [-q --quiet] [SHELL_TYPE]`](/cli/activate.md)
- [`mise activate [FLAGS] [SHELL_TYPE]`](/cli/activate.md)
- [`mise alias [-p --plugin <PLUGIN>] [--no-header] <SUBCOMMAND>`](/cli/alias.md)
- [`mise alias get <PLUGIN> <ALIAS>`](/cli/alias/get.md)
- [`mise alias ls [--no-header] [TOOL]`](/cli/alias/ls.md)
Expand Down
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:"
3 changes: 3 additions & 0 deletions mise.usage.kdl
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ Customize status output with `status` settings."#
long_help "Use shims instead of modifying PATH\nEffectively the same as:\n\n PATH=\"$HOME/.local/share/mise/shims:$PATH\""
}
flag "-q --quiet" help="Suppress non-error messages"
flag "--no-hook-env" help="Do not automatically call hook-env" {
long_help "Do not automatically call hook-env\n\nThis 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."
}
arg "[SHELL_TYPE]" help="Shell type to generate the script for" {
choices "bash" "elvish" "fish" "nu" "xonsh" "zsh"
}
Expand Down
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
21 changes: 8 additions & 13 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 @@ -138,18 +138,13 @@ impl HookEnv {
) -> Result<Vec<EnvDiffOperation>> {
let full = join_paths(&*env::PATH)?.to_string_lossy().to_string();
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 => (
split_paths(pre).collect_vec(),
split_paths(&format!("{orig_path}{post}")).collect_vec(),
),
_ => (vec![], split_paths(&full).collect_vec()),
}
}
Some(orig_path) => match full.split_once(&format!("{PATH_ENV_SEP}{orig_path}")) {
Some((pre, post)) if !settings.activate_aggressive => (
split_paths(pre).collect_vec(),
split_paths(&format!("{orig_path}{post}")).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,
Copy link
Owner Author

Choose a reason for hiding this comment

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

my understanding is this shouldn't work because we serialize to messagepack which doesn't have bigint... but it makes the test pass so ¯\_(ツ)_/¯

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
28 changes: 21 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 @@ -114,6 +122,7 @@ impl Display for Bash {
#[cfg(test)]
mod tests {
use insta::assert_snapshot;
use std::path::Path;
use test_log::test;

use crate::test::replace_path;
Expand All @@ -124,7 +133,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
19 changes: 13 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 @@ -93,6 +94,7 @@ impl Display for Elvish {
#[cfg(test)]
mod tests {
use insta::assert_snapshot;
use std::path::Path;
use test_log::test;

use crate::test::replace_path;
Expand All @@ -103,7 +105,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
Loading
Loading