Skip to content

Commit 78b0f36

Browse files
lordkekz9999years
andauthored
Convert creates tempdir in parent of destination (#99)
Fixes issue #96 I tested the change on some repos (no longer gives the error) and ran the tests with `nix flake check`. Please do review this carefully since I'm still pretty new to Rust. ^^ ### Change summary `git prole convert` now creates a tempdir in the parent of the determined destination of the converted repo. This prevents some errors when the /tmp directory is on a different filesystem, such as a tmpfs. The tempdir will be deleted if and only if it is empty after conversion. This should always be the case and is to prevent any data loss. --------- Co-authored-by: Rebecca Turner <[email protected]>
1 parent eddd725 commit 78b0f36

File tree

6 files changed

+62
-6
lines changed

6 files changed

+62
-6
lines changed

clippy.toml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ reason = "Use git_prole::fs::read_to_string"
6262
path = "std::fs::read_to_string"
6363
reason = "Use git_prole::fs::read_to_string"
6464

65+
[[disallowed-methods]]
66+
path = "fs_err::read_dir"
67+
reason = "Use git_prole::fs::read_dir"
68+
69+
[[disallowed-methods]]
70+
path = "std::fs::read_dir"
71+
reason = "Use git_prole::fs::read_dir"
72+
6573
[[disallowed-methods]]
6674
path = "fs_err::copy"
6775
reason = "Use git_prole::fs::copy"

src/convert.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ where
211211
// suffix removed
212212
// - Otherwise just use the git dir path.
213213

214-
let tempdir = Utf8TempDir::new()?.into_path();
215214
// Tests:
216215
// - `convert_from_bare`
217216
// - `convert_bare_dot_git`
@@ -224,8 +223,14 @@ where
224223
let destination_name = destination
225224
.file_name()
226225
.ok_or_else(|| miette!("Destination has no basename: {destination}"))?;
226+
let destination_parent = destination
227+
.parent()
228+
.ok_or_else(|| miette!("Destination has no parent: {destination}"))?
229+
.to_path_buf();
227230
tracing::debug!(%destination, "Destination determined");
228231

232+
let tempdir = Utf8TempDir::new(&destination_parent)?.into_path();
233+
229234
let default_branch = match opts.default_branch {
230235
// Tests:
231236
// - `convert_explicit_default_branch`
@@ -550,6 +555,8 @@ where
550555
);
551556
tracing::info!("You may need to `cd .` to refresh your shell");
552557

558+
remove_tempdir_if_empty(&self.tempdir)?;
559+
553560
Ok(())
554561
}
555562

@@ -663,3 +670,31 @@ impl MainWorktreePlan {
663670
self.inner.destination(convert_plan).join(".git")
664671
}
665672
}
673+
674+
fn remove_tempdir_if_empty(tempdir: &Utf8Path) -> miette::Result<()> {
675+
let contents = fs::read_dir(tempdir)?.collect::<Vec<_>>();
676+
// From `std::fs::read_dir` documentation:
677+
// > Entries for the current and parent directories (typically . and ..) are skipped.
678+
if !contents.is_empty() {
679+
tracing::warn!(
680+
"Temporary directory isn't empty: {}\n{}",
681+
tempdir.display_path_cwd(),
682+
display_dir_contents(&contents)
683+
);
684+
} else {
685+
fs::remove_dir(tempdir)?;
686+
}
687+
688+
Ok(())
689+
}
690+
691+
fn display_dir_contents(contents: &[std::io::Result<fs_err::DirEntry>]) -> String {
692+
format_bulleted_list(contents.iter().map(|item| {
693+
match item {
694+
Ok(entry) => entry.file_name().display_path_cwd(),
695+
Err(error) => error
696+
.if_supports_color(Stream::Stdout, |text| text.red())
697+
.to_string(),
698+
}
699+
}))
700+
}

src/fs.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
44
use std::fmt::Debug;
55
use std::path::Path;
6+
use std::path::PathBuf;
67

78
use miette::IntoDiagnostic;
89
use tracing::instrument;
@@ -72,3 +73,12 @@ where
7273
#[expect(clippy::disallowed_methods)]
7374
fs_err::write(path, contents).into_diagnostic()
7475
}
76+
77+
#[instrument(level = "trace")]
78+
pub fn read_dir<P>(path: P) -> miette::Result<fs_err::ReadDir>
79+
where
80+
P: Into<PathBuf> + Debug,
81+
{
82+
#[expect(clippy::disallowed_methods)]
83+
fs_err::read_dir(path).into_diagnostic()
84+
}

src/path_display.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@ pub trait PathDisplay: Debug + AsRef<Path> {
3030

3131
impl<P> PathDisplay for P
3232
where
33-
P: AsRef<Utf8Path> + AsRef<Path> + Debug,
33+
P: AsRef<Path> + Debug,
3434
{
3535
fn display_path_from(&self, base: impl AsRef<Utf8Path>) -> String {
36-
try_display(self, base).unwrap_or_else(|| display_backup(self))
36+
let path = self.as_ref();
37+
Utf8Path::from_path(path)
38+
.and_then(|utf8path| try_display(utf8path, base))
39+
.unwrap_or_else(|| display_backup(self))
3740
}
3841
}
3942

src/utf8tempdir.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ pub struct Utf8TempDir {
1414
}
1515

1616
impl Utf8TempDir {
17-
pub fn new() -> miette::Result<Self> {
18-
let inner = tempfile::tempdir().into_diagnostic()?;
17+
pub fn new(parent_dir: &Utf8PathBuf) -> miette::Result<Self> {
18+
let inner = tempfile::tempdir_in(parent_dir).into_diagnostic()?;
1919
let path = inner.path().to_owned().try_into().into_diagnostic()?;
2020
Ok(Self {
2121
inner: Some(inner),

test-harness/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ pub struct GitProle {
2929

3030
impl GitProle {
3131
pub fn new() -> miette::Result<Self> {
32-
let mut tempdir = Utf8TempDir::new()?;
32+
let mut tempdir = Utf8TempDir::new(&Utf8PathBuf::from("./"))?;
3333

3434
if std::env::var("KEEP_TEMP").is_ok() {
3535
tempdir.persist();

0 commit comments

Comments
 (0)