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

util: Add rename fallback for copying across devices #364

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

richiejp
Copy link
Contributor

@richiejp richiejp commented Mar 3, 2025

If a file is being moved across devices then rename() won't work. The
file needs to be copied instead.

/tmp and /home are on separate drives on one of my machines. I can't save the config and load extensions without on that machine without this.

@richiejp richiejp force-pushed the replace-file-fallback branch from afe530b to 1696511 Compare March 3, 2025 15:50
@liquidaty
Copy link
Owner

Thank you, this is helpful. Would like to make a few further changes-- if you'd like to do so please do, otherwise lmk and I will:

  1. change fopen read/write to binary ("rb" and "wb")
  2. Is there any easy way to add a test? Am guessing no, since it requires a separate physical device. If that is the case (it's not easy), will add an issue to track that missing test
  3. on fail, return errno instead of always returning -1

@richiejp richiejp force-pushed the replace-file-fallback branch from 1696511 to 1d5d60c Compare March 4, 2025 12:04
@richiejp
Copy link
Contributor Author

richiejp commented Mar 4, 2025

Is there any easy way to add a test? Am guessing no, since it requires a separate physical device. If that is the case (it's not easy), will add an issue to track that missing test

I think it should be possible on Linux by mounting a filesystem with FUSE or tmpfs. Apparently Github's runners already have multiple file systems mounted that we have read/write access to. However I also think it could be a bit of time sink.

The rest of the changes I made.

@liquidaty
Copy link
Owner

Great, thank you. Assuming the file could be binary, any reason to not use fread/fwrite over fgets/fputs?

If a file is being moved across devices then rename() won't work. The
file needs to be copied instead.
@richiejp richiejp force-pushed the replace-file-fallback branch from 1d5d60c to c94d24f Compare March 5, 2025 10:05
@richiejp
Copy link
Contributor Author

richiejp commented Mar 5, 2025

No reason, I have changed it.

@liquidaty liquidaty merged commit 0560845 into liquidaty:main Mar 10, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants