Conversation
…s helpers Implements the five top-level subcommands (add, update, status, remove, list) driven by a clap derive CLI in the new cli.rs. Two trait changes support the implementation: checkout_vendor now returns the full overlay tree OID (so callers can mint a commit from the same tree without re-running the overlay), and prepare_merge is promoted from an internal exe helper to a first-class VendorWorktree method. The gitattributes quoting helpers (quote_attr_pattern, unquote_attr_pattern, find_pattern_end) are replaced with two focused helpers: check_attr_pattern returns Error::InvalidPath for paths that would require C-style quoting (space, #, control chars — never seen on real git source paths), and split_attr_line delegates to gix_quote::ansi_c::undo for the decode side. gix-quote is already a transitive dependency; this makes it direct. The attributes feature is enabled on gix for future use. Unit tests move to src/exe_tests.rs and use rstest for parameterized cases. feat: add CLI subcommands add, update, status, remove, list feat: checkout_vendor returns full overlay tree OID feat: add prepare_merge to VendorWorktree trait refactor: replace quoting helpers with check_attr_pattern + split_attr_line feat: add Error::InvalidPath chore: add gix-quote direct dep; enable gix attributes feature Assisted-by: Claude:claude-sonnet-4-6
Covers merge mode, squash mode, and overwrite behaviour in prepare_merge. Adds a test asserting that track_vendor rejects paths requiring gitattributes quoting with Error::InvalidPath, replacing the former path_with_space_is_quoted / idempotent_for_path_with_space tests that were invalidated when quoting support was removed. test: add prepare_merge integration tests (merge, squash, overwrite) test: add path_with_space_returns_invalid_path_error to track_vendor Assisted-by: Claude:claude-sonnet-4-6
split_attr_line now returns None for comment lines (starting with '#') and lines with a leading-whitespace separator, matching its documented contract. .gitvendors is now written into every commit tree and staged into the index alongside .gitattributes; previously it was only written to disk, so collaborators who pulled got vendored files but no config. On merge conflicts, save_config + stage_gitvendors + prepare_merge are now called before exit(1), so the user's git commit produces a proper merge commit and the config entry survives the resolution round-trip. cmd_add always auto-commits; the message.is_some() gate was removed because the CLI docs advertise a default message, making the split behaviour surprising. cmd_remove now removes deleted vendor file entries from the index after deleting them from the worktree, so a plain git commit records the removal rather than leaving the files tracked. fix: split_attr_line returns None for comment and whitespace-only lines fix: .gitvendors included in commit tree and staged alongside .gitattributes fix: conflict path writes MERGE_HEAD and saves config before exiting fix: cmd_add always auto-commits fix: cmd_remove stages index deletions for removed vendor files test: extend split_no_attr_returns_none with comment and whitespace cases Assisted-by: Claude:claude-sonnet-4-6
git vendor add <url> now derives the vendor name from the last path component of the URL, stripping .git / .bundle suffixes, matching the convention used by git clone and git submodule add. Assisted-by: Claude:claude-sonnet-4-6
Default prefix to vendor/<name>/, rename --ref-name to --ref, add --dry-run to add and update, and add rm/ls as visible aliases for remove/list. feat: default vendored files to vendor/<name>/ when no patterns given feat: rename --ref-name to --ref feat: add --dry-run to add and update feat: add visible aliases rm and ls Assisted-by: Claude:claude-fable-5[1m]
`git vendor apply [name]` rebuilds vendored files from the recorded upstream base per the current `.gitvendors` patterns, without fetching. Editing a vendor's pattern entries and running apply moves or refilters its files; the change is recorded as a single-parent commit. A vendor whose files carry local modifications is skipped with a list of the modified paths unless --force is given, since re-materializing discards them. Assisted-by: Claude:claude-fable-5[1m]
When a non-bare local repo fetches with `+HEAD:refs/vendor/<name>` and shares the upstream's default branch name, gix writes the vendor ref as a symbolic ref pointing to the local branch. Re-reading that ref via `peel_to_id()` returns the local HEAD instead of the upstream tip, and no upstream objects land in the odb. The refmap built during the fetch carries the upstream OID (via `Mapping::remote.peeled_id()`) before any local ref resolution, so reading from there sidesteps the issue entirely. The bare-only guard on the File transport scheme is also removed — it existed only because of this bug. Removes `#[should_panic]` from the pinning regression test added in 4c83407. Upstream: GitoxideLabs/gitoxide#2613. fix: read upstream OID from outcome.ref_map rather than local vendor ref fix: allow local (file://) transport for non-bare repos Assisted-by: Claude:claude-sonnet-4-6
fix: append trailing / to --prefix so it is always treated as a directory feat: `add` stages vendored files and sets MERGE_HEAD rather than auto-committing; user runs `git commit` to finalize Assisted-by: Claude:claude-sonnet-4-6
`cmd_remove` wrote the updated `.gitvendors` to the working tree via `save_config` but never staged it, so the index kept pointing at the old blob that still listed the removed vendor; the user's next commit would re-record the deleted entry. Stage the rewritten config like the other mutating commands do. Assisted-by: Claude:claude-opus-4-8
The conflict branches of `cmd_add` and `cmd_update` checked out the conflicted tree and prepared the merge but skipped `reconcile_tracked_paths`. A path introduced by the merge (e.g. a new upstream file landing next to a conflict) was therefore never written to `.gitattributes`, so after the user resolved and committed, later `status`/`update`/`remove` could not see it. The conflict is in file content, not in which paths exist, so the mapping can be recorded up front just as the clean path does. Assisted-by: Claude:claude-opus-4-8
The helper was named and documented as if it wrote `.gitattributes` ("Write
`content` as a blob, upsert the index entry"), but `track_vendor` already
stages the file and this only reads back the resulting blob OID. Rename it to
`staged_attrs_blob`, correct the doc, and drop the two `cmd_add` calls that
discarded the OID — they asserted a post-condition `track_vendor` already
guarantees.
Assisted-by: Claude:claude-opus-4-8
Use `is_ascii_control()` for the C0-and-DEL range and drop the redundant tab arm, leaving only the four genuinely special characters in the `matches!` list. Assisted-by: Claude:claude-opus-4-8
Splits the binary into a minimal entry point and a dedicated executor module. All command logic migrates from free functions in `main.rs` to methods on `Executor` that accept `&mut Io`, enabling output capture for future tests and alternate frontends. The `VendorWorktree` impl and its helpers move from the library's `exe` module into `lib.rs` alongside `VendorRepository`, eliminating the library-side `exe` module. The worktree test file is renamed to `attr_tests.rs` to match its scope. refactor: move `VendorWorktree` impl from `exe.rs` into `lib.rs` refactor: introduce `Executor` struct and `Io` for injectable output refactor: replace `std::process::exit` on conflict with `ConflictExit` refactor: rename `exe_tests.rs` to `attr_tests.rs` Assisted-by: Claude:claude-sonnet-4-6
Both methods now return the OID of the staged `.gitattributes` blob instead of `()`. `reconcile_tracked_paths` propagates the OID to its callers, eliminating the `staged_attrs_blob` round-trip that read the OID back out of the index after the fact. refactor: `track_vendor` -> `Result<gix::ObjectId, Error>` refactor: `untrack_vendor` -> `Result<Option<gix::ObjectId>, Error>` refactor: `stage_gitattributes` -> `Result<gix::ObjectId, Error>` refactor: `reconcile_tracked_paths` -> `Result<gix::ObjectId>` refactor: remove `staged_attrs_blob` Assisted-by: Claude:claude-sonnet-4-6
gixs fetch machinery can write `refs/vendor/<name>` as a symref into the local branch namespace instead of a direct ref to the fetched commit, when the remotes HEAD is symbolic and a local branch shares its target name (gitoxide#2613). `fetch_vendor` already reads the correct OID from the refmap to work around this for its own return value, but `vendor_tip`/`vendor_status` read the ref directly and were still silently resolving the corrupted symref to the local branch tip. Force-write the ref to the resolved OID after every fetch so all readers see the same, correct value; this also means the ref never stores an intermediate tag object for annotated tags, so it always matches `fetch_vendor`s peeled return value. fixes: `vendor_tip`/`vendor_status` reading a gix-corrupted local ref Assisted-by: Claude:claude-sonnet-5
`peeled_id()` on the refmap mapping only returns a peeled value when the ref advertisement carried one. Fetching an annotated tag by name gets that advertisement, but fetching by the tag objects own SHA (`Source::ObjectId`) does not, so `fetch_vendor` could return the raw tag object instead of its target commit. Downstream code assumes `entry.base` and the vendor ref always name a commit. Peel explicitly after the refmap lookup so both fetch paths agree. fixes: `fetch_vendor` returning an unpeeled tag object for `--ref <sha>` Assisted-by: Claude:claude-sonnet-5
Both call sites used `.unwrap_or_default()`, so a real error (e.g. an object-database read failure) was silently treated as "vendor owns no old paths". `reconcile_tracked_paths` would then never untrack files the upstream removed, leaving stale `vendor=<name>` lines in `.gitattributes` that misclassify later user files as vendor-owned. `tests/vendor_paths/table.rs` (`non_commit_ours_is_error`, `nonexistent_oid_ours_is_error`) already proves `vendor_paths` returns `Err` for bad input; a CLI-level repro that reaches this exact call site is impractical since `current_head` there is always a real HEAD commit, so no new test is added for the `update`/`apply` path itself. fixes: `update`/`apply` silently ignoring `vendor_paths` failures Assisted-by: Claude:claude-sonnet-5
Every caller already knows the exact parent commit it started from, but advance_head hardcoded `PreviousValue::Any`, so anything else moving HEAD between the snapshot and the ref write (a hook, a concurrent process, a slow multi-vendor update loop) got silently overwritten with no error. Take the expected parent explicitly and require HEAD to still match it at write time. fixes: advance_head force-moving HEAD instead of compare-and-swapping Assisted-by: Claude:claude-sonnet-5
`untrack_vendor` was called only inside the `!keep_files` branch, so `--keep-files` left stale `vendor=<name>` lines in `.gitattributes` even though its own help text promises to remove config and attribute tracking while leaving the files in place. A later `add` of the same vendor name would then see the kept files as already vendor-owned via those stale attributes. Split the function so untracking always runs when the entry has tracked paths, and only file/index deletion is gated on `!keep_files`. fixes: `remove --keep-files` leaving stale `.gitattributes` entries Assisted-by: Claude:claude-sonnet-5
File deletion, `untrack_vendor`, and index cleanup were all gated on `repo.head_commit()` succeeding. On an unborn HEAD (`git init; git vendor add ...` before the first commit), `remove` printed "Removed vendor" and exited zero, but only the `.gitvendors` entry actually went away — vendored files stayed on disk, in the index, and in `.gitattributes`. `vendor_paths` needs a commit to read a tree from, so add `resolve_vendor_paths_uncommitted` to resolve the same `vendor=<name>` attribute selection from the current index instead, and use it when there is no HEAD commit yet. fixes: `remove` on an unborn HEAD leaving the vendor in place Assisted-by: Claude:claude-sonnet-5
`check_attr_pattern` rejects any upstream path that would need C-style quoting in `.gitattributes`, but it only ran inside `track_vendor`, which every `add`/`update`/`apply` call site invokes *after* `checkout_vendor`/`checkout_vendor_conflicted` has already rewritten the working tree and index. An upstream containing an unquotable path (e.g. a filename with a space) left vendor files checked out and staged with no `.gitvendors` entry and no `.gitattributes` tracking — a half-applied state with no way to retry, since the vendor could never be added at all. Expose the check as `validate_trackable_paths` and call it against the merge/upstream tree's paths before any checkout, in every branch of `add`, `update`, and `apply`. fixes: `add`/`update`/`apply` leaving a half-applied checkout on an unquotable path Assisted-by: Claude:claude-sonnet-5
`check_attr_pattern` rejected whitespace/quote/control characters but not `*`, `?`, `[`, or a pattern-initial `!`/`#`, and `track_vendor` wrote the raw upstream path bytes verbatim as the pattern. A file literally named `a*` became the live glob `vendor/mylib/a*`, matching unrelated siblings, which `checkout_vendor`'s cleanup or `remove` could then delete as if they were vendor-owned. A file named `a[0].c` had the opposite problem: its own generated pattern never matched it. Escape `*`/`?`/`[`/leading `!`/`#` with a backslash on write, and unescape symmetrically wherever a parsed pattern is compared back against a raw path (dedup in `track_vendor`, removal in `untrack_vendor`) so re-running `update` doesn't see every escaped line as new and duplicate it. fixes: unescaped glob metacharacters in generated `.gitattributes` patterns Assisted-by: Claude:claude-sonnet-5
The on-disk index was rebuilt from `full_tree`, the vendor tree overlaid onto HEAD's *committed* tree — never the index. Any entry staged but not yet committed (an addition or a modification) was silently dropped: `git add newfile.txt` followed by `git vendor update` unstaged `newfile.txt` with no warning. On an unborn HEAD this wiped the entire pre-existing index, since there was no committed tree to overlay onto at all. Derive the on-disk index from the actual current index instead (or an empty one if none exists yet), and apply only this vendor's own path removals/additions to it, leaving every other entry untouched. `remove_entries`/`dangerously_push_entry` don't invalidate the index's cached-tree extension, so also drop it explicitly — otherwise a native `git commit` right after would trust the stale cache and record the previous tree, silently dropping the vendor's changes. `full_tree` is still computed and returned unchanged; callers need that OID to mint commits regardless of what the on-disk index holds. fixes: `checkout_vendor` dropping staged-but-uncommitted index entries Assisted-by: Claude:claude-sonnet-5
A vendor pattern that maps an upstream file onto `.gitattributes` itself (e.g. a root mapping with no destination prefix) can produce a genuine merge conflict on that path, which `checkout_vendor_conflicted` correctly splices into the index as unmerged stage 1/2/3 entries. But `reconcile_tracked_paths` always went on to call `track_vendor`, which reads and rewrites the (conflict-marker-laden) working copy file, and `stage_gitattributes`, which unconditionally replaces every existing stage with a single stage-0 entry — silently resolving the conflict so `git commit` would succeed with conflict-marker text committed as normal content. Check for unmerged stages on `.gitattributes` before writing, and skip the write entirely when found, leaving the stages exactly as `checkout_vendor_conflicted` left them and telling the user to resolve it manually. This is the most speculative of the ten findings from the adversarial review (a conflict landing on `.gitattributes` itself requires an unusual root-mapping pattern); the accompanying test drives the guard directly with a synthetic conflicted `VendorMerge` rather than reproducing the full upstream layout end to end. fixes: silent collapse of an unresolved .gitattributes merge conflict Assisted-by: Claude:claude-sonnet-5
Matches the existing lib.rs/attr_tests.rs convention: cfg(test) mod with #[path] pointing at a standalone file, keeping test code out of the source file. Assisted-by: Claude:claude-sonnet-5
Neither `git submodule add` nor `git submodule update --remote` commit on the caller's behalf, so `git vendor` now matches: `update` (fetch or --no-fetch) always stages its result and tells the user to run `git commit`, instead of auto-committing multi-vendor runs while single-vendor runs staged. `apply` duplicated `update`'s "rebuild from recorded base" logic under a different name, so it's now `update --no-fetch`. fix: reject `add` of a vendor name that already exists Re-running `add` with an existing name silently re-fetched and staged nothing (an empty "in-progress merge" with no diff), forcing the user to discover `git merge --abort` on their own. It now errors up front, same as `git remote add`. refactor: drop `apply`, `advance_head`, `author_sig`, `committer_sig` fix: stop multi-vendor `update` from clobbering earlier vendors' pending MERGE_HEAD fix: reject `add <url> <existing-name>` instead of silently corrupting merge state Assisted-by: Claude:claude-sonnet-5
`add` and `update` each inlined an identical checkout/reconcile/stage sequence for applying a merge result; factored into `apply_merge`. The name-or-all entry lookup repeated in `update`, `update_no_fetch`, and `status` is now `resolve_entries`. The local-modification diff in `update_no_fetch` is now `locally_modified_paths`. Assisted-by: Claude:claude-sonnet-5
`remove` deleted individual vendored files but left now-empty parent directories behind on disk. Assisted-by: Claude:claude-sonnet-5
Seed from a run that hit ENOSPC mid-test, not a genuine logic failure; case now passes and is checked in per convention. Assisted-by: Claude:claude-sonnet-5
The seed came from a run that hit ENOSPC mid-test, not a genuine logic failure in the code under test. Assisted-by: Claude:claude-sonnet-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
reqwestfeatures.gitvendorsonremoveadd/updateconflictsstaged_attrs_blobonly reads the indexcheck_attr_patternspecial-character testtrack_vendoranduntrack_vendorrefs/vendor/<name>to a direct ref after fetchvendor_pathserrors inupdate/applyremove --keep-filesremovework on an unborn HEADapplyintoupdate --no-fetch, stop auto-committing