Allow checkouts of repositories into non-empty directories by j-walther · Pull Request #2508 · GitoxideLabs/gitoxide

Jan Walther (j-walther) · 2026-04-09T12:29:27Z

Allow gix::clone::PrepareFetch callers to clone and check out into a destination directory that is not empty.

Before this change, PrepareFetch::new() unconditionally overrode create::Options::destination_must_be_empty to true regardless of what the caller passed, so the flag was effectively non-functional and there was no way to opt in to a non-empty destination through the public API.

gix::create::Options::destination_must_be_empty now controls this for non-bare repositories:

None (default for gix::init) — the directory may already contain files as long as no .git directory is present.
Some(true) (effective default for prepare_clone / prepare_clone_bare) — require an empty destination, preserving the prior clone behavior.
Some(false) — explicitly allow a non-empty destination for clones.

Bare repositories always require an empty destination.

If a clone fails before completion and the prepare handle is dropped, only the .git directory created by the clone is removed; any pre-existing user files in the destination are preserved.

Sebastian Thiel (Byron)

Thanks, but I think this needs test coverage.
I would also be afraid that it now allows writing non-empty directories, wiping data in the process.

Jan Walther (j-walther) · 2026-04-13T07:15:22Z

I would also be afraid that it now allows writing non-empty directories, wiping data in the process.
The current state is that it's effectively impossible to check-out into a non-empty directory making that user-facing flag effectively superfluous.

What would you expect tests for this to look like? Checking out into a non empty directory with the flag both set and not set? The latter would just be a regular checkout.

Sebastian Thiel (Byron) · 2026-04-13T07:32:01Z

There needs to be a test that fails if the change of this PR isn't present.
Also, there must be a test that shows non-empty directories cannot be checked out into to address

I would also be afraid that it now allows writing non-empty directories, wiping data in the process.

It might already exist as well, in which case you can point it out to me.

Jan Walther (j-walther) · 2026-04-13T10:17:55Z

I've added the tests. What I think should be done is to change the Default implementation to set destination_must_be_empty to true by default.

Sebastian Thiel (Byron)

I don't think I understand the premise. The reason that it won't allow cloning into a non-empty directory is that it will brutally overwrite everything. Git also rejects this, AFAIK.

❯ git clone https://github.com/GitoxideLabs/gitoxide ./src/
fatal: destination path './src' already exists and is not an empty directory.

The PR title is "Allow checkouts of empty repositories". Thus a test should show that empty repositories couldn't be checked out before, and after the fix they can.

I've added the tests. What I think should be done is to change the Default implementation to set destination_must_be_empty to true by default.

I agree, and wonder why I didn't that before.

One problem remains: I don't understand what the PR is trying to fix.

Copilot

Pull request overview

This PR adjusts gix::clone::PrepareFetch::new() to honor the caller-provided gix::create::Options instead of unconditionally requiring an empty destination directory for worktree clones, aligning clone initialization behavior with the documented semantics of create::Options. It also adds regression tests covering the new default behavior and the opt-in strict mode.

Changes:

Stop forcing create_opts.destination_must_be_empty = true in PrepareFetch::new_inner().
Add a test proving fetch+checkout into a non-empty destination directory works by default (and preserves pre-existing files).
Add a test proving destination_must_be_empty: true still rejects non-empty destinations for worktree clones.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
gix/src/clone/mod.rs	Removes the unconditional override of `destination_must_be_empty`, so clone respects caller-provided creation options.
gix/tests/gix/clone.rs	Adds coverage for cloning into non-empty worktree destinations by default and for the strict “must be empty” option.

Jan Walther (j-walther) · 2026-04-13T11:14:01Z

One problem remains: I don't understand what the PR is trying to fix.

Before my changes every code path set the destination_must_be_empty flag to true in a part of the code the user couldn't influence so you could never check out into a non-empty directory. This made the flag essentially meaningless since it'd effectively always be true. This PR removes the line that forces it to always be true so the user can actually opt into checking out into non-empty directories.

Jan Walther (j-walther) · 2026-04-13T11:18:25Z

I've updated the tests to better reflect the change and made it so destination_must_be_empty is true by default.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

## Summary Closes 9 compat-mode rows in tests/journey/parity/branch.sh, lands three reusable foundations consumed by future sprints (commit, push, tag, merge), and folds in 6 upstream syncs that arrived during the sprint. ## Closed parity rows (all bytes-mode, steward-verified, on-disk state asserted) - `-v` / `--verbose` — column-aligned name + SHA + subject - `-vv` — adds `[upstream: ahead N, behind N]` tracking annotation - `--abbrev=N` / `--no-abbrev` — SHA width override - `--column=always` — column-major packing - `-u` / `--set-upstream-to` — writes branch.<n>.{remote,merge} - `--unset-upstream` — clears them; emits `fatal: branch '<n>' has no upstream information` + exit 128 when absent - `--track` (create-side) — writes upstream config after ref creation - `--edit-description` — spawns editor, writes branch.<n>.description Plus three on-disk-state guard rows (chained gix → gix invocations) so bytes-mode parity tests can no longer mask missing-side-effect regressions in branch-config writers. ## Foundations introduced - gix::Repository::{set_branch_upstream, unset_branch_upstream, set_branch_description} with $GIT_DIR/config persistence via gix_config::File::write_to_filter on Source::Local sections (matches gix/src/clone/fetch/util.rs::setup_branch_config). - Branch::DESCRIPTION key constant + SnapshotMut::clear_subsection_value helper. - gitoxide-core::shared::columns::write_columns — column-major packer (reusable for status --column). - gitoxide-core::shared::editor::edit_file — $GIT_EDITOR/core.editor/$VISUAL/$EDITOR/vi resolver + shell-spawn (reusable for commit -e, tag --edit, merge --edit). ## Upstream syncs folded in - merge: pull upstream gix-main into gix-brit (security hardening + fuzz expansion) - merge: pull GitoxideLabs#2508 — allow checkouts into non-empty directories - merge: pull GitoxideLabs#2375 — filters and partial cloning: initial support - merge: pull GitoxideLabs#2503 — IOUC (UNTR extension) dirwalk speedup - merge: pull GitoxideLabs#2457 — gix-blame: Incremental - merge: pull GitoxideLabs#2465 — barebones server-side upload-pack ## Adjustments - steward agent: surface idiomatic Rust as an explicit design priority, reframing vendor/git as the *what* (flag surface, exit codes, output bytes) while gitoxide idioms anchor *how*. - Post-merge cleanup: dropped duplicate clone --filter stub (superseded by GitoxideLabs#2375) and pr-2457's orphaned log helpers (our log_all already handles paths). Plan: etc/plan/foundations-sprint-branch-render-and-config.md Pre-squash backup tag: pre-squash-foundations Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Jan Walther (j-walther) · 2026-05-11T15:10:23Z

Hi Sebastian Thiel (@Byron), this is rebased on top of main and addresses the open Drop-safety concern that was raised in both your review and the Copilot reviewer's follow-ups.

Changes since you last looked:

Drop safety fix (36f3805): PrepareFetch and PrepareCheckout now track whether the destination directory was empty at construction time. On drop, only the .git directory we created is removed when the destination held pre-existing user files — remove_dir_all(workdir) is reserved for the empty-destination case. This directly addresses your concern that opting in to non-empty destinations could otherwise wipe data on a failed clone.
Regression test for that exact failure path (drop_after_failed_fetch_into_non_empty_directory_preserves_pre_existing_files in gix/tests/gix/clone.rs) — I verified it fails without the Drop fix and passes with it.
Field doc updated on Options::destination_must_be_empty to spell out the None / Some(true) / Some(false) semantics.
Rebased onto main (clean except for two small test-file conflicts already resolved).

The bool → Option<bool> change is intentional and the cleanest way I found to let gix::init keep its documented permissive default while prepare_clone stays strict by default. Happy to revisit the API shape if you'd prefer a different split.

CI is green.

Also make turn `destination_must_be_empty` into `Option<bool>`

Sebastian Thiel (Byron)

This PR got lucky as I nerd-sniped myself into probably putting more time in it as it would have cost me if I would have implemented this feature from the start.

Nowadays, I think it costs more to lift a mediocre PR into what's needed than to do it from scratch given agent support, and it's something I will consider in future, which probably leans more towards closing such PRs than engaging with it.

Lesson learned, I hope.

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Co-authored-by: Sebastian Thiel <[email protected]>

Jan Walther (j-walther) force-pushed the feat/allow-empty-dir branch from 3c5b2d5 to 352d216 Compare April 9, 2026 12:33

Sebastian Thiel (Byron) requested changes Apr 11, 2026

View reviewed changes

Jan Walther (j-walther) force-pushed the feat/allow-empty-dir branch from dfadc8b to 592b69d Compare April 13, 2026 09:45

Sebastian Thiel (Byron) requested changes Apr 13, 2026

View reviewed changes

Sebastian Thiel (Byron) requested a review from Copilot April 13, 2026 10:42

Copilot started reviewing on behalf of Sebastian Thiel (Byron) April 13, 2026 10:43 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Comment thread gix/src/clone/mod.rs

Jan Walther (j-walther) changed the title ~~Allow checkouts of empty repositories~~ Allow checkouts of repositories into non-empty directories Apr 13, 2026

Jan Walther (j-walther) force-pushed the feat/allow-empty-dir branch from d090222 to e6727f1 Compare April 13, 2026 11:30

Jan Walther (j-walther) commented Apr 13, 2026

View reviewed changes

Comment thread gix/src/lib.rs Outdated

Jan Walther (j-walther) requested review from Sebastian Thiel (Byron) and Copilot April 14, 2026 15:17

Copilot started reviewing on behalf of Jan Walther (j-walther) April 14, 2026 15:17 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Comment thread gix/src/clone/mod.rs Outdated

Comment thread gix/src/create.rs Outdated

Jan Walther (j-walther) requested a review from Copilot April 20, 2026 14:44

Copilot started reviewing on behalf of Jan Walther (j-walther) April 20, 2026 14:44 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Comment thread gix/src/create.rs Outdated

Comment thread gix/src/clone/mod.rs

Comment thread gix/tests/gix/clone.rs

Comment thread gix/src/create.rs Outdated

Comment thread gix/src/create.rs

Jan Walther (j-walther) force-pushed the feat/allow-empty-dir branch from a71f6ad to 36f3805 Compare May 11, 2026 14:36

feat!: Allow checkouts of empty repositories

e75fbfa

Also make turn `destination_must_be_empty` into `Option<bool>`

Sebastian Thiel (Byron) force-pushed the feat/allow-empty-dir branch from add0e65 to 444ece4 Compare May 12, 2026 07:48

Sebastian Thiel (Byron) approved these changes May 12, 2026

View reviewed changes

Sebastian Thiel (Byron) force-pushed the feat/allow-empty-dir branch from 444ece4 to d6da476 Compare May 12, 2026 07:52

Sebastian Thiel (Byron) requested a review from Copilot May 12, 2026 08:10

Copilot started reviewing on behalf of Sebastian Thiel (Byron) May 12, 2026 08:11 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread gix/src/clone/mod.rs Outdated

Comment thread gix/src/create.rs Outdated

Comment thread gix/src/init.rs Outdated

Comment thread gix/tests/gix/clone.rs

Sebastian Thiel (Byron) force-pushed the feat/allow-empty-dir branch from d6da476 to c77081d Compare May 12, 2026 08:33

Sebastian Thiel (Byron) requested a review from Copilot May 12, 2026 08:36

Copilot started reviewing on behalf of Sebastian Thiel (Byron) May 12, 2026 08:37 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread gix/src/clone/mod.rs Outdated

Sebastian Thiel (Byron) enabled auto-merge May 12, 2026 08:43

review

8606b7a

Co-authored-by: Sebastian Thiel <[email protected]>

Sebastian Thiel (Byron) force-pushed the feat/allow-empty-dir branch from c77081d to 8606b7a Compare May 12, 2026 08:45

Sebastian Thiel (Byron) merged commit 9013c3f into GitoxideLabs:main May 12, 2026
32 checks passed

Uh oh!

Uh oh!

Conversation

Jan Walther (j-walther) commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sebastian Thiel (Byron) left a comment

Choose a reason for hiding this comment

Uh oh!

Jan Walther (j-walther) commented Apr 13, 2026

Uh oh!

Sebastian Thiel (Byron) commented Apr 13, 2026

Uh oh!

Jan Walther (j-walther) commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sebastian Thiel (Byron) left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Jan Walther (j-walther) commented Apr 13, 2026

Uh oh!

Jan Walther (j-walther) commented Apr 13, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jan Walther (j-walther) commented May 11, 2026

Uh oh!

Sebastian Thiel (Byron) left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jan Walther (j-walther) commented Apr 9, 2026 •

edited

Loading

Jan Walther (j-walther) commented Apr 13, 2026 •

edited

Loading