Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/config/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ impl Settings {
settings.yes = true;
}
if settings.all_compile {
settings.node.compile = Some(true);
if settings.node.compile.is_none() {
settings.node.compile = Some(true);
}
if settings.python.compile.is_none() {
settings.python.compile = Some(true);
}
Expand Down
128 changes: 80 additions & 48 deletions src/plugins/core/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,61 +27,27 @@ pub struct NodePlugin {
ba: Arc<BackendArg>,
}

enum FetchOutcome {
Downloaded,
NotFound,
}

impl NodePlugin {
pub fn new() -> Self {
Self {
ba: plugins::core::new_backend_arg("node").into(),
}
}

async fn install_precompiled(
async fn fetch_binary(
&self,
ctx: &InstallContext,
tv: &mut ToolVersion,
opts: &BuildOpts,
) -> Result<()> {
let settings = Settings::get();
match self
.fetch_tarball(
ctx,
tv,
&ctx.pr,
&opts.binary_tarball_url,
&opts.binary_tarball_path,
&opts.version,
)
.await
{
Err(e)
if settings.node.compile != Some(false)
&& matches!(http::error_code(&e), Some(404)) =>
{
debug!("precompiled node not found");
return self.install_compiled(ctx, tv, opts).await;
}
e => e,
}?;
let tarball_name = &opts.binary_tarball_name;
ctx.pr.set_message(format!("extract {tarball_name}"));
file::remove_all(&opts.install_path)?;
file::untar(
&opts.binary_tarball_path,
&opts.install_path,
&TarOptions {
format: TarFormat::TarGz,
strip_components: 1,
pr: Some(&ctx.pr),
},
)?;
Ok(())
}
extract: impl FnOnce() -> Result<()>,
) -> Result<FetchOutcome> {
debug!("{:?}: we will fetch a precompiled version", self);

async fn install_windows(
&self,
ctx: &InstallContext,
tv: &mut ToolVersion,
opts: &BuildOpts,
) -> Result<()> {
match self
.fetch_tarball(
ctx,
Expand All @@ -93,13 +59,30 @@ impl NodePlugin {
)
.await
{
Ok(()) => {
debug!("{:?}: successfully downloaded node archive", self);
}
Err(e) if matches!(http::error_code(&e), Some(404)) => {
bail!("precompiled node not found {e}");
debug!("{:?}: precompiled node archive not found {e}", self);
return Ok(FetchOutcome::NotFound);
}
e => e,
}?;
Err(e) => return Err(e),
};

let tarball_name = &opts.binary_tarball_name;
ctx.pr.set_message(format!("extract {tarball_name}"));
debug!("{:?}: extracting precompiled node", self);

if let Err(e) = extract() {
debug!("{:?}: extraction failed: {e}", self);
return Err(e);
}

debug!("{:?}: precompiled node extraction was successful", self);
Ok(FetchOutcome::Downloaded)
}

fn extract_zip(&self, opts: &BuildOpts, _ctx: &InstallContext) -> Result<()> {
let tmp_extract_path = tempdir_in(opts.install_path.parent().unwrap())?;
file::unzip(
&opts.binary_tarball_path,
Expand All @@ -114,12 +97,60 @@ impl NodePlugin {
Ok(())
}

async fn install_compiled(
async fn install_precompiled(
&self,
ctx: &InstallContext,
tv: &mut ToolVersion,
opts: &BuildOpts,
) -> Result<()> {
match self
.fetch_binary(ctx, tv, opts, || {
file::untar(
&opts.binary_tarball_path,
&opts.install_path,
&TarOptions {
format: TarFormat::TarGz,
strip_components: 1,
pr: Some(&ctx.pr),
},
)?;
Ok(())
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing Cleanup in Precompiled Install Path

The install_precompiled path is missing the file::remove_all(&opts.install_path) call that previously cleared the install directory before extraction. This omission can lead to stale files or installation failures when installing over an existing version. Other installation paths, like Windows or compiling from source, still include this cleanup.

Fix in Cursor Fix in Web

.await?
{
FetchOutcome::Downloaded => Ok(()),
FetchOutcome::NotFound => {
if Settings::get().node.compile != Some(false) {
self.install_compiling(ctx, tv, opts).await
} else {
bail!("precompiled node archive not found and compilation is disabled")
}
}
}
}

async fn install_windows(
&self,
ctx: &InstallContext,
tv: &mut ToolVersion,
opts: &BuildOpts,
) -> Result<()> {
match self
.fetch_binary(ctx, tv, opts, || self.extract_zip(opts, ctx))
.await?
{
FetchOutcome::Downloaded => Ok(()),
FetchOutcome::NotFound => bail!("precompiled node archive not found (404)"),
}
}

async fn install_compiling(
&self,
ctx: &InstallContext,
tv: &mut ToolVersion,
opts: &BuildOpts,
) -> Result<()> {
debug!("{:?}: we will fetch the source and compile", self);
let tarball_name = &opts.source_tarball_name;
self.fetch_tarball(
ctx,
Expand Down Expand Up @@ -454,10 +485,11 @@ impl Backend for NodePlugin {
if cfg!(windows) {
self.install_windows(ctx, &mut tv, &opts).await?;
} else if settings.node.compile == Some(true) {
self.install_compiled(ctx, &mut tv, &opts).await?;
self.install_compiling(ctx, &mut tv, &opts).await?;
} else {
self.install_precompiled(ctx, &mut tv, &opts).await?;
}
debug!("{:?}: checking installation is working as expected", self);
self.test_node(&ctx.config, &tv, &ctx.pr).await?;
if !cfg!(windows) {
self.install_npm_shim(&tv)?;
Expand Down
Loading