Repository navigation
Fix dependency expansion error handling - #13
Conversation
Reviewer's GuideDependency expansion now preserves ownership of partially built arrays across errors, cleans up nested allocations, rejects truncated manifest paths, and enforces the recursion limit on all branches; package tests add coverage confirming cyclic path dependencies terminate with a clear failure. Sequence diagram for dependency expansion error handlingsequenceDiagram
participant Install as pkg_install_core
participant Expand as pkg_expand_deps
participant Load as pkg_load
participant Collect as pkg_collect_deps
participant Free as pkg_free_deps
Install->>Expand: pkg_expand_deps(..., &expand_ok)
Expand->>Expand: Check depth > 32
alt Manifest path too long
Expand-->>Install: Return partially built list, expand_ok = false
else Manifest load fails
Expand->>Load: pkg_load(manifest_path)
Load-->>Expand: Failure
Expand-->>Install: Return partially built list, expand_ok = false
else Nested dependency collection fails
Expand->>Collect: pkg_collect_deps(...)
Collect-->>Expand: Failure
Expand->>Free: pkg_free_deps(nested, nested_count)
Expand-->>Install: Return partially built list, expand_ok = false
else Expansion succeeds
Expand-->>Install: Return expanded list, expand_ok = true
end
Install->>Install: Set deps = expanded and count = filled
alt expand_ok is false
Install->>Free: pkg_free_deps(deps, count)
end
Flow diagram for cyclic dependency depth-limit handlingflowchart TD
A[pkg_expand_deps] --> B{depth > 32?}
B -- No --> C[Load and collect nested dependencies]
C --> D{Nested dependency points back into cycle?}
D -- No --> E[Append dependencies and recurse]
E --> A
D -- Yes --> F[Recursion reaches depth limit]
B -- Yes --> G[Set error: dependency chain is deeper than 32]
G --> H[Set success = false]
H --> I[Return partially built list]
I --> J[pkg_install_core frees dependencies and exits with failure]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. If the expansion state or ownership handling is wrong, dependency installation could fail, retain an incorrect dependency set, or leave bounded package/lockfile changes that are not undone merely by reverting the code. The effects are repairable by cleaning up or rerunning installation, rather than creating an unbounded or irreversible security or data-loss impact.
Keep dependency arrays valid on expansion errors, clean up partial dependencies, reject overlong paths, and bound cyclic dependency resolution. Add a cycle regression test. GCC, Clang, macOS, Windows, Rust, and Sourcery checks pass.