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).

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

edited

Loading