Skip to content

Commit 9013c3f

Browse files
ByronByron
authored
Merge pull request #2508 from j-walther/feat/allow-empty-dir
Allow checkouts of repositories into non-empty directories
2 parents 2d4a6f2 + 8606b7a commit 9013c3f

10 files changed

Lines changed: 254 additions & 27 deletions

File tree

‎gix/src/clone/access.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl PrepareFetch {
7373
impl Drop for PrepareFetch {
7474
fn drop(&mut self) {
7575
if let Some(repo) = self.repo.take() {
76-
std::fs::remove_dir_all(repo.workdir().unwrap_or_else(|| repo.path())).ok();
76+
super::cleanup_clone_destination_on_drop(&repo, self.remove_worktree_on_drop);
7777
}
7878
}
7979
}

‎gix/src/clone/checkout.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ impl PrepareCheckout {
171171
impl Drop for PrepareCheckout {
172172
fn drop(&mut self) {
173173
if let Some(repo) = self.repo.take() {
174-
std::fs::remove_dir_all(repo.workdir().unwrap_or_else(|| repo.path())).ok();
174+
super::cleanup_clone_destination_on_drop(&repo, self.remove_worktree_on_drop);
175175
}
176176
}
177177
}

‎gix/src/clone/fetch/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ impl PrepareFetch {
318318
crate::clone::PrepareCheckout {
319319
repo: repo.into(),
320320
ref_name: self.ref_name.clone(),
321+
remove_worktree_on_drop: self.remove_worktree_on_drop,
321322
},
322323
fetch_outcome,
323324
))

‎gix/src/clone/mod.rs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ pub struct PrepareFetch {
4242
/// The name of the reference to fetch. If `None`, the reference pointed to by `HEAD` will be checked out.
4343
#[cfg_attr(not(feature = "blocking-network-client"), allow(dead_code))]
4444
ref_name: Option<gix_ref::PartialName>,
45+
/// If `true`, drop removes the entire worktree. Otherwise leave it alone.
46+
remove_worktree_on_drop: bool,
4547
}
4648

4749
/// The error returned by [`PrepareFetch::new()`].
@@ -102,7 +104,19 @@ impl PrepareFetch {
102104
mut create_opts: crate::create::Options,
103105
open_opts: crate::open::Options,
104106
) -> Result<Self, Error> {
105-
create_opts.destination_must_be_empty = true;
107+
if create_opts.destination_must_be_empty.is_none() {
108+
create_opts.destination_must_be_empty = Some(true);
109+
}
110+
111+
// Capture this before init_opts creates `.git`, otherwise the check below would see our own files.
112+
let remove_worktree_on_drop = match std::fs::read_dir(path) {
113+
Ok(mut entries) => entries.next().is_none(),
114+
// Non-existent destinations will be created by init_opts.
115+
Err(err) if err.kind() == std::io::ErrorKind::NotFound => true,
116+
// If we can't verify emptiness, keep cleanup conservative and leave the destination untouched.
117+
Err(_) => false,
118+
};
119+
106120
let mut repo = crate::ThreadSafeRepository::init_opts(path, kind, create_opts, open_opts)?.to_thread_local();
107121
url.canonicalize(repo.options.current_dir_or_empty())
108122
.map_err(|err| Error::CanonicalizeUrl {
@@ -122,6 +136,7 @@ impl PrepareFetch {
122136
configure_connection: None,
123137
shallow: remote::fetch::Shallow::NoChange,
124138
ref_name: None,
139+
remove_worktree_on_drop,
125140
})
126141
}
127142
}
@@ -136,6 +151,21 @@ pub struct PrepareCheckout {
136151
pub(self) repo: Option<crate::Repository>,
137152
/// The name of the reference to check out. If `None`, the reference pointed to by `HEAD` will be checked out.
138153
pub(self) ref_name: Option<gix_ref::PartialName>,
154+
/// If `true`, drop removes the entire worktree. Otherwise leave it alone.
155+
pub(self) remove_worktree_on_drop: bool,
156+
}
157+
158+
fn cleanup_clone_destination_on_drop(repo: &crate::Repository, remove_worktree_on_drop: bool) {
159+
let path_to_remove = if remove_worktree_on_drop {
160+
Some(repo.workdir().unwrap_or_else(|| repo.path()))
161+
} else {
162+
// The destination held pre-existing user files. Leave everything, including the `.git` we created,
163+
// so the user can inspect or clean up the partially cloned repository with Git tooling.
164+
None
165+
};
166+
if let Some(path_to_remove) = path_to_remove {
167+
std::fs::remove_dir_all(path_to_remove).ok();
168+
}
139169
}
140170

141171
// This module encapsulates functionality that works with both feature toggles. Can be combined with `fetch`

‎gix/src/create.rs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,23 @@ fn create_dir(p: &Path) -> Result<(), Error> {
108108
}
109109

110110
/// Options for use in [`into()`];
111-
#[derive(Copy, Clone, Default)]
111+
#[derive(Copy, Default, Clone)]
112112
pub struct Options {
113-
/// If true, and the kind of repository to create has a worktree, then the destination directory must be empty.
113+
/// Control whether the destination directory must be empty when creating a repository with a worktree.
114114
///
115-
/// By default repos with worktree can be initialized into a non-empty repository as long as there is no `.git` directory.
116-
pub destination_must_be_empty: bool,
115+
/// - `None` (default): initialize like Git and allow a non-empty destination directory, as long as no `.git`
116+
/// directory is present.
117+
/// - `Some(true)`: require an empty destination directory.
118+
/// - `Some(false)`: explicitly allow initialization into a non-empty destination directory (still requires that no
119+
/// `.git` directory is present).
120+
///
121+
/// For clones, checkout failure cleanup is based on whether the destination was already present and non-empty before
122+
/// initialization began, not on this option alone. In particular, if the destination was empty or had to be created,
123+
/// cleanup may remove the entire destination, including the created `.git` directory. Preservation of the destination
124+
/// for inspection or manual cleanup is only guaranteed when the destination was non-empty before the clone started.
125+
///
126+
/// Bare repositories always require an empty destination, regardless of this option.
127+
pub destination_must_be_empty: Option<bool>,
117128
/// If set, use these filesystem capabilities to populate the respective git-config fields.
118129
/// If `None`, the directory will be probed.
119130
pub fs_capabilities: Option<gix_fs::Capabilities>,
@@ -140,7 +151,7 @@ pub fn into(
140151
let mut dot_git = directory.into();
141152
let bare = matches!(kind, Kind::Bare);
142153

143-
if bare || destination_must_be_empty {
154+
if bare || destination_must_be_empty.unwrap_or(false) {
144155
let num_entries_in_dot_git = fs::read_dir(&dot_git)
145156
.or_else(|err| {
146157
if err.kind() == std::io::ErrorKind::NotFound {

‎gix/src/init.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ pub enum Error {
4242
impl ThreadSafeRepository {
4343
/// Create a repository with work-tree within `directory`, creating intermediate directories as needed.
4444
///
45-
/// Fails without action if there is already a `.git` repository inside of `directory`, but
46-
/// won't mind if the `directory` otherwise is non-empty.
45+
/// Fails without action if the destination directory isn't empty unless
46+
/// [`create::Options::destination_must_be_empty`][crate::create::Options::destination_must_be_empty] is `None`
47+
/// or `Some(false)`. Note that initialization still fails if a `.git` directory already exists in
48+
/// the destination.
4749
pub fn init(
4850
directory: impl AsRef<Path>,
4951
kind: crate::create::Kind,

‎gix/tests/fixtures/generated-archives/make_clone_destinations.tar‎

10 KB
Binary file not shown.

‎gix/tests/fixtures/make_clone_destinations.sh‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#!/usr/bin/env bash
2+
set -eu -o pipefail
3+
4+
mkdir non-empty
5+
printf "Pre-existing user content.\n" > non-empty/existing.txt
6+
7+
mkdir non-empty-with-conflicting-file
8+
printf "Pre-existing user content.\n" > non-empty-with-conflicting-file/file
9+
10+
cp -R non-empty non-empty-with-dot-git
11+
mkdir non-empty-with-dot-git/.git
12+
printf "ref: refs/heads/pre-existing\n" > non-empty-with-dot-git/.git/HEAD

‎gix/tests/gix/clone.rs‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ mod blocking_io {
2020
use gix_ref::TargetRef;
2121
use gix_refspec::parse::Operation;
2222

23+
const EXISTING_CONTENT: &[u8] = b"Pre-existing user content.\n";
24+
const EXISTING_HEAD_CONTENT: &[u8] = b"ref: refs/heads/pre-existing\n";
25+
2326
fn shallow_ids(repo: &gix::Repository, expected: &'static str) -> crate::Result<Vec<gix::ObjectId>> {
2427
let commits = repo.shallow_commits()?.expect(expected);
2528
Ok(std::iter::once(commits.head)
@@ -585,6 +588,144 @@ mod blocking_io {
585588
Ok(())
586589
}
587590

591+
#[test]
592+
fn fetch_and_checkout_into_non_empty_directory() -> crate::Result {
593+
let fixture = gix_testtools::scripted_fixture_writable("make_clone_destinations.sh")?;
594+
let destination = fixture.path().join("non-empty");
595+
let existing_path = destination.join("existing.txt");
596+
597+
let mut prepare = gix::clone::PrepareFetch::new(
598+
remote::repo("base").path(),
599+
&destination,
600+
gix::create::Kind::WithWorktree,
601+
gix::create::Options {
602+
destination_must_be_empty: Some(false),
603+
..Default::default()
604+
},
605+
restricted(),
606+
)?;
607+
let (mut checkout, _out) =
608+
prepare.fetch_then_checkout(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?;
609+
let (repo, _) = checkout.main_worktree(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?;
610+
611+
let index = repo.index()?;
612+
assert_eq!(index.entries().len(), 1, "All entries are known as per HEAD tree");
613+
assure_index_entries_on_disk(&index, repo.workdir().expect("non-bare"));
614+
615+
assert_eq!(std::fs::read(&existing_path)?, EXISTING_CONTENT);
616+
Ok(())
617+
}
618+
619+
#[test]
620+
fn fetch_and_checkout_into_non_empty_directory_does_not_overwrite_pre_existing_tracked_file() -> crate::Result {
621+
let fixture = gix_testtools::scripted_fixture_writable("make_clone_destinations.sh")?;
622+
let destination = fixture.path().join("non-empty-with-conflicting-file");
623+
let existing_path = destination.join("file");
624+
let remote_file_content = std::fs::read(remote::repo("base").workdir().expect("non-bare").join("file"))?;
625+
assert_ne!(
626+
EXISTING_CONTENT, remote_file_content,
627+
"the fixture must differ from the file that checkout would write"
628+
);
629+
630+
let mut prepare = gix::clone::PrepareFetch::new(
631+
remote::repo("base").path(),
632+
&destination,
633+
gix::create::Kind::WithWorktree,
634+
gix::create::Options {
635+
destination_must_be_empty: Some(false),
636+
..Default::default()
637+
},
638+
restricted(),
639+
)?;
640+
let (mut checkout, _out) = prepare.fetch_then_checkout(gix::progress::Discard, &AtomicBool::default())?;
641+
let (repo, outcome) = checkout.main_worktree(gix::progress::Discard, &AtomicBool::default())?;
642+
643+
assert_eq!(
644+
std::fs::read(&existing_path)?,
645+
EXISTING_CONTENT,
646+
"checkout must not overwrite the pre-existing tracked path"
647+
);
648+
assert_eq!(repo.index()?.entries().len(), 1, "the index is still written");
649+
assert_eq!(
650+
outcome.collisions,
651+
[gix_worktree_state::checkout::Collision {
652+
path: BString::from("file"),
653+
error_kind: std::io::ErrorKind::AlreadyExists
654+
}],
655+
"the pre-existing tracked path is reported as a normal checkout collision"
656+
);
657+
Ok(())
658+
}
659+
660+
#[test]
661+
fn fetch_and_checkout_into_non_empty_directory_with_existing_dot_git_is_rejected() -> crate::Result {
662+
let fixture = gix_testtools::scripted_fixture_writable("make_clone_destinations.sh")?;
663+
let destination = fixture.path().join("non-empty-with-dot-git");
664+
let existing_path = destination.join("existing.txt");
665+
let dot_git = destination.join(".git");
666+
let head_path = dot_git.join("HEAD");
667+
668+
let err = gix::clone::PrepareFetch::new(
669+
remote::repo("base").path(),
670+
&destination,
671+
gix::create::Kind::WithWorktree,
672+
gix::create::Options {
673+
destination_must_be_empty: Some(false),
674+
..Default::default()
675+
},
676+
restricted(),
677+
)
678+
.map(drop)
679+
.expect_err("an existing .git directory must not be reused for clone");
680+
681+
assert!(
682+
matches!(
683+
err,
684+
gix::clone::Error::Init(gix::init::Error::Init(gix::create::Error::DirectoryExists { ref path }))
685+
if *path == dot_git
686+
),
687+
"unexpected error: {err}"
688+
);
689+
assert_eq!(std::fs::read(&existing_path)?, EXISTING_CONTENT);
690+
assert_eq!(std::fs::read(&head_path)?, EXISTING_HEAD_CONTENT);
691+
Ok(())
692+
}
693+
694+
#[test]
695+
fn drop_after_failed_fetch_into_non_empty_directory_preserves_destination() -> crate::Result {
696+
let fixture = gix_testtools::scripted_fixture_writable("make_clone_destinations.sh")?;
697+
let destination = fixture.path().join("non-empty");
698+
let existing_path = destination.join("existing.txt");
699+
700+
let mut prepare = gix::clone::PrepareFetch::new(
701+
remote::repo("base").path(),
702+
&destination,
703+
gix::create::Kind::WithWorktree,
704+
gix::create::Options {
705+
destination_must_be_empty: Some(false),
706+
..Default::default()
707+
},
708+
restricted(),
709+
)?
710+
.with_ref_name(Some("does-not-exist"))?;
711+
712+
prepare
713+
.fetch_then_checkout(gix::progress::Discard, &AtomicBool::default())
714+
.expect_err("non-existing ref must fail");
715+
drop(prepare);
716+
717+
assert_eq!(
718+
std::fs::read(&existing_path)?,
719+
EXISTING_CONTENT,
720+
"pre-existing user files must survive a failed clone+drop"
721+
);
722+
assert!(
723+
destination.join(".git").is_dir(),
724+
"the .git directory we created should remain for user cleanup"
725+
);
726+
Ok(())
727+
}
728+
588729
#[test]
589730
fn fetch_and_checkout_specific_ref() -> crate::Result {
590731
let tmp = gix_testtools::tempfile::TempDir::new()?;
@@ -832,6 +973,26 @@ fn clone_and_destination_must_be_empty() -> crate::Result {
832973
Ok(())
833974
}
834975

976+
#[test]
977+
fn clone_with_worktree_and_destination_must_be_empty() -> crate::Result {
978+
let fixture = gix_testtools::scripted_fixture_writable("make_clone_destinations.sh")?;
979+
let destination = fixture.path().join("non-empty");
980+
let err = gix::clone::PrepareFetch::new(
981+
remote::repo("base").path(),
982+
&destination,
983+
gix::create::Kind::WithWorktree,
984+
Default::default(),
985+
restricted(),
986+
)
987+
.map(drop)
988+
.expect_err("this should fail as the directory isn't empty");
989+
assert!(
990+
err.to_string()
991+
.starts_with("Refusing to initialize the non-empty directory as ")
992+
);
993+
Ok(())
994+
}
995+
835996
#[test]
836997
fn clone_bare_into_empty_directory_and_early_drop() -> crate::Result {
837998
let tmp = gix_testtools::tempfile::TempDir::new()?;

‎gix/tests/gix/init.rs‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,40 @@ mod non_bare {
164164
}
165165

166166
#[test]
167-
fn init_into_non_empty_directory_is_not_allowed_if_option_is_set_as_used_for_clone() -> crate::Result {
167+
fn init_into_non_empty_directory_is_allowed_if_option_is_none_or_false() -> crate::Result {
168+
for destination_must_be_empty in [None, Some(false)] {
169+
let tmp = tempfile::tempdir()?;
170+
std::fs::write(tmp.path().join("existing.txt"), b"I was here before you")?;
171+
let repo: gix::Repository = gix::ThreadSafeRepository::init_opts(
172+
tmp.path(),
173+
gix::create::Kind::WithWorktree,
174+
gix::create::Options {
175+
destination_must_be_empty,
176+
..Default::default()
177+
},
178+
gix::open::Options::isolated(),
179+
)?
180+
.into();
181+
assert_eq!(repo.workdir().expect("present"), tmp.path());
182+
assert_eq!(
183+
repo.git_dir(),
184+
tmp.path().join(".git"),
185+
"gitdir is inside of the workdir"
186+
);
187+
}
188+
Ok(())
189+
}
190+
191+
#[test]
192+
fn init_into_non_empty_directory_is_not_allowed_if_option_is_true() -> crate::Result {
168193
let tmp = tempfile::tempdir()?;
169194
std::fs::write(tmp.path().join("existing.txt"), b"I was here before you")?;
170195

171196
let err = gix::ThreadSafeRepository::init_opts(
172197
tmp.path(),
173198
gix::create::Kind::WithWorktree,
174199
gix::create::Options {
175-
destination_must_be_empty: true,
200+
destination_must_be_empty: Some(true),
176201
..Default::default()
177202
},
178203
gix::open::Options::isolated(),
@@ -184,19 +209,4 @@ mod non_bare {
184209
);
185210
Ok(())
186211
}
187-
188-
#[test]
189-
fn init_into_non_empty_directory_is_allowed_by_default() -> crate::Result {
190-
let tmp = tempfile::tempdir()?;
191-
std::fs::write(tmp.path().join("existing.txt"), b"I was here before you")?;
192-
193-
let repo = gix::init(tmp.path())?;
194-
assert_eq!(repo.workdir().expect("present"), tmp.path());
195-
assert_eq!(
196-
repo.git_dir(),
197-
tmp.path().join(".git"),
198-
"gitdir is inside of the workdir"
199-
);
200-
Ok(())
201-
}
202212
}

0 commit comments

Comments
 (0)