Skip to content

Commit

Permalink
fix: Fix auto-install re-installing tools. (#256)
Browse files Browse the repository at this point in the history
  • Loading branch information
milesj authored Oct 24, 2023
1 parent 92c9406 commit 9b3834a
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 16 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
- [Rust](https://github.com/moonrepo/rust-plugin/blob/master/CHANGELOG.md)
- [Schema](https://github.com/moonrepo/schema-plugin/blob/master/CHANGELOG.md)

## Unreleased

#### 🐞 Fixes

- Fixed an issue where auto-install would keep re-installing a tool.

## 0.20.3

#### 🚀 Updates
Expand Down
14 changes: 7 additions & 7 deletions crates/cli/src/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn pin_version(

// via `pin-latest` setting
if initial_version.is_latest() {
let user_config = tool.proto.get_user_config()?;
let user_config = tool.proto.load_user_config()?;

if let Some(pin_type) = user_config.pin_latest {
args.global = matches!(pin_type, PinType::Global);
Expand All @@ -73,7 +73,7 @@ fn pin_version(
Ok(())
}

pub async fn internal_install(args: InstallArgs, tool: Option<Tool>) -> SystemResult {
pub async fn internal_install(args: InstallArgs, tool: Option<Tool>) -> miette::Result<Tool> {
let mut tool = match tool {
Some(tool) => tool,
None => load_tool(&args.id).await?,
Expand All @@ -97,7 +97,7 @@ pub async fn internal_install(args: InstallArgs, tool: Option<Tool>) -> SystemRe
color::path(tool.get_tool_dir()),
);

return Ok(());
return Ok(tool);
}

if tool.disable_progress_bars() {
Expand Down Expand Up @@ -135,7 +135,7 @@ pub async fn internal_install(args: InstallArgs, tool: Option<Tool>) -> SystemRe
pb.finish_and_clear();

if !installed {
return Ok(());
return Ok(tool);
}

pin_version(&mut tool, &version, args.pin)?;
Expand All @@ -154,17 +154,17 @@ pub async fn internal_install(args: InstallArgs, tool: Option<Tool>) -> SystemRe
})?;

// Sync shell profile
update_shell(tool, args.passthrough.clone())?;
update_shell(&tool, args.passthrough.clone())?;

// Clean plugins
debug!("Auto-cleaning plugins");

clean_plugins(7).await?;

Ok(())
Ok(tool)
}

fn update_shell(tool: Tool, passthrough_args: Vec<String>) -> miette::Result<()> {
fn update_shell(tool: &Tool, passthrough_args: Vec<String>) -> miette::Result<()> {
if !tool.plugin.has_func("sync_shell_profile") {
return Ok(());
}
Expand Down
9 changes: 3 additions & 6 deletions crates/cli/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
}

let version = detect_version(&tool, args.spec.clone()).await?;
let user_config = tool.proto.get_user_config()?;
let user_config = tool.proto.load_user_config()?;

// Check if installed or install
if !tool.is_setup(&version).await? {
Expand All @@ -78,20 +78,17 @@ pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
// Install the tool
debug!("Auto-install setting is configured, attempting to install");

internal_install(
tool = internal_install(
InstallArgs {
canary: false,
id: args.id.clone(),
pin: false,
passthrough: vec![],
spec: Some(tool.get_resolved_version().to_unresolved_spec()),
},
None,
Some(tool),
)
.await?;

// Find the new binaries
tool.locate_bins().await?;
}

// Determine the binary path to execute
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub struct ToolsLoader {
impl ToolsLoader {
pub fn new() -> miette::Result<Self> {
let proto = ProtoEnvironment::new()?;
let user_config = proto.get_user_config()?;
let user_config = proto.load_user_config()?;

let mut tools_config = ToolsConfig::load_upwards_from(&proto.cwd, false)?;
tools_config.inherit_builtin_plugins();
Expand Down
29 changes: 29 additions & 0 deletions crates/cli/tests/run_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,35 @@ mod run {
));
}

#[test]
fn doesnt_auto_install_subsequently() {
let temp = create_empty_sandbox();

temp.create_file("config.toml", "auto-install = true");

let mut cmd = create_proto_command(temp.path());
let assert = cmd
.arg("run")
.arg("node")
.arg("19.0.0")
.arg("--")
.arg("--version")
.assert();

assert.stderr(predicate::str::contains("Node.js has been installed"));

let mut cmd = create_proto_command(temp.path());
let assert = cmd
.arg("run")
.arg("node")
.arg("19.0.0")
.arg("--")
.arg("--version")
.assert();

assert.stderr(predicate::str::contains("Node.js has been installed").not());
}

#[test]
fn errors_if_plugin_not_configured() {
let temp = create_empty_sandbox();
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl ProtoEnvironment {
})
}

pub fn get_user_config(&self) -> miette::Result<UserConfig> {
pub fn load_user_config(&self) -> miette::Result<UserConfig> {
UserConfig::load_from(&self.root)
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/tool_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub async fn load_tool_from_locator(

pub async fn load_tool(id: &Id) -> miette::Result<Tool> {
let proto = ProtoEnvironment::new()?;
let user_config = proto.get_user_config()?;
let user_config = proto.load_user_config()?;
let mut locator = None;

debug!(
Expand Down

0 comments on commit 9b3834a

Please sign in to comment.