Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert creates tempdir in parent of destination #99

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

lordkekz
Copy link
Contributor

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.

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

Fixes issue 9999years#96
@github-actions github-actions bot added the patch label Oct 28, 2024
@9999years
Copy link
Owner

Thanks for the contribution!

* `fs::read_dir` skips entries for the current and parent directories, so
  we want it to be completely empty to delete the temporary directory.

* Add a local `fs::read_dir` wrapper for `fs_err::read_dir` to match the
  others in that module.

* Show a warning, not an info message, if the tempdir isn't empty.

* Include the paths in the tempdir in the warning message.
@9999years
Copy link
Owner

Looks good! I improved the error message a bit and added some wrappers to match the rest of the codebase, so I'll merge it after tests

@9999years 9999years merged commit 78b0f36 into 9999years:main Nov 18, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants