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

File size displayed incorrectly after copy #1417

Closed
miki2119 opened this issue Sep 10, 2023 · 6 comments · Fixed by #1638
Closed

File size displayed incorrectly after copy #1417

miki2119 opened this issue Sep 10, 2023 · 6 comments · Fixed by #1638

Comments

@miki2119
Copy link

When copying multiple large files to a different directory, lf incorrectly lists the file size of the last copied file.

Using the option "set period 1" for autorefresh

When copying file a and b (both 500MB) to a new directory, lf will first show file a with a random size between 0 and 500.
When multiple lf instance are showing the same target directory, the listed sizes even differ between lf instances.

After file a has finished copying, it is correctly displayed with the correct size, however after file b has finished copying, the correct size is not show and instead a random file size value is listed.

This can probably be fixed by refreshing the file stats after the copy job has finished.

@joelim-work
Copy link
Collaborator

I did some investigation, it looks like the random size comes from the period timer 'capturing' the size when the file is being written to. Both the period timer and the paste command internally triggers the load command to refresh the information, however load only performs an actual refresh if the mtime of the directory changes.

I was able to trigger the same problem by copying a single file that was very large (10G). The sequence of events looks like this:

  1. A very large file is copied and pasted. This is done by opening a new file, and writing the contents of the original file to it.
  2. Opening a new file modifies the containing directory.
  3. The period timer triggers, causing the file sizes to be refreshed. However at this point the copy operation is still in progress and hasn't finished writing to the file, resulting in an incomplete size.
  4. The copy operation finishes, and triggers the load command.
  5. At this point the directory has not been modified again since step 2, since no new file was created. Therefore the file size won't be refreshed.

This seems to be an extension of the problem where the file size doesn't update when writing to an already existing file (e.g. try running the commands $touch foo.txt followed by $date > foo.txt), in which case a manual reload is required. This is in a way similar to #776

@miki2119
Copy link
Author

If I understand this correctly, the core of the issue is located here: https://github.com/gokcehan/lf/blob/master/nav.go#L517

There seem to be quite a few issues are related to this as well, like #1172 #776 #1226

Any changes to files that do not trigger an updated mtime stat will be ignored until a manual reload, which is most noticeable when using the period option, but also affect issues like this one.

There are some cases where executing "reload" at certain functions would work but ideally any changes to files that were done outside of lf would also be updated.

The question is how this can be done efficiently. Other file managers may already have a solution that could also work for lf.

@miki2119
Copy link
Author

Would it be easily possible to use ctime instead of mtime to trigger the update in the checkDir function? If so, I think that would solve all of these issues

@joelim-work
Copy link
Collaborator

That line of code you pointed out is indeed where the mtime check happens, but anyway the main problem is that writing to a file does not modify the containing directory. Checking the ctime of the directory won't help in this case either.

I guess it would be possible to fix this by introducing some kind of forceload command which would forcefully reload a directory regardless of its mtime, I'm not 100% sure about committing to this decision though, maybe it's better to wait and see if anyone else has any better ideas.

@miki2119
Copy link
Author

miki2119 commented Sep 25, 2023

That line of code you pointed out is indeed where the mtime check happens, but anyway the main problem is that writing to a file does not modify the containing directory. Checking the ctime of the directory won't help in this case either.

You are right and changing the check to ctime will probably not fix this issue, but I am pretty sure many of the related/linked issues would be solved, leaving us with a more simple solution for this one like a single forced check after copy operations.

edit:
Unfortunately it seems to be more difficult then I thought.
I changed the mentioned line from os.Stat to times.Stat to use the ChangeTime() function since we already use that package anyway, but the only solution for these issues seems to be to check the timing of every single file instead of just the directory. That would probably take too much execution time?

Maybe anyone can tell how other file managers do this?

@joelim-work
Copy link
Collaborator

I tried with ranger, when copying a 10G file, it initially creates an entry with some random incomplete file size, and when the copy finishes, the size is updated to correctly show 10G.

I had a very quick look at the code, it looks like the copy operation does a hard refresh of the directory when it finishes: https://github.com/ranger/ranger/blob/136416c7e2ecc27315fe2354ecadfe09202df7dd/ranger/core/loader.py#L146-L147

Though I have never really worked with ranger before, you may want to check with their devs on this, or spend some time playing around with it yourself.

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 a pull request may close this issue.

2 participants