Skip to content

Commit 09fcc77

Browse files
zkatzkat
committed
feat(optional): ignore install script failures for truly optional dependencies
Fixes: #253
1 parent 52213c4 commit 09fcc77

3 files changed

Lines changed: 59 additions & 10 deletions

File tree

‎crates/node-maintainer/src/graph.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ use indexmap::IndexMap;
99
use kdl::KdlDocument;
1010
use nassun::{package::Package, PackageResolution, PackageSpec};
1111
use oro_common::CorgiManifest;
12-
use petgraph::stable_graph::{EdgeIndex, NodeIndex, StableGraph};
12+
use petgraph::{
13+
stable_graph::{EdgeIndex, NodeIndex, StableGraph},
14+
Direction,
15+
};
1316
use unicase::UniCase;
1417

1518
use crate::{error::NodeMaintainerError, Lockfile, LockfileNode};
@@ -160,6 +163,15 @@ impl IndexMut<EdgeIndex> for Graph {
160163
}
161164

162165
impl Graph {
166+
pub fn is_optional(&self, node: NodeIndex) -> bool {
167+
for edge_ref in self.inner.edges_directed(node, Direction::Incoming) {
168+
if edge_ref.weight().dep_type != DepType::Opt {
169+
return false;
170+
}
171+
}
172+
true
173+
}
174+
163175
pub fn resolve_dep(&self, node: NodeIndex, dep: &UniCase<String>) -> Option<NodeIndex> {
164176
for parent in self.node_parent_iter(node) {
165177
if let Some(resolved) = parent.children.get(dep) {

‎crates/node-maintainer/src/linkers/hoisted.rs‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,8 @@ impl HoistedLinker {
387387
root.join("node_modules").join(subdir)
388388
};
389389

390+
let is_optional = graph.is_optional(idx);
391+
390392
let build_mani = BuildManifest::from_path(package_dir.join("package.json"))
391393
.map_err(|e| {
392394
NodeMaintainerError::BuildManifestReadError(
@@ -406,12 +408,21 @@ impl HoistedLinker {
406408
on_script_start(&graph[idx].package, &event);
407409
}
408410
std::mem::drop(_span_enter);
409-
let mut script = async_std::task::spawn_blocking(move || {
411+
let mut script = match async_std::task::spawn_blocking(move || {
410412
OroScript::new(package_dir, event)?
411413
.workspace_path(root)
412414
.spawn()
413415
})
414-
.await?;
416+
.await
417+
{
418+
Ok(script) => script,
419+
Err(e) if is_optional => {
420+
let e: NodeMaintainerError = e.into();
421+
tracing::debug!("Error in optional dependency script: {}", e);
422+
return Ok(());
423+
}
424+
Err(e) => return Err(e.into()),
425+
};
415426
let stdout = script.stdout.take();
416427
let stderr = script.stderr.take();
417428
let stdout_name = name.clone();
@@ -420,7 +431,7 @@ impl HoistedLinker {
420431
let stderr_on_line = self.0.on_script_line.clone();
421432
let stdout_span = span;
422433
let stderr_span = stdout_span.clone();
423-
futures::try_join!(
434+
let join = futures::try_join!(
424435
async_std::task::spawn_blocking(move || {
425436
let _enter = stdout_span.enter();
426437
if let Some(stdout) = stdout {
@@ -451,7 +462,15 @@ impl HoistedLinker {
451462
script.wait()?;
452463
Ok::<_, NodeMaintainerError>(())
453464
}),
454-
)?;
465+
);
466+
match join {
467+
Ok(_) => {}
468+
Err(e) if is_optional => {
469+
tracing::debug!("Error in optional dependency script: {}", e);
470+
return Ok(());
471+
}
472+
Err(e) => return Err(e),
473+
}
455474
}
456475

457476
Ok::<_, NodeMaintainerError>(())

‎crates/node-maintainer/src/linkers/isolated.rs‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,8 @@ impl IsolatedLinker {
397397
.join(pkg.name())
398398
};
399399

400+
let is_optional = graph.is_optional(idx);
401+
400402
let build_mani =
401403
BuildManifest::from_path(pkg_dir.join("package.json")).map_err(|e| {
402404
NodeMaintainerError::BuildManifestReadError(pkg_dir.join("package.json"), e)
@@ -413,13 +415,21 @@ impl IsolatedLinker {
413415
on_script_start(&graph[idx].package, &event);
414416
}
415417
std::mem::drop(_span_enter);
416-
let mut script = async_std::task::spawn_blocking(move || {
418+
let mut script = match async_std::task::spawn_blocking(move || {
417419
OroScript::new(package_dir, event)?
418-
// For isolated installs, we don't add ancestor node_modules/.bin
419420
.workspace_path(package_dir_clone)
420421
.spawn()
421422
})
422-
.await?;
423+
.await
424+
{
425+
Ok(script) => script,
426+
Err(e) if is_optional => {
427+
let e: NodeMaintainerError = e.into();
428+
tracing::debug!("Error in optional dependency script: {}", e);
429+
return Ok(());
430+
}
431+
Err(e) => return Err(e.into()),
432+
};
423433
let stdout = script.stdout.take();
424434
let stderr = script.stderr.take();
425435
let stdout_name = name.clone();
@@ -428,7 +438,7 @@ impl IsolatedLinker {
428438
let stderr_on_line = self.0.on_script_line.clone();
429439
let stdout_span = span;
430440
let stderr_span = stdout_span.clone();
431-
futures::try_join!(
441+
let join = futures::try_join!(
432442
async_std::task::spawn_blocking(move || {
433443
let _enter = stdout_span.enter();
434444
if let Some(stdout) = stdout {
@@ -459,7 +469,15 @@ impl IsolatedLinker {
459469
script.wait()?;
460470
Ok::<_, NodeMaintainerError>(())
461471
}),
462-
)?;
472+
);
473+
match join {
474+
Ok(_) => {}
475+
Err(e) if is_optional => {
476+
tracing::debug!("Error in optional dependency script: {}", e);
477+
return Ok(());
478+
}
479+
Err(e) => return Err(e),
480+
}
463481
}
464482

465483
Ok::<_, NodeMaintainerError>(())

0 commit comments

Comments
 (0)