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

Outdated file information for recreated directories #1753

Closed
valoq opened this issue Jun 21, 2024 · 9 comments
Closed

Outdated file information for recreated directories #1753

valoq opened this issue Jun 21, 2024 · 9 comments

Comments

@valoq
Copy link
Contributor

valoq commented Jun 21, 2024

When a directory with e.g 5 files inside is deleted and then a directory with the same name recreated with 10 files inside, lf will show the directory with only 5 files, indicating the cached information for the deleted directory are reused.

This can be reproduced with an archive:

  1. extract A.zip containing the directory A with 10 files inside.
  2. delete the files inside A (lf now shows A as empty)
  3. delete A
  4. extract A.zip again, lf will show the directory A as empty even though there are 10 files inside (until manual refresh)

I suspect there was a similar (closed?) issue that was assumed to be fixed but I was unable to find it just now.

There have been several occasions where this bug felt quite dangerous to me as a directory that is shown as empty may be deleted without much though, thereby unintentionally deleting possibly important files.

@joelim-work
Copy link
Collaborator

This is probably related to #1426. Unlike newly created files/directories, extracted files/directories typically retain their original mtime from the archive, which undermines the reloading mechanism (checking for updated mtime values) in lf.

But I found a way to work around this using the watch option, by invalidating cache entries from remove/rename updates. Can you try #1756 to see if it works?

@valoq
Copy link
Contributor Author

valoq commented Jun 22, 2024

Now the preview will always show the directory as empty instead.

  1. extract A.zip containing the directory A with 10 files inside.
  2. delete 5 of files inside A
  3. delete A
  4. extract A.zip again
  5. lf will show the directory A as empty even though there are 10 files inside and the previous version still had 5 files.

The dircounts shows the correct number of files in the new directory though.

@joelim-work
Copy link
Collaborator

Oh I fixed a bug just now in 7292788, but I think you commented before I pushed it. Can you try again?

@valoq
Copy link
Contributor Author

valoq commented Jun 22, 2024

Still the same issue. The newly extracted directory is shown as empty

@joelim-work
Copy link
Collaborator

It was working for me, but I think there's maybe one or two times where the recreated directory doesn't load for some reason and shows up as failed. I guess maybe it's not consistent, but I can't figure out why though.

@valoq
Copy link
Contributor Author

valoq commented Jun 22, 2024

I tested this again on a fresh install and for me the bug is always present.

Given a default lfrc file, the bug would not appear at all since there is no automatic file refresh, so the question may be related to how the directory is updated.

There are three possible ways to trigger the bug that I found so far:

  1. Use an extract* command that will make the newly created directory appear (with the outdated info)
  2. use the period option
  3. use the watch option

*Command I tested this with is as follows:

cmd extract ${{
    set -f
    atool -x $f
}}
map a extract

@joelim-work
Can you reproduce the bug with your default config and the mainline branch? If so, can you share your lfrc?

@joelim-work
Copy link
Collaborator

It's not a problem with updating the directory contents, it's a problem with stale entries in the directory cache. An entry will only be replaced if the corresponding directory has a later mtime than its own, and this won't be the case for directories extracted from archives. This means that the original load and period mechanisms aren't useful here. But for watch updates, cache entries can be invalidated when a remove/rename notification is received.

Anyway I ended up merging that PR after a couple more changes - it's probably a good idea to disable the load command when watch is enabled, as they are essentially competing mechanisms and seem to cause weird issues when enabled together. So now this is working for me, and I can see the updated file list when re-extracting directories.

@valoq
Copy link
Contributor Author

valoq commented Jun 23, 2024

This fixed the issue for when the watch option is used. Thank you.

Extracting an archive will still show the outdated directory with the default lf options, though there is also no default extract command either.
One workaround I found is to modify the extract command like this:

cmd extract ${{
    set -f
    atool -x $f
}}

map a : extract; reload

That could be added to the wiki in order to help users without automated file refresh avoid the issue as well.

@joelim-work
Copy link
Collaborator

The "default lf options" you're referring to just simply uses the load command I mentioned earlier. It's an internal command sent after various operations (e.g. delete, paste) to update the files (subject to mtime values being increased).

So the trick of adding reload at the end of such commands technically works, and in fact it's already been suggested in various issues before so it's nothing new. But it's also kind of a hacky workaround and I'm not sure I want to recommend such a solution in the wiki. Maybe the answer is to just encourage more users to use watch instead.

Anyway I will close this issue since it's solved now. Thanks for reporting it.

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

No branches or pull requests

2 participants