Repository: FuelLabs/fuelup (rust)

Issue:
`forc deploy` binary not found when fuel-toolchain.toml is present
### Problem

When inside a directory with a fuel-toolchain.toml ([example](https://github.com/sarahschwartz/sway-farm/tree/main/contract)), `fuel deploy --help` executes the `forc` executable rather than `forc-deploy`

If I use the full path to the executable, i.e. `$HOME/.fuelup/toolchains/beta-3-aarch64-apple-darwin/bin/forc-deploy --help`, it executes `forc-deploy` correctly. However, if I use `$HOME/.fuelup/bin/forc-deploy --help`, it does not work.

### Steps

If I use the full path to the executable, i.e. `$HOME/.fuelup/toolchains/beta-3-aarch64-apple-darwin/bin/forc-deploy --help`, it executes `forc-deploy` correctly. However, if I use `$HOME/.fuelup/bin/forc-deploy --help`, it does not work.

### Possible Solution(s)

_No response_

### Notes

_No response_

### Fuelup version

```console
fuelup 0.18.1
```


### Installed components

```console
beta-3-aarch64-apple-darwin
  forc - Up to date : 0.35.5
  fuel-core - Up to date : 0.17.4
  fuel-indexer - Up to date : 0.6.0
fuelup - Up to date : 0.18.1
```


Pull request:
fix: proxy_cli erroneously executes `forc` when forc plugins are called
closes #422 

The entire bug was a result of a one-line mistake:

https://github.com/FuelLabs/fuelup/blob/784edcddd8dc28b7499b4410d5c3fc8802ebe281/src/proxy_cli.rs#L74

If you look at the source code you can see that `component_name` is used to derive the path of a binary if a version was specified in an override. In the case of forc, we require this `component_name` instead of using `proc_name` as seen in the function because forc plugins are packaged together with forc - which means if we want to find  the directory that contains `forc-deploy v0.35.5`, this would be found in `forc-0.35.5`, which means we would have to use `component_name` (forc) instead of `proc_name` (forc-deploy, for example)

The bug was caused by using the `component_name` to invoke the binary instead of using the `proc_name` in the case that an entire toolchain was overridden. In that scenario, since that specific version is already linked to the toolchain directory, we do not have to derive the path, we just have to call what's inside the toolchain bin directory, i.e. `~/.fuelup/toolchains/beta-3.../bin/forc-deploy`.

I also added a simple test in this PR to emulate the scenario with a custom injected override file so we can hopefully prevent any regressions on this behavior.

Patch:
diff --git a/src/file.rs b/src/file.rs
index 647157b2..39a6579f 100644
--- a/src/file.rs
+++ b/src/file.rs
@@ -14,7 +14,7 @@ pub(crate) fn hardlink(original: &Path, link: &Path) -> io::Result<()> {
     fs::hard_link(original, link)
 }
 
-pub(crate) fn hard_or_symlink_file(original: &Path, link: &Path) -> Result<()> {
+pub fn hard_or_symlink_file(original: &Path, link: &Path) -> Result<()> {
     if hardlink_file(original, link).is_err() {
         symlink_file(original, link)?;
     }
diff --git a/src/proxy_cli.rs b/src/proxy_cli.rs
index 49cb01af..a0c35c5e 100644
--- a/src/proxy_cli.rs
+++ b/src/proxy_cli.rs
@@ -50,7 +50,8 @@ fn direct_proxy(proc_name: &str, args: &[OsString], toolchain: &Toolchain) -> Re
             } else {
                 proc_name
             };
-            // Install components within [components] that are declared but missing from the store.
+
+            // If a specific version is declared, we want to call it from the store and not from the toolchain directory.
             if let Some(version) = to.get_component_version(component_name) {
                 let store = Store::from_env()?;
 
@@ -60,6 +61,7 @@ fn direct_proxy(proc_name: &str, args: &[OsString], toolchain: &Toolchain) -> Re
                         TargetTriple::from_component(component_name)?,
                         Some(version.clone()),
                     )?;
+                    // Install components within [components] that are declared but missing from the store.
                     store.install_component(&download_cfg)?;
                 };
 
@@ -70,10 +72,7 @@ fn direct_proxy(proc_name: &str, args: &[OsString], toolchain: &Toolchain) -> Re
                     description.to_string(),
                 )
             } else {
-                (
-                    toolchain.bin_path.join(component_name),
-                    description.to_string(),
-                )
+                (toolchain.bin_path.join(proc_name), description.to_string())
             }
         }
         None => (


Test patch:
diff --git a/tests/override.rs b/tests/override.rs
new file mode 100644
index 00000000..577964f5
--- /dev/null
+++ b/tests/override.rs
@@ -0,0 +1,42 @@
+use anyhow::Result;
+use std::str::FromStr;
+
+pub mod testcfg;
+use fuelup::{
+    constants::FUEL_TOOLCHAIN_TOML_FILE,
+    toolchain_override::{self, OverrideCfg, ToolchainCfg, ToolchainOverride},
+};
+use testcfg::FuelupState;
+
+#[test]
+fn check_correct_forc_plugin_called() -> Result<()> {
+    // We execute both 'forc wallet' and 'forc-wallet' in this test
+    // to ensure both work as intended.
+    //
+    // The test environment is set up similarly to a real fuelup environment,
+    // complete with links.
+    testcfg::setup(FuelupState::AllInstalled, &|cfg| {
+        let mut stdout = cfg.forc(&["wallet", "--version"]).stdout;
+        assert_eq!(stdout, "forc-wallet 0.1.0\n");
+        stdout = cfg.exec("forc-wallet", &["--version"]).stdout;
+        assert_eq!(stdout, "forc-wallet 0.1.0\n");
+
+        let toolchain_override = ToolchainOverride {
+            cfg: OverrideCfg::new(
+                ToolchainCfg {
+                    channel: toolchain_override::Channel::from_str("nightly-2022-08-30").unwrap(),
+                },
+                None,
+            ),
+            path: cfg.home.join(FUEL_TOOLCHAIN_TOML_FILE),
+        };
+        testcfg::setup_override_file(toolchain_override).unwrap();
+
+        stdout = cfg.forc(&["wallet", "--version"]).stdout;
+        assert_eq!(stdout, "forc-wallet 0.2.0\n");
+        stdout = cfg.exec("forc-wallet", &["--version"]).stdout;
+        assert_eq!(stdout, "forc-wallet 0.2.0\n");
+    })?;
+
+    Ok(())
+}
diff --git a/tests/testcfg/mod.rs b/tests/testcfg/mod.rs
index 2fedaacb..47941e1a 100644
--- a/tests/testcfg/mod.rs
+++ b/tests/testcfg/mod.rs
@@ -1,6 +1,7 @@
 use anyhow::Result;
 use fuelup::channel::{BETA_1, LATEST, NIGHTLY};
 use fuelup::constants::FUEL_TOOLCHAIN_TOML_FILE;
+use fuelup::file::hard_or_symlink_file;
 use fuelup::settings::SettingsFile;
 use fuelup::target_triple::TargetTriple;
 use fuelup::toolchain_override::{self, OverrideCfg, ToolchainCfg, ToolchainOverride};
@@ -42,7 +43,11 @@ pub enum FuelupState {
 #[derive(Debug)]
 pub struct TestCfg {
     /// The path to the test environment's fuelup executable. This should usually be <TMP_DIR>/.fuelup/bin/fuelup.
+    /// This should be used to execute fuelup in the test environment.
     pub fuelup_path: PathBuf,
+    /// The path to the test environment's fuelup/bin directory. This should usually be <TMP_DIR>/.fuelup/bin/.
+    /// This should be used to execute other binaries (eg. forc) in the test environment.
+    pub fuelup_bin_dirpath: PathBuf,
     /// The path to the test environment's home. This should usually be a created tempfile::tempdir::TempDir.
     pub home: PathBuf,
 }
@@ -76,8 +81,12 @@ pub static ALL_BINS: &[&str] = &[
 ];
 
 impl TestCfg {
-    pub fn new(fuelup_path: PathBuf, home: PathBuf) -> Self {
-        Self { fuelup_path, home }
+    pub fn new(fuelup_path: PathBuf, fuelup_bin_dirpath: PathBuf, home: PathBuf) -> Self {
+        Self {
+            fuelup_path,
+            fuelup_bin_dirpath,
+            home,
+        }
     }
 
     pub fn toolchains_dir(&self) -> PathBuf {
@@ -102,18 +111,24 @@ impl TestCfg {
             .unwrap()
     }
 
-    pub fn fuelup(&mut self, args: &[&str]) -> TestOutput {
-        let output = Command::new(&self.fuelup_path)
+    /// A function for executing binaries within the fuelup test configuration.
+    ///
+    /// This invokes std::process::Command with some default environment variables
+    /// set up nicely for testing fuelup and its managed binaries.
+    pub fn exec(&mut self, proc_name: &str, args: &[&str]) -> TestOutput {
+        let path = self.fuelup_bin_dirpath.join(proc_name);
+        let output = Command::new(path)
             .args(args)
             .current_dir(&self.home)
             .env("HOME", &self.home)
-            .env("CARGO_HOME", self.home.join(".cargo").to_str().unwrap())
+            .env("CARGO_HOME", &self.home.join(".cargo"))
             .env(
                 "PATH",
                 format!(
-                    "{}:{}",
+                    "{}:{}:{}",
                     &self.home.join(".local/bin").display(),
-                    &self.home.join(".cargo/bin").display()
+                    &self.home.join(".cargo/bin").display(),
+                    &self.home.join(".fuelup/bin").display(),
                 ),
             )
             .env("TERM", "dumb")
@@ -127,6 +142,18 @@ impl TestCfg {
             status: output.status,
         }
     }
+
+    /// A convenience wrapper for executing 'forc' binaries within the fuelup test configuration.
+    /// This is just a testcfg::exec() call with "forc" as its first argument.
+    pub fn forc(&mut self, args: &[&str]) -> TestOutput {
+        self.exec("forc", args)
+    }
+
+    /// A convenience wrapper for executing 'fuelup' within the fuelup test configuration.
+    /// This is just a testcfg::exec() call with "fuelup" as its first argument.
+    pub fn fuelup(&mut self, args: &[&str]) -> TestOutput {
+        self.exec("fuelup", args)
+    }
 }
 
 #[cfg(unix)]
@@ -155,6 +182,7 @@ fn setup_toolchain(fuelup_home_path: &Path, toolchain: &str) -> Result<()> {
         .join(toolchain)
         .join("bin");
     fs::create_dir_all(&bin_dir).expect("Failed to create temporary latest toolchain bin dir");
+    let fuelup_bin_dirpath = fuelup_home_path.join("bin");
 
     for bin in ALL_BINS {
         let version = match toolchain.starts_with("latest") {
@@ -162,6 +190,13 @@ fn setup_toolchain(fuelup_home_path: &Path, toolchain: &str) -> Result<()> {
             _ => VERSION_2,
         };
         create_fuel_executable(bin, &bin_dir.join(bin), version)?;
+
+        if !fuelup_bin_dirpath.join(bin).exists() {
+            hard_or_symlink_file(
+                &fuelup_bin_dirpath.join("fuelup"),
+                &fuelup_bin_dirpath.join(bin),
+            )?;
+        }
     }
 
     Ok(())
@@ -203,9 +238,9 @@ pub fn setup(state: FuelupState, f: &dyn Fn(&mut TestCfg)) -> Result<()> {
         .parent()
         .expect("fuelup's directory")
         .to_path_buf();
-    fs::hard_link(
-        root.parent().unwrap().join("fuelup"),
-        tmp_fuelup_bin_dir_path.join("fuelup"),
+    hard_or_symlink_file(
+        &root.parent().unwrap().join("fuelup"),
+        &tmp_fuelup_bin_dir_path.join("fuelup"),
     )?;
 
     let target = TargetTriple::from_host().unwrap();
@@ -229,6 +264,7 @@ pub fn setup(state: FuelupState, f: &dyn Fn(&mut TestCfg)) -> Result<()> {
             setup_toolchain(&tmp_fuelup_root_path, &latest)?;
             setup_settings_file(&tmp_fuelup_root_path, &latest)?;
 
+            // Intentionally create 'binaries' that conflict with update
             fs::create_dir_all(tmp_home.join(".local/bin"))?;
             create_fuel_executable("forc", &tmp_home.join(".local/bin/forc"), VERSION)?;
 
@@ -237,12 +273,6 @@ pub fn setup(state: FuelupState, f: &dyn Fn(&mut TestCfg)) -> Result<()> {
                 &tmp_home.join(".local/bin/forc-wallet"),
                 VERSION,
             )?;
-            create_fuel_executable(
-                "forc-wallet",
-                &tmp_home.join(".fuelup/bin/forc-wallet"),
-                VERSION,
-            )?;
-
             fs::create_dir_all(tmp_home.join(".cargo/bin"))?;
 
             create_fuel_executable(
@@ -250,13 +280,12 @@ pub fn setup(state: FuelupState, f: &dyn Fn(&mut TestCfg)) -> Result<()> {
                 &tmp_home.join(".cargo/bin/forc-explore"),
                 VERSION,
             )?;
-            create_fuel_executable(
-                "forc-explore",
-                &tmp_home.join(".fuelup/bin/forc-explore"),
-                VERSION,
-            )?;
-
             create_fuel_executable("fuel-core", &tmp_home.join(".cargo/bin/fuel-core"), VERSION)?;
+
+            // Here we intentionally remove some of the 'binaries' that were linked in the
+            // setup_toolchain() step so we can expect some error messages from tests.
+            fs::remove_file(tmp_home.join(".fuelup/bin/forc"))?;
+            fs::remove_file(tmp_home.join(".fuelup/bin/fuel-core"))?;
         }
         FuelupState::NightlyInstalled => {
             setup_toolchain(&tmp_fuelup_root_path, &nightly)?;
@@ -298,6 +327,7 @@ pub fn setup(state: FuelupState, f: &dyn Fn(&mut TestCfg)) -> Result<()> {
 
     f(&mut TestCfg::new(
         tmp_fuelup_bin_dir_path.join("fuelup"),
+        tmp_fuelup_bin_dir_path,
         tmp_home.to_path_buf(),
     ));