refactor(pm): refact rebuild.rs (#2232)

Author: elrrrrrrr

crates/pm/src/cmd/rebuild.rs

@@ -1,34 +1,9 @@
-use crate::service::package::PackageService;
-use anyhow::{Context, Result};
+use crate::helper::lock::ensure_package_lock;
+use crate::service::rebuild::RebuildService;
+use anyhow::Result;
 use std::path::Path;
 
 pub async fn rebuild(root_path: &Path) -> Result<()> {
-    let packages = PackageService::collect_packages(root_path)
-        .await
-        .map_err(|e| anyhow::anyhow!("Failed to collect packages: {}", e))?;
-
-    let execution_queues = PackageService::create_execution_queues(packages)
-        .context("Failed to create execution queues")?;
-    PackageService::execute_queues(execution_queues)
-        .await
-        .map_err(|e| anyhow::anyhow!("Failed to execute queues: {}", e))?;
-
-    // Handle project's own rebuild logic
-    // Since package-lock.json doesn't have root node information for "", need to manually add
-    // We align with npm cli's logic: after dependency reify completes, manually execute project's own scripts
-    // const scripts = [
-    //     'preinstall',
-    //     'install',
-    //     'postinstall',
-    //     'prepublish', // XXX(npm9) should we remove this finally??
-    //     'preprepare',
-    //     'prepare',
-    //     'postprepare',
-    //   ]
-
-    PackageService::process_project_hooks(root_path)
-        .await
-        .map_err(|e| anyhow::anyhow!("Failed to process project hooks: {}", e))?;
-
-    Ok(())
+    let package_lock = ensure_package_lock(root_path).await?;
+    RebuildService::rebuild(&package_lock, root_path, false).await
 }

crates/pm/src/helper/lock.rs

@@ -8,6 +8,7 @@ use std::sync::Arc;
 use crate::helper::workspace::find_workspaces;
 use crate::model::node::{EdgeType, Node};
 use crate::model::override_rule::Overrides;
+use crate::model::package_lock::LockPackage;
 use crate::util::config::get_legacy_peer_deps;
 use crate::util::json::{load_package_json_from_path, load_package_lock_json_from_path};
 use crate::util::logger::{log_verbose, log_warning};
@@ -20,22 +21,14 @@ use crate::{cmd::deps::build_deps, util::logger::log_info};
 
 use super::workspace::find_workspace_path;
 
+// Use the model's LockPackage but create a simplified PackageLock for helper functions
 #[derive(Deserialize, Serialize, Clone)]
 pub struct PackageLock {
-    pub packages: HashMap<String, Package>,
+    pub packages: HashMap<String, LockPackage>,
 }
 
-#[derive(Deserialize, Serialize, Debug, Clone)]
-#[serde(rename_all = "snake_case")]
-pub struct Package {
-    pub name: Option<String>,
-    pub version: Option<String>,
-    pub resolved: Option<String>,
-    pub link: Option<bool>,
-    pub cpu: Option<Value>,
-    pub os: Option<Value>,
-    pub has_install_script: Option<bool>,
-}
+// Type alias for backward compatibility
+pub type Package = LockPackage;
 
 pub fn group_by_depth(
     packages: &HashMap<String, Package>,

crates/pm/src/model/package_lock.rs

@@ -5,12 +5,13 @@ use std::fs;
 use std::path::Path;
 
 use crate::{
+    helper::lock::path_to_pkg_name,
     service::dependency_graph::{DependencyGraphService, DependencyType, PackageNode},
     util::logger::log_verbose,
 };
 
 /// Represents package information in package-lock.json
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub struct LockPackage {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub name: Option<String>,
@@ -51,6 +52,13 @@ pub struct LockPackage {
     pub has_install_script: Option<bool>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub workspaces: Option<Vec<String>>,
+    // Additional fields from helper/lock.rs Package
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub link: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub cpu: Option<serde_json::Value>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub os: Option<serde_json::Value>,
 }
 
 impl LockPackage {
@@ -61,8 +69,10 @@ impl LockPackage {
         } else if path.is_empty() {
             "root".to_string()
         } else {
-            // Extract package name from path
-            path.split('/').next_back().unwrap_or("unknown").to_string()
+            // Use existing path_to_pkg_name helper function
+            path_to_pkg_name(path)
+                .map(|s| s.to_string())
+                .unwrap_or_else(|| "unknown".to_string())
         }
     }
 
@@ -73,6 +83,23 @@ impl LockPackage {
             .unwrap_or_else(|| "unknown".to_string())
     }
 
+    /// Parse and cache bin files from the bin field
+    pub fn parse_bin_files(&self, package_name: &str) -> Vec<(String, String)> {
+        match &self.bin {
+            Some(serde_json::Value::Object(obj)) => obj
+                .iter()
+                .map(|(k, v)| (k.clone(), v.as_str().unwrap_or_default().to_string()))
+                .collect(),
+            Some(serde_json::Value::String(s)) => vec![(package_name.to_string(), s.clone())],
+            _ => Vec::new(),
+        }
+    }
+
+    /// Check if package has install scripts
+    pub fn has_install_scripts(&self) -> bool {
+        self.has_install_script.unwrap_or(false)
+    }
+
     /// Convert to PackageNode
     pub fn to_package_node(&self, path: &str) -> PackageNode {
         let name = self.get_name(path);
@@ -257,4 +284,103 @@ mod tests {
         assert_eq!(package_lock.lockfile_version, 3);
         assert_eq!(package_lock.packages.len(), 3);
     }
+
+    #[test]
+    fn test_lock_package_parse_bin_files() {
+        // Test with object-style bin
+        let mut package = LockPackage {
+            name: Some("test-package".to_string()),
+            version: Some("1.0.0".to_string()),
+            bin: Some(serde_json::json!({"cli": "bin/cli.js", "tool": "bin/tool.js"})),
+            ..LockPackage::default()
+        };
+
+        let bin_files = package.parse_bin_files("test-package");
+        assert_eq!(bin_files.len(), 2);
+        assert!(bin_files.contains(&("cli".to_string(), "bin/cli.js".to_string())));
+        assert!(bin_files.contains(&("tool".to_string(), "bin/tool.js".to_string())));
+
+        // Test with string-style bin
+        package.bin = Some(serde_json::json!("index.js"));
+        let bin_files = package.parse_bin_files("test-package");
+        assert_eq!(bin_files.len(), 1);
+        assert_eq!(
+            bin_files[0],
+            ("test-package".to_string(), "index.js".to_string())
+        );
+
+        // Test with no bin
+        package.bin = None;
+        let bin_files = package.parse_bin_files("test-package");
+        assert_eq!(bin_files.len(), 0);
+
+        // Test with invalid bin value
+        package.bin = Some(serde_json::json!(123));
+        let bin_files = package.parse_bin_files("test-package");
+        assert_eq!(bin_files.len(), 0);
+    }
+
+    #[test]
+    fn test_lock_package_has_install_scripts() {
+        let mut package = LockPackage {
+            name: Some("test-package".to_string()),
+            version: Some("1.0.0".to_string()),
+            has_install_script: Some(true),
+            ..LockPackage::default()
+        };
+
+        assert!(package.has_install_scripts());
+
+        package.has_install_script = Some(false);
+        assert!(!package.has_install_scripts());
+
+        package.has_install_script = None;
+        assert!(!package.has_install_scripts());
+    }
+
+    #[test]
+    fn test_lock_package_get_name() {
+        let mut package = LockPackage {
+            name: Some("test-package".to_string()),
+            version: Some("1.0.0".to_string()),
+            ..LockPackage::default()
+        };
+
+        // Test with explicit name
+        assert_eq!(
+            package.get_name("node_modules/some-package"),
+            "test-package"
+        );
+
+        // Test without name (infer from path)
+        package.name = None;
+        assert_eq!(package.get_name("node_modules/lodash"), "lodash");
+        assert_eq!(
+            package.get_name("node_modules/@scope/package"),
+            "@scope/package"
+        );
+
+        // Test with empty path
+        assert_eq!(package.get_name(""), "root");
+
+        // Test with complex path
+        assert_eq!(
+            package.get_name("node_modules/parent/node_modules/child"),
+            "child"
+        );
+    }
+
+    #[test]
+    fn test_lock_package_get_version() {
+        let mut package = LockPackage {
+            name: Some("test-package".to_string()),
+            version: Some("1.2.3".to_string()),
+            ..LockPackage::default()
+        };
+
+        assert_eq!(package.get_version(), "1.2.3");
+
+        package.version = None;
+        assert_eq!(package.get_version(), "unknown");
+    }
 }

crates/pm/src/service/install.rs

@@ -8,7 +8,6 @@ use std::pin::Pin;
 use std::sync::Arc;
 use tokio::sync::Semaphore;
 
-use crate::cmd::rebuild::rebuild;
 use crate::helper::global_bin::get_global_bin_dir;
 use crate::helper::lock::{
     Package, ensure_package_lock, extract_package_name, group_by_depth, path_to_pkg_name,
@@ -17,6 +16,7 @@ use crate::helper::lock::{
 use crate::helper::workspace;
 use crate::helper::{is_cpu_compatible, is_os_compatible};
 use crate::model::package::PackageInfo;
+use crate::service::rebuild::RebuildService;
 use crate::util::cache::get_cache_dir;
 use crate::util::cloner::clone;
 use crate::util::downloader::download;
@@ -400,19 +400,21 @@ impl InstallService {
 
         finish_progress_bar("node_modules cloned");
 
+        // Execute rebuild operations based on ignore_scripts parameter
         if !ignore_scripts {
             log_info(
                 "Starting to execute dependency hook scripts, you can add --ignore-scripts to skip",
             );
-            rebuild(root_path).await?;
+            RebuildService::rebuild(&package_lock, root_path, false).await?;
             log_info("💫 All dependencies installed successfully");
-            Ok(())
         } else {
+            log_info("Processing binary files...");
+            RebuildService::rebuild(&package_lock, root_path, true).await?;
             log_info(
-                "💫 All dependencies installed successfully (you can run 'utoo rebuild' to trigger dependency hooks)",
+                "💫 All dependencies installed successfully (scripts skipped, binaries linked)",
             );
-            Ok(())
         }
+        Ok(())
     }
 
     pub async fn install_global_package(npm_spec: &str, prefix: Option<&str>) -> Result<()> {

crates/pm/src/service/mod.rs

@@ -8,6 +8,7 @@ pub mod http_client;
 pub mod install;
 pub mod package;
 pub mod package_management;
+pub mod rebuild;
 pub mod registry;
 pub mod script;
 pub mod update;