feat(pm): internalize git dependency resolution into ruborist#2603
feat(pm): internalize git dependency resolution into ruborist#2603killagu-claw wants to merge 1 commit intoutooland:nextfrom
Conversation
Summary of ChangesHello @killagu-claw, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors how git dependencies are managed within the project's package management system. By moving the core git cloning and resolution capabilities into the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant feature by internalizing git dependency resolution into ruborist. The changes are well-structured, moving git-related logic behind a feature flag and providing a clear separation between registry and git dependency handling. The implementation uses gix for git operations and includes caching for cloned repositories. The parsing of different package specifications is now centralized and more robust.
I've found one potential issue regarding cache collisions for git repositories and a minor point of code clarity. Overall, this is a great enhancement to the package manager's capabilities.
8a0b47a to
254abed
Compare
e49ed37 to
37cdfea
Compare
| // The resolver still emits a PackageResolved event with a git tarball URL, | ||
| // so we intentionally skip the download here to avoid a redundant fetch. | ||
| if tarball_url.starts_with("git+") || tarball_url.starts_with("git://") { | ||
| tracing::debug!( |
There was a problem hiding this comment.
pipeline 模式下,worker 也应该前置进行 clone 操作
crates/ruborist/Cargo.toml
Outdated
|
|
||
| [features] | ||
| default = [] | ||
| git = ["gix", "tempfile"] |
There was a problem hiding this comment.
feature 命令为 git 不太语义,git/github 依赖可以通过 http api 来 fallback ?
wasm 场景应该也能支持,或者通过 cfg 明确进行报错
There was a problem hiding this comment.
这个先这样吧。我另外再起一个 PR,让 gix 也支持 wasm 的。现在这个 pr 有点太大了。
37cdfea to
675dc69
Compare
Move git clone & resolution logic into `utoo-ruborist` behind a `native-git` Cargo feature flag. Git specs (git+https://, github:, etc.) are now resolved on-the-fly during BFS traversal via `resolve_non_registry_dep`, so transitive git dependencies work. Key changes ----------- - Add gix/tempfile as optional deps under `native-git` feature - Add `resolve_non_registry_dep` with dual cfg (enabled / noop error) - Route `is_non_registry_spec` edges through git resolver in BFS - Add `PackageSpec` enum and `parse_cli_spec` for typed spec parsing - Add shared `git_cache_path` helper with URL-hash namespacing to prevent cross-repo SHA prefix collisions (layout: `_git/<url_hash>/<sha_prefix>/package/`) - Extract `format_save_spec` for clean version-to-write logic - Extract `resolve_cache_path` / `is_git_url` / `git_cache_lookup` so download_to_cache only handles registry tarballs - PM enables `utoo-ruborist/native-git` instead of depending on gix - `BuildDepsOptions<G, R>` stays at 2 generics; no git types leak to PM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1747c26 to
f9e9479
Compare
Summary
utoo-ruboristbehind agitCargo feature flaggit+https://,github:, etc.) are now resolved on-the-fly during BFS traversal viaresolve_non_registry_dep, so transitive git dependencies work correctlygixdirectly — it enablesutoo-ruborist/gitinsteadBuildDepsOptions<G, R>stays at 2 generics; noGitResolvertrait in public APIgit_cache_pathhelper to deduplicate SHA-prefix cache logicis_local_specalias, clarify worker.rs skip commentTest plan
cargo build -p utoo-ruborist(without git feature)cargo build -p utoo-ruborist --features gitcargo build -p utoo-pmcargo test -p utoo-ruborist— all 122 tests pass (1 pre-existing doctest failure on non-darwin)cargo test -p utoo-pm— 190/191 pass (1 pre-existing failure intest_run_script_not_found)ut add github:user/reporesolves inside BFS🤖 Generated with Claude Code