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

cache isdir status on traverse item #413

Closed
plastikfan opened this issue Jan 18, 2024 · 6 comments · Fixed by #414 or #416
Closed

cache isdir status on traverse item #413

plastikfan opened this issue Jan 18, 2024 · 6 comments · Fixed by #414 or #416
Assignees
Labels
feature New feature or request

Comments

@plastikfan
Copy link
Contributor

This is required because sometimes, file items can be transient in nature. eg, the journalling feature in pixa needs to be able to delete journal files during traversal. But this deletion is interfering with the navigator because it attempts to query the directory status of a file after it has been deleted.

To fix this, we should change IsDir on the traverse item to cache the directory flag so that the underlying isdir functionality never has to be queried.

@plastikfan plastikfan added the feature New feature or request label Jan 18, 2024
@plastikfan plastikfan self-assigned this Jan 18, 2024
@plastikfan plastikfan linked a pull request Jan 18, 2024 that will close this issue
@plastikfan plastikfan reopened this Jan 18, 2024
@plastikfan
Copy link
Contributor Author

this method was overlooked in addressing this issue:

func (a *navigationAgent) notify(params *agentNotifyParams) (SkipTraversal, error) {
	skip := SkipNoneTraversalEn

	if params.readErr != nil {
		if a.doInvoke.Get() {
			clone := params.current.clone()
			clone.Error = i18n.NewThirdPartyErr(params.readErr)

			// Second call, to report ReadDir error
			//
			if le := params.frame.proxy(clone, nil); le != nil {
				if errors.Is(params.readErr, fs.SkipAll) && (clone.Entry != nil && clone.Entry.IsDir()) {
					params.readErr = nil
				}

				return SkipAllTraversalEn, i18n.NewThirdPartyErr(params.readErr)
			}
		} else {
			return SkipAllTraversalEn, i18n.NewThirdPartyErr(params.readErr)
		}
	}

	return skip, nil
}

Need to make sure there are no more calls to Entry.IsDir or Info.IsDir, they should always use the one on TraverseItem

Actually, this code looks like it was old code that should have been updated, long ago.

@plastikfan
Copy link
Contributor Author

plastikfan commented Jan 18, 2024

To distingusih betweed the is dir functionality on TraverseItem and FileInfo.IsDir/DirEntry.IsDir, rename TraverseItem version to IsDirectory.

Note: the sampling-while-iterator deals with fs.FileInfo instances directly, but for now this is ok and IsDir is called directly on it, because with its current design, doesnt have access to a TraverseItem instance to use instead.

@plastikfan
Copy link
Contributor Author

Still failing

end to end REAL [It] should: tinkle the ivories
/Users/plastikfan/dev/github/snivilised/pixa/src/app/proxy/controller_test.go:584

  [FAILED] execution result non nil (lstat /Users/plastikfan/dev/test/pics/screen-shot-1.png.$journal.txt: no such file or directory)
  Expected
      <*fs.PathError | 0xc000592030>:
      lstat /Users/plastikfan/dev/test/pics/screen-shot-1.png.$journal.txt: no such file or directory
      {
          Op: "lstat",
          Path: "/Users/plastikfan/dev/test/pics/screen-shot-1.png.$journal.txt",
          Err: <syscall.Errno>0x2,
      }
  to be nil
  In [It] at: /Users/plastikfan/dev/github/snivilised/pixa/src/app/proxy/controller_test.go:610 @ 01/18/24 14:40:44.023

@plastikfan
Copy link
Contributor Author

plastikfan commented Jan 18, 2024

this could be a problem:

func (a *navigationAgent) traverse(params *agentTraverseParams) (*TraverseItem, error) {
	for _, entry := range params.entries {
		path := filepath.Join(params.parent.Path, entry.Name())
		info, e := entry.Info() // <---- PROBLEM!!!!

...

func (d *unixDirent) Info() (FileInfo, error) {
	if d.info != nil {
		return d.info, nil
	}
	return lstat(d.parent + "/" + d.name)
}

... and this is where the lstat error is coming from.

@plastikfan
Copy link
Contributor Author

plastikfan commented Jan 18, 2024

so if we see a PathError, then ignore it. Actually, it is better just to implement the exclusions issue. But as a test, in pixa, let us filter out journal files in the ReadDirectory hook, which we have already overridden to ignore .DS_Store files.

@plastikfan
Copy link
Contributor Author

was resolved by modifying the ReadDirectory hook function to filter out journal files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
1 participant