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

Recreating a folder with the same name shows files from the "old" folder #1426

Closed
karabaja4 opened this issue Sep 16, 2023 · 8 comments
Closed

Comments

@karabaja4
Copy link

karabaja4 commented Sep 16, 2023

Say I have two archives and they both contain a folder with the same name, e.g. "etc", but with different files inside the folder.

Extract the first archive, go into the "etc" folder, then go back and delete the "etc" folder.

Then extract the second archive and go into the "etc" folder again, and you will notice that it does not display the files from the second archive, but from the first one.

Calling "reload" while inside the folder fixes things and the files from the second archive appear.

The explanation is a bit clumsy so I made a video:
https://www.youtube.com/watch?v=rrr2im5I5N0

Regards,

Igor

EDIT: Setting set dircache false fixes this... so I assume it has something to do with how the cache is implemented...

@joelim-work
Copy link
Collaborator

I thought this kind of issue was fixed in #1292, what version are you running?

There hasn't been any new release containing this bug fix yet, but it is coming shortly (see #1407). You can also build directly from source if you don't want to wait.

@karabaja4
Copy link
Author

karabaja4 commented Sep 16, 2023

Nope, I've been actually running git version with #1292 fixed (I provided you with the reproduce steps for that one). This is a different bug.

I just cloned latest git HEAD and the behavior described here is still present. You can try to reproduce it like I did in the video, here are the files:

https://archlinux.org/packages/core/x86_64/pam/download/
https://archlinux.org/packages/core/any/pambase/download/

It's like lf caches the contents of the folder based on path alone when set dircache true is set, and then displays the old set of files when the folder actually contains a new set of files.

@joelim-work
Copy link
Collaborator

joelim-work commented Sep 16, 2023

Thanks for clarifying.

Directory caching is based on mtime, to avoid unnecessarily reloading directories. What this means is that lf will only reload a directory if its mtime is later than the last time it was loaded (i.e. 'has changed since').

In your example, when the first archive is extracted, etc does not exist in the cache so it is loaded properly, and the last load time is set to the current time. When the second archive is extracted, the (preserved) mtime of its etc directory is earlier than the last load time, so it will not be reloaded.

I think your best options are to either extract without preserving mtimes, or add a lf -remote "send $id reload" call at the end of your extract function.

P.S. You are right about the implementation of the directory cache, it is simply a mapping from a string path to a data structure representing a directory:

lf/nav.go

Line 444 in ffc756c

dirCache map[string]*dir

@karabaja4
Copy link
Author

karabaja4 commented Sep 16, 2023

Got it. If this is by design, I will close this bug. Can you confirm that this is the case?

@joelim-work
Copy link
Collaborator

Yeah, relying on mtime is an intentional design decision, otherwise lf wouldn't know if cache entries are out of date or not.

@karabaja4
Copy link
Author

OK, thank you for the explanation. Closing.

@gokcehan
Copy link
Owner

@joelim-work Since :delete is a builtin command, can we not simply use the same approach in #1138 for this? Something along the line:

diff --git a/nav.go b/nav.go
index 8889f54..d4d5c67 100644
--- a/nav.go
+++ b/nav.go
@@ -1519,6 +1519,9 @@ func (nav *nav) del(app *app) error {
                                echo.args[0] = fmt.Sprintf("[%d] %s", errCount, err)
                                app.ui.exprChan <- echo
                        }
+
+                       delete(nav.regCache, path)
+                       delete(nav.dirCache, path)
                }

                nav.deleteTotalChan <- -len(list)

This does seem to work for the simple case, but I think maybe we may need to delete everything recursively so subdirectories in the cache would also be removed. I think this might also be required for the :rename as well since you can rename a directory which possibly have its subdirectories in the cache.

@joelim-work
Copy link
Collaborator

@gokcehan I guess there's no problem with deleting cache entries upon delete, rename (and also paste with cut), although when I was looking at the code, it looks like cache entries are only updated when necessary. It's not possible for lf to know about external changes and delete the cache entries accordingly, for example this will still be a problem if the user doesn't use delete and instead uses some trash manager.

That being said, I don't much opinion on this either way.

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

3 participants