diff --git a/Cargo.lock b/Cargo.lock index 2aff81a9..b9f2c419 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1453,6 +1453,7 @@ name = "pacquet-package-manager" version = "0.0.1" dependencies = [ "async-recursion", + "dashmap", "derive_more", "futures-util", "insta", diff --git a/crates/cli/src/cli_args/add.rs b/crates/cli/src/cli_args/add.rs index ee441b15..18ee1b8c 100644 --- a/crates/cli/src/cli_args/add.rs +++ b/crates/cli/src/cli_args/add.rs @@ -85,7 +85,8 @@ impl AddArgs { pub async fn run(self, mut state: State) -> miette::Result<()> { // TODO: if a package already exists in another dependency group, don't remove the existing entry. - let State { tarball_mem_cache, http_client, config, manifest, lockfile } = &mut state; + let State { tarball_mem_cache, http_client, config, manifest, lockfile, resolved_packages } = + &mut state; Add { tarball_mem_cache, @@ -96,6 +97,7 @@ impl AddArgs { list_dependency_groups: || self.dependency_options.dependency_groups(), package_name: &self.package_name, save_exact: self.save_exact, + resolved_packages, } .run() .await diff --git a/crates/cli/src/cli_args/install.rs b/crates/cli/src/cli_args/install.rs index 46da07a3..a31d2564 100644 --- a/crates/cli/src/cli_args/install.rs +++ b/crates/cli/src/cli_args/install.rs @@ -49,7 +49,8 @@ pub struct InstallArgs { impl InstallArgs { pub async fn run(self, state: State) -> miette::Result<()> { - let State { tarball_mem_cache, http_client, config, manifest, lockfile } = &state; + let State { tarball_mem_cache, http_client, config, manifest, lockfile, resolved_packages } = + &state; let InstallArgs { dependency_options, frozen_lockfile } = self; Install { @@ -60,6 +61,7 @@ impl InstallArgs { lockfile: lockfile.as_ref(), dependency_groups: dependency_options.dependency_groups(), frozen_lockfile, + resolved_packages, } .run() .await; diff --git a/crates/cli/src/state.rs b/crates/cli/src/state.rs index 84bdf8d1..99be0f01 100644 --- a/crates/cli/src/state.rs +++ b/crates/cli/src/state.rs @@ -3,6 +3,7 @@ use miette::Diagnostic; use pacquet_lockfile::{LoadLockfileError, Lockfile}; use pacquet_network::ThrottledClient; use pacquet_npmrc::Npmrc; +use pacquet_package_manager::ResolvedPackages; use pacquet_package_manifest::{PackageManifest, PackageManifestError}; use pacquet_tarball::MemCache; use pipe_trait::Pipe; @@ -20,6 +21,8 @@ pub struct State { pub manifest: PackageManifest, /// Data from the `pnpm-lock.yaml` file. pub lockfile: Option, + /// In-memory cache for packages that have started resolving dependencies. + pub resolved_packages: ResolvedPackages, } /// Error type of [`State::init`]. @@ -45,6 +48,7 @@ impl State { .map_err(InitStateError::LoadLockfile)?, http_client: ThrottledClient::new_from_cpu_count(), tarball_mem_cache: MemCache::new(), + resolved_packages: ResolvedPackages::new(), }) } } diff --git a/crates/cli/tests/install.rs b/crates/cli/tests/install.rs index 1298e6c7..455c6aab 100644 --- a/crates/cli/tests/install.rs +++ b/crates/cli/tests/install.rs @@ -12,6 +12,7 @@ use pipe_trait::Pipe; use std::{ fs::{self, OpenOptions}, io::Write, + path::Path, }; #[test] @@ -164,6 +165,29 @@ fn frozen_lockfile_should_be_able_to_handle_big_lockfile() { eprintln!("Executing command..."); pacquet.with_args(["install", "--frozen-lockfile"]).assert().success(); +} + +#[test] +fn should_install_circular_dependencies() { + let CommandTempCwd { pacquet, root, workspace, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + + eprintln!("Creating package.json..."); + let manifest_path = workspace.join("package.json"); + let package_json_content = serde_json::json!({ + "dependencies": { + "@pnpm.e2e/circular-deps-1-of-2": "1.0.2", + }, + }); + fs::write(manifest_path, package_json_content.to_string()).expect("write to package.json"); + + eprintln!("Executing command..."); + pacquet.with_arg("install").assert().success(); + + assert!(workspace.join("./node_modules/@pnpm.e2e/circular-deps-1-of-2").exists()); + assert!(workspace.join("./node_modules/.pnpm/@pnpm.e2e+circular-deps-1-of-2@1.0.2").exists()); + assert!(workspace.join("./node_modules/.pnpm/@pnpm.e2e+circular-deps-2-of-2@1.0.2").exists()); drop((root, mock_instance)); // cleanup } diff --git a/crates/package-manager/Cargo.toml b/crates/package-manager/Cargo.toml index 027f4e01..9c7b9d99 100644 --- a/crates/package-manager/Cargo.toml +++ b/crates/package-manager/Cargo.toml @@ -20,6 +20,7 @@ pacquet-registry = { workspace = true } pacquet-tarball = { workspace = true } async-recursion = { workspace = true } +dashmap = { workspace = true } derive_more = { workspace = true } futures-util = { workspace = true } node-semver = { workspace = true } diff --git a/crates/package-manager/src/add.rs b/crates/package-manager/src/add.rs index e4076e55..bc932f81 100644 --- a/crates/package-manager/src/add.rs +++ b/crates/package-manager/src/add.rs @@ -1,4 +1,4 @@ -use crate::Install; +use crate::{Install, ResolvedPackages}; use derive_more::{Display, Error}; use miette::Diagnostic; use pacquet_lockfile::Lockfile; @@ -17,6 +17,7 @@ where DependencyGroupList: IntoIterator, { pub tarball_mem_cache: &'a MemCache, + pub resolved_packages: &'a ResolvedPackages, pub http_client: &'a ThrottledClient, pub config: &'static Npmrc, pub manifest: &'a mut PackageManifest, @@ -51,6 +52,7 @@ where list_dependency_groups, package_name, save_exact, + resolved_packages, } = self; let latest_version = PackageVersion::fetch_from_registry( @@ -77,6 +79,7 @@ where lockfile, dependency_groups: list_dependency_groups(), frozen_lockfile: false, + resolved_packages, } .run() .await; diff --git a/crates/package-manager/src/install.rs b/crates/package-manager/src/install.rs index 2a59b475..f07cac4c 100644 --- a/crates/package-manager/src/install.rs +++ b/crates/package-manager/src/install.rs @@ -1,4 +1,4 @@ -use crate::{InstallFrozenLockfile, InstallWithoutLockfile}; +use crate::{InstallFrozenLockfile, InstallWithoutLockfile, ResolvedPackages}; use pacquet_lockfile::Lockfile; use pacquet_network::ThrottledClient; use pacquet_npmrc::Npmrc; @@ -12,6 +12,7 @@ where DependencyGroupList: IntoIterator, { pub tarball_mem_cache: &'a MemCache, + pub resolved_packages: &'a ResolvedPackages, pub http_client: &'a ThrottledClient, pub config: &'static Npmrc, pub manifest: &'a PackageManifest, @@ -28,6 +29,7 @@ where pub async fn run(self) { let Install { tarball_mem_cache, + resolved_packages, http_client, config, manifest, @@ -42,6 +44,7 @@ where (false, _, _) => { InstallWithoutLockfile { tarball_mem_cache, + resolved_packages, http_client, config, manifest, @@ -122,6 +125,7 @@ mod tests { DependencyGroup::Optional, ], frozen_lockfile: false, + resolved_packages: &Default::default(), } .run() .await; diff --git a/crates/package-manager/src/install_without_lockfile.rs b/crates/package-manager/src/install_without_lockfile.rs index 48f1903d..6525ed9d 100644 --- a/crates/package-manager/src/install_without_lockfile.rs +++ b/crates/package-manager/src/install_without_lockfile.rs @@ -1,5 +1,6 @@ use crate::InstallPackageFromRegistry; use async_recursion::async_recursion; +use dashmap::DashSet; use futures_util::future; use node_semver::Version; use pacquet_network::ThrottledClient; @@ -9,6 +10,12 @@ use pacquet_registry::PackageVersion; use pacquet_tarball::MemCache; use pipe_trait::Pipe; +/// In-memory cache for packages that have started resolving dependencies. +/// +/// The contents of set is the package's virtual_store_name. +/// e.g. `@pnpm.e2e/dep-1@1.0.0` → `@pnpm.e2e+dep-1@1.0.0` +pub type ResolvedPackages = DashSet; + /// This subroutine install packages from a `package.json` without reading or writing a lockfile. /// /// **Brief overview for each package:** @@ -21,6 +28,7 @@ use pipe_trait::Pipe; #[must_use] pub struct InstallWithoutLockfile<'a, DependencyGroupList> { pub tarball_mem_cache: &'a MemCache, + pub resolved_packages: &'a ResolvedPackages, pub http_client: &'a ThrottledClient, pub config: &'static Npmrc, pub manifest: &'a PackageManifest, @@ -39,6 +47,7 @@ impl<'a, DependencyGroupList> InstallWithoutLockfile<'a, DependencyGroupList> { config, manifest, dependency_groups, + resolved_packages, } = self; let _: Vec<()> = manifest @@ -62,6 +71,7 @@ impl<'a, DependencyGroupList> InstallWithoutLockfile<'a, DependencyGroupList> { config, manifest, dependency_groups: (), + resolved_packages, } .install_dependencies_from_registry(&dependency) .await; @@ -75,7 +85,19 @@ impl<'a> InstallWithoutLockfile<'a, ()> { /// Install dependencies of a dependency. #[async_recursion] async fn install_dependencies_from_registry(&self, package: &PackageVersion) { - let InstallWithoutLockfile { tarball_mem_cache, http_client, config, .. } = self; + let InstallWithoutLockfile { + tarball_mem_cache, + http_client, + config, + resolved_packages, + .. + } = self; + + // This package has already resolved, there is no need to reinstall again. + if !resolved_packages.insert(package.to_virtual_store_name()) { + tracing::info!(target: "pacquet::install", package = ?package.to_virtual_store_name(), "Skip subset"); + return; + } let node_modules_path = self .config