Stop the npm cache from growing without bound (#61750)
# Objective - Fixes #59409. Closes FR-143. Zed's managed npm directory reaches 10–17GB on machines that use external agents, and is never pruned. - Two independent causes: registry agents are launched with `npm exec`, which keys its install directory on the requested version, so every agent release leaves a full ~250MB copy behind; and npm never evicts anything from its download cache. ## Solution - Install registry agents into a directory we reuse, so npm replaces the previous version in place instead of accumulating one copy per release. As a side effect, updates now download only the changed dependencies rather than the whole tree. - Empty the download cache on startup. Nothing in it needs to survive a restart, since packages are installed elsewhere. It has to go wholesale: deleting individual downloads leaves npm's index pointing at missing files, and npm then fails with `ENOENT` rather than fetching them again. - The first launch after this does one full agent install as it moves into its new home, and reclaims whatever the old directories were holding. ## Testing - Installed the real agent at 0.33.1, then upgraded to 0.42.0 in the same directory: 253MB → 256MB, against two separate copies today. The resolved executable answers an ACP `initialize`. - macOS only. Windows deserves a look — it should be better than before, since the agent is now launched as a plain `.js` file with Node instead of through npm's `.cmd` shim, but I can't verify it. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed the bundled npm cache growing without bound, which could consume many gigabytes of disk.
This commit is contained in:
1 parent
8bd6eb0b9f
commit
a153cbfbaf
2 files changed
+91
-32
No files matched your search
@@ -717,6 +717,8 @@ impl ManagedNodeRuntime {
|
||||
log::info!("Extracted Node.js to {}", node_containing_dir.display())
|
||||
}
|
||||
|
||||
_ = fs::remove_dir_all(node_dir.join("cache")).await;
|
||||
|
||||
// Note: Not in the `if !valid {}` so we can populate these for existing installations
|
||||
_ = fs::create_dir(node_dir.join("cache")).await;
|
||||
_ = fs::write(node_dir.join("blank_user_npmrc"), []).await;
|
||||
@@ -828,7 +830,7 @@ impl NodeRuntimeTrait for ManagedNodeRuntime {
|
||||
subcommand,
|
||||
args,
|
||||
);
|
||||
let command_env = npm_command_env(Some(&node_binary));
|
||||
let command_env = npm_command_env(&node_binary);
|
||||
|
||||
Ok(NpmCommand {
|
||||
path: node_binary,
|
||||
@@ -881,6 +883,7 @@ impl SystemNodeRuntime {
|
||||
|
||||
let scratch_dir = paths::data_dir().join("node");
|
||||
fs::create_dir(&scratch_dir).await.ok();
|
||||
_ = fs::remove_dir_all(scratch_dir.join("cache")).await;
|
||||
fs::create_dir(scratch_dir.join("cache")).await.ok();
|
||||
|
||||
Ok(Self {
|
||||
@@ -966,7 +969,7 @@ impl NodeRuntimeTrait for SystemNodeRuntime {
|
||||
subcommand,
|
||||
args,
|
||||
);
|
||||
let command_env = npm_command_env(Some(&self.node));
|
||||
let command_env = npm_command_env(&self.node);
|
||||
|
||||
Ok(NpmCommand {
|
||||
path: self.npm.clone(),
|
||||
@@ -1013,6 +1016,53 @@ pub async fn read_package_installed_version(
|
||||
Ok(Some(package_json.version))
|
||||
}
|
||||
|
||||
pub async fn read_package_executable(
|
||||
node_module_directory: PathBuf,
|
||||
name: &str,
|
||||
) -> Result<PathBuf> {
|
||||
let package_directory = node_module_directory.join(name);
|
||||
|
||||
#[derive(Deserialize)]
|
||||
#[serde(untagged)]
|
||||
enum Bin {
|
||||
Path(String),
|
||||
Named(HashMap<String, String>),
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct PackageJson {
|
||||
bin: Option<Bin>,
|
||||
}
|
||||
|
||||
let package_json_path = package_directory.join("package.json");
|
||||
let mut file = fs::File::open(&package_json_path)
|
||||
.await
|
||||
.with_context(|| format!("opening {}", package_json_path.display()))?;
|
||||
let mut contents = String::new();
|
||||
file.read_to_string(&mut contents).await?;
|
||||
let package_json: PackageJson = serde_json::from_str(&contents)
|
||||
.with_context(|| format!("parsing {}", package_json_path.display()))?;
|
||||
|
||||
let relative_path = match package_json.bin {
|
||||
Some(Bin::Path(path)) => path,
|
||||
Some(Bin::Named(bins)) => {
|
||||
let unscoped_name = name.rsplit('/').next().unwrap_or(name);
|
||||
let path = if bins.len() == 1 {
|
||||
bins.values().next()
|
||||
} else {
|
||||
bins.get(unscoped_name)
|
||||
};
|
||||
path.with_context(|| {
|
||||
format!("npm package {name} declares no executable named {unscoped_name}")
|
||||
})?
|
||||
.clone()
|
||||
}
|
||||
None => bail!("npm package {name} declares no executable"),
|
||||
};
|
||||
|
||||
Ok(package_directory.join(relative_path))
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct UnavailableNodeRuntime {
|
||||
error_message: Arc<String>,
|
||||
@@ -1107,12 +1157,10 @@ fn build_npm_command_args(
|
||||
command_args
|
||||
}
|
||||
|
||||
fn npm_command_env(node_binary: Option<&Path>) -> HashMap<String, String> {
|
||||
pub fn npm_command_env(node_binary: &Path) -> HashMap<String, String> {
|
||||
let mut command_env = HashMap::new();
|
||||
if let Some(node_binary) = node_binary {
|
||||
let env_path = path_with_node_binary_prepended(node_binary).unwrap_or_default();
|
||||
command_env.insert("PATH".into(), env_path.to_string_lossy().into_owned());
|
||||
}
|
||||
let env_path = path_with_node_binary_prepended(node_binary).unwrap_or_default();
|
||||
command_env.insert("PATH".into(), env_path.to_string_lossy().into_owned());
|
||||
|
||||
if let Ok(node_ca_certs) = env::var(NODE_CA_CERTS_ENV_VAR) {
|
||||
if !node_ca_certs.is_empty() {
|
||||
|
||||
@@ -1371,7 +1371,7 @@ impl ExternalAgentServer for LocalRegistryNpxAgent {
|
||||
let node_runtime = self.node_runtime.clone();
|
||||
let project_environment = self.project_environment.downgrade();
|
||||
let registry_id = self.registry_id.clone();
|
||||
let package = bounded_npm_package_spec(&self.package);
|
||||
let package = self.package.clone();
|
||||
let args = self.args.clone();
|
||||
let distribution_env = self.distribution_env.clone();
|
||||
let settings_env = self.settings_env.clone();
|
||||
@@ -1384,34 +1384,39 @@ impl ExternalAgentServer for LocalRegistryNpxAgent {
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
|
||||
let prefix_dir = paths::external_agents_dir()
|
||||
let install_dir = paths::external_agents_dir()
|
||||
.join("registry")
|
||||
.join("npx")
|
||||
.join(sanitize_path_component(®istry_id));
|
||||
fs.create_dir(&prefix_dir).await?;
|
||||
fs.create_dir(&install_dir).await?;
|
||||
|
||||
let mut exec_args = vec!["--yes".to_string(), "--".to_string(), package];
|
||||
exec_args.extend(args);
|
||||
|
||||
let npm_command = node_runtime
|
||||
.npm_command(
|
||||
Some(&prefix_dir),
|
||||
"exec",
|
||||
&exec_args.iter().map(|a| a.as_str()).collect::<Vec<_>>(),
|
||||
let (package_name, package_spec) = bounded_npm_package_spec(&package);
|
||||
node_runtime
|
||||
.run_npm_subcommand(
|
||||
Some(&install_dir),
|
||||
"install",
|
||||
&[package_spec.as_str(), "--save-exact"],
|
||||
)
|
||||
.await?;
|
||||
let executable = node_runtime::read_package_executable(
|
||||
install_dir.join("node_modules"),
|
||||
package_name,
|
||||
)
|
||||
.await?;
|
||||
|
||||
env.extend(npm_command.env);
|
||||
let node_binary = node_runtime.binary_path().await?;
|
||||
env.extend(node_runtime::npm_command_env(&node_binary));
|
||||
env.extend(distribution_env);
|
||||
env.extend(extra_env);
|
||||
env.extend(settings_env);
|
||||
|
||||
let mut args = npm_command.args;
|
||||
args.extend(extra_args);
|
||||
let mut command_args = vec![executable.to_string_lossy().into_owned()];
|
||||
command_args.extend(args);
|
||||
command_args.extend(extra_args);
|
||||
|
||||
let command = AgentServerCommand {
|
||||
path: npm_command.path,
|
||||
args,
|
||||
path: node_binary,
|
||||
args: command_args,
|
||||
env: Some(env),
|
||||
};
|
||||
|
||||
@@ -1448,15 +1453,18 @@ impl ExternalAgentServer for LocalRegistryNpxAgent {
|
||||
/// strips during parsing. PS only re-adds CRT-style transport quotes around native command args
|
||||
/// containing whitespace, so `package@<=0.25.3` reaches cmd.exe bare and the unquoted `<` is
|
||||
/// interpreted as input redirection. See zed-industries/zed#55921.
|
||||
fn bounded_npm_package_spec(package_spec: &str) -> String {
|
||||
fn bounded_npm_package_spec(package_spec: &str) -> (&str, String) {
|
||||
let Some((package_name, version)) = package_spec.rsplit_once('@') else {
|
||||
return package_spec.to_string();
|
||||
return (package_spec, package_spec.to_string());
|
||||
};
|
||||
if package_name.is_empty() || Version::parse(version).is_err() {
|
||||
return package_spec.to_string();
|
||||
if package_name.is_empty() {
|
||||
return (package_spec, package_spec.to_string());
|
||||
}
|
||||
if Version::parse(version).is_err() {
|
||||
return (package_name, package_spec.to_string());
|
||||
}
|
||||
|
||||
format!("{package_name}@0.0.0 - {version}")
|
||||
(package_name, format!("{package_name}@0.0.0 - {version}"))
|
||||
}
|
||||
|
||||
struct LocalCustomAgent {
|
||||
@@ -1830,19 +1838,22 @@ mod tests {
|
||||
fn builds_bounded_npm_package_specs() {
|
||||
assert_eq!(
|
||||
bounded_npm_package_spec("agent-package@1.2.3"),
|
||||
"agent-package@0.0.0 - 1.2.3"
|
||||
("agent-package", "agent-package@0.0.0 - 1.2.3".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
bounded_npm_package_spec("@scope/agent-package@1.2.3-beta.1"),
|
||||
"@scope/agent-package@0.0.0 - 1.2.3-beta.1"
|
||||
(
|
||||
"@scope/agent-package",
|
||||
"@scope/agent-package@0.0.0 - 1.2.3-beta.1".to_string()
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
bounded_npm_package_spec("@scope/agent-package"),
|
||||
"@scope/agent-package"
|
||||
("@scope/agent-package", "@scope/agent-package".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
bounded_npm_package_spec("agent-package@latest"),
|
||||
"agent-package@latest"
|
||||
("agent-package", "agent-package@latest".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user