Skip to content

Commit

Permalink
Convert creates tempdir in parent of destination (#99)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
lordkekz and 9999years authored Nov 18, 2024
1 parent eddd725 commit 78b0f36
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 6 deletions.
8 changes: 8 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ reason = "Use git_prole::fs::read_to_string"
path = "std::fs::read_to_string"
reason = "Use git_prole::fs::read_to_string"

[[disallowed-methods]]
path = "fs_err::read_dir"
reason = "Use git_prole::fs::read_dir"

[[disallowed-methods]]
path = "std::fs::read_dir"
reason = "Use git_prole::fs::read_dir"

[[disallowed-methods]]
path = "fs_err::copy"
reason = "Use git_prole::fs::copy"
Expand Down
37 changes: 36 additions & 1 deletion src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ where
// suffix removed
// - Otherwise just use the git dir path.

let tempdir = Utf8TempDir::new()?.into_path();
// Tests:
// - `convert_from_bare`
// - `convert_bare_dot_git`
Expand All @@ -224,8 +223,14 @@ where
let destination_name = destination
.file_name()
.ok_or_else(|| miette!("Destination has no basename: {destination}"))?;
let destination_parent = destination
.parent()
.ok_or_else(|| miette!("Destination has no parent: {destination}"))?
.to_path_buf();
tracing::debug!(%destination, "Destination determined");

let tempdir = Utf8TempDir::new(&destination_parent)?.into_path();

let default_branch = match opts.default_branch {
// Tests:
// - `convert_explicit_default_branch`
Expand Down Expand Up @@ -550,6 +555,8 @@ where
);
tracing::info!("You may need to `cd .` to refresh your shell");

remove_tempdir_if_empty(&self.tempdir)?;

Ok(())
}

Expand Down Expand Up @@ -663,3 +670,31 @@ impl MainWorktreePlan {
self.inner.destination(convert_plan).join(".git")
}
}

fn remove_tempdir_if_empty(tempdir: &Utf8Path) -> miette::Result<()> {
let contents = fs::read_dir(tempdir)?.collect::<Vec<_>>();
// From `std::fs::read_dir` documentation:
// > Entries for the current and parent directories (typically . and ..) are skipped.
if !contents.is_empty() {
tracing::warn!(
"Temporary directory isn't empty: {}\n{}",
tempdir.display_path_cwd(),
display_dir_contents(&contents)
);
} else {
fs::remove_dir(tempdir)?;
}

Ok(())
}

fn display_dir_contents(contents: &[std::io::Result<fs_err::DirEntry>]) -> String {
format_bulleted_list(contents.iter().map(|item| {
match item {
Ok(entry) => entry.file_name().display_path_cwd(),
Err(error) => error
.if_supports_color(Stream::Stdout, |text| text.red())
.to_string(),
}
}))
}
10 changes: 10 additions & 0 deletions src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::fmt::Debug;
use std::path::Path;
use std::path::PathBuf;

use miette::IntoDiagnostic;
use tracing::instrument;
Expand Down Expand Up @@ -72,3 +73,12 @@ where
#[expect(clippy::disallowed_methods)]
fs_err::write(path, contents).into_diagnostic()
}

#[instrument(level = "trace")]
pub fn read_dir<P>(path: P) -> miette::Result<fs_err::ReadDir>
where
P: Into<PathBuf> + Debug,
{
#[expect(clippy::disallowed_methods)]
fs_err::read_dir(path).into_diagnostic()
}
7 changes: 5 additions & 2 deletions src/path_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ pub trait PathDisplay: Debug + AsRef<Path> {

impl<P> PathDisplay for P
where
P: AsRef<Utf8Path> + AsRef<Path> + Debug,
P: AsRef<Path> + Debug,
{
fn display_path_from(&self, base: impl AsRef<Utf8Path>) -> String {
try_display(self, base).unwrap_or_else(|| display_backup(self))
let path = self.as_ref();
Utf8Path::from_path(path)
.and_then(|utf8path| try_display(utf8path, base))
.unwrap_or_else(|| display_backup(self))
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/utf8tempdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ pub struct Utf8TempDir {
}

impl Utf8TempDir {
pub fn new() -> miette::Result<Self> {
let inner = tempfile::tempdir().into_diagnostic()?;
pub fn new(parent_dir: &Utf8PathBuf) -> miette::Result<Self> {
let inner = tempfile::tempdir_in(parent_dir).into_diagnostic()?;
let path = inner.path().to_owned().try_into().into_diagnostic()?;
Ok(Self {
inner: Some(inner),
Expand Down
2 changes: 1 addition & 1 deletion test-harness/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub struct GitProle {

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

if std::env::var("KEEP_TEMP").is_ok() {
tempdir.persist();
Expand Down

0 comments on commit 78b0f36

Please sign in to comment.