refactor(pm): directly run add command

Author: fengmk2

crates/vite_package_manager/src/add.rs

@@ -1,7 +1,10 @@
-use std::collections::HashMap;
+use std::{collections::HashMap, process::ExitStatus};
+
+use vite_error::Error;
+use vite_path::AbsolutePath;
 
 use crate::package_manager::{
-    PackageManager, PackageManagerType, ResolveCommandResult, format_path_env,
+    PackageManager, PackageManagerType, ResolveCommandResult, format_path_env, run_command,
 };
 
 /// The type of dependency to save.
@@ -33,6 +36,19 @@ pub struct AddCommandOptions<'a> {
 }
 
 impl PackageManager {
+    /// Run the add command with the package manager.
+    /// Return the exit status of the command.
+    #[must_use]
+    pub async fn run_add_command(
+        &self,
+        options: &AddCommandOptions<'_>,
+        cwd: impl AsRef<AbsolutePath>,
+    ) -> Result<ExitStatus, Error> {
+        let resolve_command = self.resolve_add_command(options);
+        run_command(&resolve_command.bin_path, &resolve_command.args, &resolve_command.envs, cwd)
+            .await
+    }
+
     /// Resolve the add command.
     #[must_use]
     pub fn resolve_add_command(&self, options: &AddCommandOptions) -> ResolveCommandResult {

crates/vite_package_manager/src/package_manager.rs

@@ -4,11 +4,12 @@ use std::{
     fs::{self, File},
     io::{BufReader, Seek, SeekFrom},
     path::Path,
+    process::{ExitStatus, Stdio},
 };
 
 use semver::{Version, VersionReq};
 use serde::{Deserialize, Serialize};
-use tokio::fs::remove_dir_all;
+use tokio::{fs::remove_dir_all, process::Command};
 use vite_error::Error;
 use vite_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf};
 use vite_str::Str;
@@ -138,8 +139,8 @@ impl PackageManagerBuilder {
 }
 
 impl PackageManager {
-    pub fn builder(workspace_root: impl AsRef<AbsolutePath>) -> PackageManagerBuilder {
-        PackageManagerBuilder::new(workspace_root)
+    pub fn builder(cwd: impl AsRef<AbsolutePath>) -> PackageManagerBuilder {
+        PackageManagerBuilder::new(cwd)
     }
 
     #[must_use]
@@ -600,6 +601,27 @@ pub(crate) fn format_path_env(bin_prefix: impl AsRef<Path>) -> String {
     env::join_paths(paths).unwrap().to_string_lossy().to_string()
 }
 
+pub(crate) async fn run_command(
+    bin_name: &str,
+    args: &Vec<String>,
+    envs: &HashMap<String, String>,
+    cwd: impl AsRef<AbsolutePath>,
+) -> Result<ExitStatus, Error> {
+    println!("Running: {} {}", bin_name, args.join(" "));
+
+    // TODO: color support for stdout/stderr
+    let status = Command::new(bin_name)
+        .args(args)
+        .envs(envs)
+        .current_dir(cwd.as_ref())
+        .stdin(Stdio::inherit())
+        .stdout(Stdio::inherit())
+        .stderr(Stdio::inherit())
+        .status()
+        .await?;
+    Ok(status)
+}
+
 #[cfg(test)]
 mod tests {
     use std::fs;

crates/vite_task/src/add.rs

@@ -1,27 +1,24 @@
-use petgraph::stable_graph::StableGraph;
+use std::process::ExitStatus;
+
 use vite_package_manager::{
     add::{AddCommandOptions, SaveDependencyType},
     package_manager::PackageManager,
 };
 use vite_path::AbsolutePathBuf;
 
-use crate::{
-    Error, ResolveCommandResult, Workspace,
-    config::ResolvedTask,
-    schedule::{ExecutionPlan, ExecutionSummary},
-};
+use crate::Error;
 
 /// Add command for adding packages to dependencies.
 ///
 /// This command automatically detects the package manager and translates
 /// the add command to the appropriate package manager-specific syntax.
 pub struct AddCommand {
-    workspace_root: AbsolutePathBuf,
+    cwd: AbsolutePathBuf,
 }
 
 impl AddCommand {
-    pub fn new(workspace_root: AbsolutePathBuf) -> Self {
-        Self { workspace_root }
+    pub fn new(cwd: AbsolutePathBuf) -> Self {
+        Self { cwd }
     }
 
     pub async fn execute(
@@ -36,15 +33,11 @@ impl AddCommand {
         global: bool,
         allow_build: Option<&str>,
         pass_through_args: Option<&[String]>,
-    ) -> Result<ExecutionSummary, Error> {
+    ) -> Result<ExitStatus, Error> {
         if packages.is_empty() {
             return Err(Error::NoPackagesSpecified);
         }
 
-        // Detect package manager
-        let package_manager = PackageManager::builder(&self.workspace_root).build().await?;
-        let workspace = Workspace::partial_load(self.workspace_root)?;
-
         let add_command_options = AddCommandOptions {
             packages,
             save_dependency_type,
@@ -57,26 +50,11 @@ impl AddCommand {
             allow_build,
             pass_through_args,
         };
-        let resolve_command = package_manager.resolve_add_command(&add_command_options);
 
-        println!("Running: {} {}", resolve_command.bin_path, resolve_command.args.join(" "));
-
-        // TODO: set cacheable to false
-        let resolved_task = ResolvedTask::resolve_from_builtin_with_command_result(
-            &workspace,
-            "add",
-            resolve_command.args.iter(),
-            ResolveCommandResult { bin_path: resolve_command.bin_path, envs: resolve_command.envs },
-            false,
-            None,
-        )?;
-
-        let mut task_graph: StableGraph<ResolvedTask, ()> = Default::default();
-        task_graph.add_node(resolved_task);
-        let summary = ExecutionPlan::plan(task_graph, false)?.execute(&workspace).await?;
-        workspace.unload().await?;
+        // Detect package manager
+        let package_manager = PackageManager::builder(&self.cwd).build().await?;
 
-        Ok(summary)
+        package_manager.run_add_command(&add_command_options, &self.cwd).await
     }
 }
 

crates/vite_task/src/lib.rs

@@ -241,7 +241,7 @@ async fn execute_add_command(
     global: bool,
     allow_build: Option<&str>,
     pass_through_args: Option<&[String]>,
-) -> Result<ExecutionSummary, Error> {
+) -> Result<ExitStatus, Error> {
     let save_dependency_type = if save_dev {
         Some(SaveDependencyType::Dev)
     } else if save_peer {
@@ -342,6 +342,37 @@ pub struct ResolvedUniversalViteConfig {
     pub fmt: Option<FmtConfig>,
 }
 
+#[cfg(unix)]
+fn fix_stdio_streams() {
+    // libuv may mark stdin/stdout/stderr as close-on-exec.
+    // As a workaround, we clear the FD_CLOEXEC flag on these file descriptors to prevent them from being closed when spawning child processes.
+    //
+    // For details see https://github.com/libuv/libuv/issues/2062
+    // Fixed by reference from https://github.com/electron/electron/pull/15555
+
+    use std::os::unix::io::RawFd;
+
+    use nix::libc;
+
+    unsafe {
+        // Helper function to clear FD_CLOEXEC flag on a file descriptor
+        let clear_cloexec = |fd: RawFd| {
+            // Get current file descriptor flags
+            let flags = libc::fcntl(fd, libc::F_GETFD);
+            if flags >= 0 {
+                // Clear the FD_CLOEXEC flag
+                let new_flags = flags & !libc::FD_CLOEXEC;
+                libc::fcntl(fd, libc::F_SETFD, new_flags);
+            }
+        };
+
+        // Clear FD_CLOEXEC on stdin, stdout, stderr
+        clear_cloexec(libc::STDIN_FILENO);
+        clear_cloexec(libc::STDOUT_FILENO);
+        clear_cloexec(libc::STDERR_FILENO);
+    }
+}
+
 /// Main entry point for vite-plus task execution.
 ///
 /// # Execution Flow
@@ -409,6 +440,9 @@ pub async fn main<
         >,
     >,
 ) -> Result<std::process::ExitStatus, Error> {
+    #[cfg(unix)]
+    fix_stdio_streams();
+
     // Auto-install dependencies if needed, but skip for install command itself, or if `VITE_DISABLE_AUTO_INSTALL=1` is set.
     if !matches!(args.commands, Commands::Install { .. })
         && std::env::var_os("VITE_DISABLE_AUTO_INSTALL") != Some("1".into())
@@ -534,7 +568,7 @@ pub async fn main<
             allow_build,
             pass_through_args,
         } => {
-            execute_add_command(
+            let exit_status = execute_add_command(
                 cwd,
                 packages,
                 *save_prod,
@@ -551,7 +585,8 @@ pub async fn main<
                 allow_build.as_deref(),
                 pass_through_args.as_deref(),
             )
-            .await?
+            .await?;
+            return Ok(exit_status);
         }
         Commands::Install { args } => {
             // Check if args contain packages - if yes, redirect to Add command
@@ -573,7 +608,7 @@ pub async fn main<
                 pass_through_args,
             }) = parse_install_as_add(args)
             {
-                execute_add_command(
+                let exit_status = execute_add_command(
                     cwd,
                     &packages,
                     save_prod,
@@ -590,7 +625,8 @@ pub async fn main<
                     allow_build.as_deref(),
                     pass_through_args.as_deref(),
                 )
-                .await?
+                .await?;
+                return Ok(exit_status);
             } else {
                 install::InstallCommand::builder(cwd).build().execute(args).await?
             }

packages/cli/binding/src/lib.rs

@@ -197,6 +197,8 @@ pub async fn run(options: CliOptions) -> Result<i32> {
     )
     .await;
 
+    tracing::debug!("Result: {result:?}");
+
     match result {
         Ok(exit_status) => Ok(exit_status.code().unwrap_or(1)),
         Err(e) => {

packages/cli/snap-tests/vitest-browser-mode/snap.txt

@@ -1,6 +1,6 @@
 > vite test
 
- RUN  v<semver> <cwd>
+ RUN  v<semver> /private/var/folders/d7/5vmw5m15727gcsyvrqxyrr9h0000gn/T/vite-plus-test-623cfd5e-c93e-4b2f-abce-5621fc6d6231/vitest-browser-mode
 
  ✓ |chromium| src/foo.test.js (1 test) <variable>ms
 
@@ -15,7 +15,7 @@
 > vite test
 ✗ cache miss: content of input 'src/foo.js' changed, executing
 
- RUN  v<semver> <cwd>
+ RUN  v<semver> /private/var/folders/d7/5vmw5m15727gcsyvrqxyrr9h0000gn/T/vite-plus-test-623cfd5e-c93e-4b2f-abce-5621fc6d6231/vitest-browser-mode
 
  ✓ |chromium| src/foo.test.js (1 test) <variable>ms
 
@@ -30,7 +30,7 @@
 > vite test
 ✓ cache hit, replaying
 
- RUN  v<semver> <cwd>
+ RUN  v<semver> /private/var/folders/d7/5vmw5m15727gcsyvrqxyrr9h0000gn/T/vite-plus-test-623cfd5e-c93e-4b2f-abce-5621fc6d6231/vitest-browser-mode
 
  ✓ |chromium| src/foo.test.js (1 test) <variable>ms
 

packages/cli/snap-tests/yarn-install-with-options/snap.txt

@@ -40,7 +40,7 @@
     --mutex <type>[:specifier]          use a mutex to ensure only one yarn instance is executing
     --emoji [bool]                      enable emoji in output (default: true)
     -s, --silent                        skip Yarn console logs, other types of logs (script output) will be printed
-    --cwd <cwd>                         working directory to use (default: <cwd>)
+    --cwd <cwd>                         working directory to use (default: /private/var/folders/d7/5vmw5m15727gcsyvrqxyrr9h0000gn/T/vite-plus-test-623cfd5e-c93e-4b2f-abce-5621fc6d6231/yarn-install-with-options)
     --proxy <host>                      
     --https-proxy <host>                
     --registry <url>                    override configuration registry

packages/global/snap-tests/command-add-npm10-with-workspace/snap.txt

@@ -1,8 +1,8 @@
 > vp add testnpm2 -D -w -- --no-audit && cat package.json packages/app/package.json packages/utils/package.json # should add package to workspace root
 Running: npm install --include-workspace-root --save-dev --no-audit testnpm2
+CWD: <cwd>
 
 added 3 packages in <variable>ms
-
 {
   "name": "command-add-npm10-with-workspace",
   "version": "1.0.0",
@@ -25,9 +25,9 @@ added 3 packages in <variable>ms
 
 > vp add @vite-plus-test/utils --workspace -- --no-audit && cat package.json packages/app/package.json packages/utils/package.json # should add @vite-plus-test/utils to workspace root
 Running: npm install --no-audit @vite-plus-test/utils
+CWD: <cwd>
 
 up to date in <variable>ms
-
 {
   "name": "command-add-npm10-with-workspace",
   "version": "1.0.0",
@@ -53,9 +53,9 @@ up to date in <variable>ms
 
 > vp add testnpm2 test-vite-plus-install@1.0.0 --filter app -- --no-audit && cat package.json packages/app/package.json packages/utils/package.json # should add packages to packages/app
 Running: npm install --workspace app --no-audit testnpm2 test-vite-plus-install@<semver>
+CWD: <cwd>
 
 added 1 package in <variable>ms
-
 {
   "name": "command-add-npm10-with-workspace",
   "version": "1.0.0",
@@ -85,9 +85,9 @@ added 1 package in <variable>ms
 
 > vp add @vite-plus-test/utils --workspace --filter app -- --no-audit && cat package.json packages/app/package.json packages/utils/package.json # should add @vite-plus-test/utils to packages/app
 Running: npm install --workspace app --no-audit @vite-plus-test/utils
+CWD: <cwd>
 
 up to date in <variable>ms
-
 {
   "name": "command-add-npm10-with-workspace",
   "version": "1.0.0",
@@ -118,9 +118,9 @@ up to date in <variable>ms
 
 > vp add testnpm2 test-vite-plus-install@1.0.0 --filter "*" -- --no-audit && cat package.json packages/app/package.json packages/utils/package.json # should add testnpm2 test-vite-plus-install to all packages except workspace root
 Running: npm install --workspace * --no-audit testnpm2 test-vite-plus-install@<semver>
+CWD: <cwd>
 
 up to date in <variable>ms
-
 {
   "name": "command-add-npm10-with-workspace",
   "version": "1.0.0",
@@ -155,9 +155,9 @@ up to date in <variable>ms
 
 > vp add -E testnpm2 test-vite-plus-install@1.0.0 --filter "*" --workspace-root -- --no-audit && cat package.json packages/app/package.json packages/utils/package.json # should add testnpm2 test-vite-plus-install to all packages include workspace root
 Running: npm install --workspace * --include-workspace-root --save-exact --no-audit testnpm2 test-vite-plus-install@<semver>
+CWD: <cwd>
 
 up to date in <variable>ms
-
 {
   "name": "command-add-npm10-with-workspace",
   "version": "1.0.0",
@@ -193,9 +193,9 @@ up to date in <variable>ms
 
 > vp install test-vite-plus-package@1.0.0 --filter "*" --workspace-root -- --no-audit && cat package.json packages/app/package.json packages/utils/package.json # should install packages alias for add command
 Running: npm install --workspace * --include-workspace-root --no-audit test-vite-plus-package@<semver>
+CWD: <cwd>
 
 added 1 package in <variable>ms
-
 {
   "name": "command-add-npm10-with-workspace",
   "version": "1.0.0",