Merge pull request #1394 from charlespierce/pnpm_feature_flag

Gate pnpm support behind a feature flag to avoid compatibility issues

crates/volta-core/src/lib.rs

@@ -20,3 +20,5 @@ pub mod sync;
 pub mod tool;
 pub mod toolchain;
 pub mod version;
+
+const VOLTA_FEATURE_PNPM: &str = "VOLTA_FEATURE_PNPM";

crates/volta-core/src/platform/mod.rs

@@ -1,8 +1,10 @@
+use std::env;
 use std::fmt;
 
 use crate::error::{ErrorKind, Fallible};
 use crate::session::Session;
 use crate::tool::{Node, Npm, Pnpm, Yarn};
+use crate::VOLTA_FEATURE_PNPM;
 use semver::Version;
 
 mod image;
@@ -284,8 +286,13 @@ impl Platform {
             Npm::new(version.clone()).ensure_fetched(session)?;
         }
 
-        if let Some(Sourced { value: version, .. }) = &self.pnpm {
-            Pnpm::new(version.clone()).ensure_fetched(session)?;
+        // Only force download of the pnpm version if the pnpm feature flag is set. If it isn't,
+        // then we won't be using the `Pnpm` tool to execute (we will be relying on the global
+        // package logic), so fetching the Pnpm version would only be redundant work.
+        if env::var_os(VOLTA_FEATURE_PNPM).is_some() {
+            if let Some(Sourced { value: version, .. }) = &self.pnpm {
+                Pnpm::new(version.clone()).ensure_fetched(session)?;
+            }
         }
 
         if let Some(Sourced { value: version, .. }) = &self.yarn {

crates/volta-core/src/run/mod.rs

@@ -7,6 +7,7 @@ use std::process::ExitStatus;
 use crate::error::{ErrorKind, Fallible};
 use crate::platform::{CliPlatform, Image, Sourced};
 use crate::session::Session;
+use crate::VOLTA_FEATURE_PNPM;
 use log::debug;
 use semver::Version;
 
@@ -85,7 +86,16 @@ fn get_executor(
             Some("node") => node::command(args, session),
             Some("npm") => npm::command(args, session),
             Some("npx") => npx::command(args, session),
-            Some("pnpm") => pnpm::command(args, session),
+            Some("pnpm") => {
+                // If the pnpm feature flag variable is set, delegate to the pnpm handler
+                // If not, use the binary handler as a fallback (prior to pnpm support, installing
+                // pnpm would be handled the same as any other global binary)
+                if env::var_os(VOLTA_FEATURE_PNPM).is_some() {
+                    pnpm::command(args, session)
+                } else {
+                    binary::command(exe, args, session)
+                }
+            }
             Some("yarn") => yarn::command(args, session),
             _ => binary::command(exe, args, session),
         }

crates/volta-core/src/tool/mod.rs

@@ -1,10 +1,12 @@
+use std::env;
 use std::fmt::{self, Display};
 
 use crate::error::{ErrorKind, Fallible};
 use crate::session::Session;
 use crate::style::{note_prefix, success_prefix, tool_version};
 use crate::sync::VoltaLock;
 use crate::version::VersionSpec;
+use crate::VOLTA_FEATURE_PNPM;
 use log::{debug, info};
 
 pub mod node;
@@ -87,8 +89,17 @@ impl Spec {
                 None => Ok(Box::new(BundledNpm)),
             },
             Spec::Pnpm(version) => {
-                let version = pnpm::resolve(version, session)?;
-                Ok(Box::new(Pnpm::new(version)))
+                // If the pnpm feature flag is set, use the special-cased package manager logic
+                // to handle resolving (and ultimately fetching / installing) pnpm. If not, then
+                // fall back to the global package behavior, which was the case prior to pnpm
+                // support being added
+                if env::var_os(VOLTA_FEATURE_PNPM).is_some() {
+                    let version = pnpm::resolve(version, session)?;
+                    Ok(Box::new(Pnpm::new(version)))
+                } else {
+                    let package = Package::new("pnpm".to_owned(), version)?;
+                    Ok(Box::new(package))
+                }
             }
             Spec::Yarn(version) => {
                 let version = yarn::resolve(version, session)?;
@@ -116,10 +127,16 @@ impl Spec {
                 feature: "Uninstalling npm".into(),
             }
             .into()),
-            Spec::Pnpm(_) => Err(ErrorKind::Unimplemented {
-                feature: "Uninstalling pnpm".into(),
+            Spec::Pnpm(_) => {
+                if env::var_os(VOLTA_FEATURE_PNPM).is_some() {
+                    Err(ErrorKind::Unimplemented {
+                        feature: "Uninstalling pnpm".into(),
+                    }
+                    .into())
+                } else {
+                    package::uninstall("pnpm")
+                }
             }
-            .into()),
             Spec::Yarn(_) => Err(ErrorKind::Unimplemented {
                 feature: "Uninstalling yarn".into(),
             }

tests/acceptance/corrupted_download.rs

@@ -108,6 +108,7 @@ fn install_corrupted_pnpm_leaves_inventory_unchanged() {
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -126,6 +127,7 @@ fn install_valid_pnpm_saves_to_inventory() {
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(

tests/acceptance/hooks.rs

@@ -342,6 +342,7 @@ fn pnpm_latest_with_hook_reads_index() {
     let s = sandbox()
         .default_hooks(&pnpm_hooks_json())
         .env("VOLTA_LOGLEVEL", "debug")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
     let _mock = mock("GET", "/pnpm/index")
         .with_status(200)
@@ -376,6 +377,7 @@ fn pnpm_no_version_with_hook_reads_index() {
     let s = sandbox()
         .default_hooks(&pnpm_hooks_json())
         .env("VOLTA_LOGLEVEL", "debug")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
     let _mock = mock("GET", "/pnpm/index")
         .with_status(200)

tests/acceptance/merged_platform.rs

@@ -381,6 +381,7 @@ fn uses_project_pnpm_if_available() {
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
         .env("VOLTA_LOGLEVEL", "debug")
         .env("VOLTA_WRITE_EVENTS_FILE", "true")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .default_hooks(&events_hooks_json())
         .executable_file(SCRIPT_FILENAME, EVENTS_EXECUTABLE)
         .build();
@@ -417,6 +418,7 @@ fn uses_default_pnpm_in_project_without_pnpm() {
         .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
         .env("VOLTA_LOGLEVEL", "debug")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -436,6 +438,7 @@ fn uses_default_pnpm_outside_project() {
         .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
         .env("VOLTA_LOGLEVEL", "debug")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -453,6 +456,7 @@ fn uses_pnpm_throws_project_error_in_project() {
     let s = sandbox()
         .platform(PLATFORM_NODE_ONLY)
         .package_json(PACKAGE_JSON_NODE_ONLY)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(

tests/acceptance/volta_install.rs

@@ -335,6 +335,7 @@ fn install_pnpm_without_node_errors() {
     let s = sandbox()
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(

tests/acceptance/volta_pin.rs

@@ -954,6 +954,7 @@ fn pin_pnpm_no_node() {
         .package_json(BASIC_PACKAGE_JSON)
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -974,6 +975,7 @@ fn pin_pnpm() {
         .package_json(&package_json_with_pinned_node("1.2.3"))
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -994,6 +996,7 @@ fn pin_pnpm_reports_info() {
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
         .env(VOLTA_LOGLEVEL, "info")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -1010,6 +1013,7 @@ fn pin_pnpm_latest() {
         .package_json(&package_json_with_pinned_node("1.2.3"))
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -1029,6 +1033,7 @@ fn pin_pnpm_no_version() {
         .package_json(&package_json_with_pinned_node("1.2.3"))
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -1046,6 +1051,7 @@ fn pin_pnpm_no_version() {
 fn pin_pnpm_missing_release() {
     let s = sandbox()
         .package_json(&package_json_with_pinned_node("1.2.3"))
+        .env("VOLTA_FEATURE_PNPM", "1")
         .mock_not_found()
         .build();
 
@@ -1070,6 +1076,7 @@ fn pin_node_and_pnpm() {
         .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -1089,6 +1096,7 @@ fn pin_pnpm_leaves_npm() {
         .package_json(&package_json_with_pinned_node_npm("1.2.3", "3.4.5"))
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(

tests/acceptance/volta_run.rs

@@ -459,6 +459,7 @@ fn command_line_pnpm() {
         .pnpm_available_versions(PNPM_VERSION_INFO)
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
         .env(VOLTA_LOGLEVEL, "debug")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -478,6 +479,7 @@ fn inherited_pnpm() {
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
         .package_json(&package_json_with_pinned_node_pnpm("10.99.1040", "7.7.1"))
         .env(VOLTA_LOGLEVEL, "debug")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(
@@ -497,6 +499,7 @@ fn force_no_pnpm() {
         .distro_mocks::<PnpmFixture>(&PNPM_VERSION_FIXTURES)
         .package_json(&package_json_with_pinned_node_pnpm("10.99.1040", "7.7.1"))
         .env(VOLTA_LOGLEVEL, "debug")
+        .env("VOLTA_FEATURE_PNPM", "1")
         .build();
 
     assert_that!(