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

This line effectively prevented checking out in non-empty directories in all cases.

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.

Copilot · 2026-04-13T10:45:47Z

    #[allow(clippy::result_large_err)]
    fn new_inner(
        mut url: gix_url::Url,
        path: &std::path::Path,
        kind: crate::create::Kind,
-        mut create_opts: crate::create::Options,
+        create_opts: crate::create::Options,
        open_opts: crate::open::Options,
    ) -> Result<Self, Error> {
-        create_opts.destination_must_be_empty = true;
        let mut repo = crate::ThreadSafeRepository::init_opts(path, kind, create_opts, open_opts)?.to_thread_local();
        url.canonicalize(repo.options.current_dir_or_empty())


The PR title mentions "checkouts of empty repositories", but the code change (and new tests) are about allowing clone/fetch+checkout into a non-empty destination directory by default (unless destination_must_be_empty is set). Consider updating the PR title (or description) to match the actual behavior change to avoid confusion in release notes/changelog context.

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.

Jan Walther (j-walther) · 2026-04-13T11:31:46Z

+            destination_must_be_empty: false,
+            ..Default::default()
+        },
+    )


Restores previous behaviour which was also enforced by the init_into_non_empty_directory_is_allowed_by_default test.

Copilot

Pull request overview

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

Copilot · 2026-04-14T15:22:39Z

+        create_opts: crate::create::Options,
        open_opts: crate::open::Options,
    ) -> Result<Self, Error> {


Now that PrepareFetch::new_inner() no longer forces destination_must_be_empty = true, callers can legitimately pass destination_must_be_empty: false. However, PrepareFetch/PrepareCheckout currently auto-clean up in Drop by calling remove_dir_all(repo.workdir()…), which will delete the entire destination directory—including pre-existing user files—if clone/fetch/checkout fails or the handle is dropped without persist(). Please adjust cleanup to be safe for non-empty destinations (e.g., track whether the destination was newly created/empty and only remove_dir_all in that case, otherwise only remove the created .git dir or disable auto-cleanup and require explicit cleanup by the caller).

Suggested change

create_opts: crate::create::Options,

open_opts: crate::open::Options,

) -> Result<Self, Error> {

mut create_opts: crate::create::Options,

open_opts: crate::open::Options,

) -> Result<Self, Error> {

create_opts.destination_must_be_empty = true;

Copilot · 2026-04-14T15:22:40Z

@@ -119,6 +119,15 @@ pub struct Options {
    pub fs_capabilities: Option<gix_fs::Capabilities>,
 }

+impl Default for Options {
+    fn default() -> Self {
+        Options {
+            destination_must_be_empty: true,
+            fs_capabilities: None,
+        }


create::Options's field docs say worktree repos can be initialized into a non-empty directory “by default” (as long as no .git exists), but Default for Options now sets destination_must_be_empty: true, which makes the default behavior the opposite. Please align documentation and behavior (either change the default back to false and have clone code set it to true, or update the docs to state the default is strict and that gix::init()/CLI init explicitly override it to false).

Copilot

Pull request overview

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

Copilot · 2026-04-20T14:49:57Z

+        if create_opts.destination_must_be_empty.is_none() {
+            create_opts.destination_must_be_empty = Some(true);
+        }


Allowing destination_must_be_empty = Some(false) makes it possible to clone/checkout into an existing directory, but PrepareFetch/PrepareCheckout currently remove_dir_all() the repository workdir on drop. If a fetch/checkout fails (or the handle is dropped early), this will delete the entire destination directory including pre-existing user files. Consider changing cleanup to only remove what was created (e.g. just the .git dir for worktree repos, or track whether the destination dir existed/was empty and only then delete the whole directory).

Copilot · 2026-04-20T14:49:57Z

+    fn fetch_and_checkout_into_non_empty_directory() -> crate::Result {
+        let tmp = gix_testtools::tempfile::TempDir::new()?;
+        let existing_path = tmp.path().join("existing.txt");
+        let existing_content = b"I was here before you";
+        std::fs::write(&existing_path, existing_content)?;
+
+        let mut prepare = gix::clone::PrepareFetch::new(
+            remote::repo("base").path(),
+            tmp.path(),
+            gix::create::Kind::WithWorktree,
+            gix::create::Options {
+                destination_must_be_empty: Some(false),
+                ..Default::default()
+            },
+            restricted(),
+        )?;
+        let (mut checkout, _out) =
+            prepare.fetch_then_checkout(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?;
+        let (repo, _) = checkout.main_worktree(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?;


This new behavior (checkout into a non-empty destination) would benefit from a regression test that covers the failure/rollback path: if fetch_then_checkout() or main_worktree() errors, dropping the prepare/checkout handle should not delete or modify pre-existing files in the destination directory.

Copilot · 2026-04-20T14:49:58Z

    /// If true, and the kind of repository to create has a worktree, then the destination directory must be empty.
    ///
    /// By default repos with worktree can be initialized into a non-empty repository as long as there is no `.git` directory.


destination_must_be_empty is now an Option<bool>, but the field docs still describe only the true/false cases. Please update the doc comment to explain the None behavior (what default is assumed, and how callers should use Some(true) vs Some(false)), especially since this is a public API surface.

Suggested change

/// If true, and the kind of repository to create has a worktree, then the destination directory must be empty.

///

/// By default repos with worktree can be initialized into a non-empty repository as long as there is no `.git` directory.

/// Control whether the destination directory must be empty when creating a repository with a worktree.

///

/// If this is `None`, the default is used: repositories with a worktree may be initialized into a non-empty

/// directory as long as there is no `.git` directory already present.

///

/// Use `Some(true)` to explicitly require an empty destination directory for repositories with a worktree.

/// Use `Some(false)` to explicitly allow initialization into a non-empty destination directory, subject to the

/// absence of a `.git` directory.

Copilot · 2026-04-20T14:49:58Z

    ///
    /// By default repos with worktree can be initialized into a non-empty repository as long as there is no `.git` directory.
-    pub destination_must_be_empty: bool,
+    pub destination_must_be_empty: Option<bool>,


Changing destination_must_be_empty from bool to Option<bool> is a breaking change to the public gix::create::Options API. If avoiding breakage is desired, consider an alternative that preserves the existing field (e.g. a new field or dedicated clone option) or ensure this is called out appropriately for downstream users (versioning/release notes).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

## 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) <noreply@anthropic.com>

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

Allow checkouts of empty repositories

592b69d

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

Add tests for checking out in non-empty folder

e6682a4

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

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

Make destination_must_be_empty default to true

e6727f1

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

Jan Walther (j-walther) added 2 commits April 14, 2026 12:24

Trigger Build

a1c482f

Fix ein binary

7ab8a06

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

Make destination_must_be_empty Option<bool>

6575382

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

Update gix/src/create.rs

a71f6ad

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

-    /// If true, and the kind of repository to create has a worktree, then the destination directory must be empty.
-    ///
-    /// By default repos with worktree can be initialized into a non-empty repository as long as there is no `.git` directory.
+    /// Control whether the destination directory must be empty when creating a repository with a worktree.
+    ///
+    /// If this is `None`, the default is used: repositories with a worktree may be initialized into a non-empty
+    /// directory as long as there is no `.git` directory already present.
+    ///
+    /// Use `Some(true)` to explicitly require an empty destination directory for repositories with a worktree.
+    /// Use `Some(false)` to explicitly allow initialization into a non-empty destination directory, subject to the
+    /// absence of a `.git` directory.

Uh oh!

Conversation

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

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!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Jan Walther (j-walther) Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

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

Uh oh!

Jan Walther (j-walther) Apr 13, 2026

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!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

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!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

edited

Loading