buffer: Store potential resolved symlinks as AbsPath

[JoeKar]

This will help us to keep track of the same file opened via different symlinks.

Fixes #3995

[dmaluka]

I've noticed an "issue" with this PR. If we have a regular file foo and a symlink bar pointing to it, and we open bar in micro, the statusline shows its name is bar (not resolved). Ok, that is not a problem in itself. Then we open foo in another pane, and the statusline shows its name is bar too, not foo. Just because bar has been already opened, so its SharedBuffer already exists (and thus is reused by foo), and b.Path in this SharedBuffer is bar, not foo. Even though it is foo that is the original file, while bar is a symlink to it.

For comparison, vim shows the resolved file name in the statusline, so it behaves predictably: it always shows the original file name.

Well, maybe we can treat this as a feature ("first wins" rule)...

[JoeKar]

For comparison, vim shows the resolved file name in the statusline, so it behaves predictably: it always shows the original file name.

Well, maybe we can treat this as a feature ("first wins" rule)...

Hm, indeed...this behavior is somehow strange.
The order of opening the file shouldn't influence how it is named in the editor.
It should be every time the file name/path the user opened or the resolved path.

EDIT:
The more tricky thing is the (relative) path before the basename of the file (set basename false).

EDIT2:
Didn't find an elegant solution so far.

[dmaluka]

Hmmmm... what is this? https://pkg.go.dev/path/filepath#EvalSymlinks :)

Worth checking if we can just use this?

Also, just realized: we should resolve each component of the path (i.e. each directory), not just the last one (the file itself), since any of the directories may be a symlink too?

And skimming through the code in https://github.com/golang/go/blob/master/src/path/filepath/symlink.go it looks like this filepath.EvalSymlinks() does resolve them all.

[JoeKar]

Hmmmm... what is this? https://pkg.go.dev/path/filepath#EvalSymlinks :)

Worth checking if we can just use this?

Whaaat...feels like this came out of nowhere. Don't know, why I didn't see this myself, especially since I had [filepath](the https://pkg.go.dev/path/filepath) documentation the last days open again.
But yes, definitely and it sounds promising, since it does exactly what we need.

Thank you for pointing me to this link!

[dmaluka]

Strictly speaking, it may fail here even if the checks in NewBufferFromFileWithCommand() didn't fail, since the state of the file may have changed in the meantime? https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use

And, checking and returning this error here isn't particularly hard?

[JoeKar]

And, checking and returning this error here isn't particularly hard?

Here I agree, but...

Strictly speaking, it may fail here even if the checks in NewBufferFromFileWithCommand() didn't fail, since the state of the file may have changed in the meantime?

...and here I only partially agree.
How often can it still change afterwards over the lifetime of the open buffer? The file, related to the stored path, is opened again at save and till then TOC/TOU is still a thing.
I though about this while writing, but didn't consider it for this PR.
The problem is, that we only track the path and no concrete file handle or any other sort of file abstraction, because we close it at exit of NewBufferFromFileWithCommand().

[dmaluka]

I don't think we should be responsible for the file throughout the lifetime of a buffer. I doubt any editor does that, and I don't think that would be a good idea.

What we are responsible for is preventing micro from crashing or behaving unpredictably.

So, here I'm only talking about a simple issue: filepath.EvalSymlinks() may still return an error here (even if that's unlikely), so, what will be the value of path then? The documentation in https://pkg.go.dev/path/filepath#EvalSymlinks doesn't say what is returned as the 1st return value in case of an error. We might optimistically assume that it will be just the original path (without symlinks resolved), or e.g. with symlinks just partially resolved, but still a correct path. And we will be wrong: from the source code in https://github.com/golang/go/blob/master/src/path/filepath/symlink.go we can see that it will be just an empty string, not a file path.

So, it will silently cause incorrect behavior: we will open an empty unnamed buffer, instead of opening the file the user asked us to open?

[dmaluka]

BTW I've noticed another issue: we should resolve the absolute path as well. The current working directory path may contain symlinks too, so if we only resolve the relative path, and prepend the current working directory path to it after that, the resulting absolute path will not be fully resolved.

For example:

mkdir dir
ln -s dir dir1
echo test >dir/file
cd dir1
micro -multiopen vsplit file ../dir/file

So we should do something like this?

		resolvedPath, err := filepath.EvalSymlinks(path)
		if err == nil {
			path = resolvedPath
		}

		absPath, err = filepath.Abs(path)
		if err != nil {
			absPath = path
		}

		resolvedPath, err = filepath.EvalSymlinks(absPath)
		if err == nil {
			absPath = resolvedPath
		}

BTW since we are doing this in 2 or even 3 places, we should add a helper?

[JoeKar]

[...] it will be just an empty string, not a file path.

So, it will silently cause incorrect behavior: we will open an empty unnamed buffer, instead of opening the file the user asked us to open?

Well, right now...yes. Even too much links could cause this.

The current working directory path may contain symlinks too, so if we only resolve the relative path, and prepend the current working directory path to it after that, the resulting absolute path will not be fully resolved.

Good catch, thank you!

I will update the PR.

Edit:
It should be feasible to create the absolute path first before possible links are evaluated, right? Then we wouldn't need the second evaluation loop.

[dmaluka]

Edit:
It should be feasible to create the absolute path first before possible links are evaluated, right? Then we wouldn't need the second evaluation loop.

Just in case, github doesn't send email notifications about edits in comments, so I would never know about your question if I hadn't accidentally noticed it just now. :)

[dmaluka]

I've noticed another corner-case issue: if the file doesn't exist yet (so it is opened as an empty file) and its directory path contains symlinks, those symlinks are not resolved. E.g.:

mkdir dir
ln -s dir dir1
micro -multiopen vsplit dir/file dir1/file

For comparison, again, vim correctly resolves it (thus correctly detects that dir/file and dir/file1 are the same file, even though this file doesn't exist yet).

[JoeKar]

Very good catch! 👍
Caused by symlink.go;l=84-87 and stat_unix.go;l=47-49, but fortunately it is wrapped into a PathError containing the causing path name. Here we can apply a little "hack" where EvalSymlinks() still does the job with the path and we pick it from the error message.

[dmaluka]

Are we relying on some undocumented behavior here?

There is another corner case: the file doesn't exist, and its directory doesn't exist either, but some of its ancestor directories exists and contains symlinks:

mkdir dir
ln -s dir dir1
micro dir1/dir/file

As you can see, this newest version handles this case utterly incorrectly.

Furthermore, even if there are no symlinks (e.g. micro dir/dir/file with the above example), it behaves in the same utterly incorrect way. I.e. it causes a regression.

[JoeKar]

Right, EvalSymlinks() stops at the first unavailable dir or file inside the path.
Unfortunately, it's not that simple anymore. 🤔

[dmaluka]

The only exception is the case in which the target file doesn't exist. We leave the path handling to EvalSymlinks() and use the path causing the error as target path.

This sounds too detailed, and at the same time completely unclear. What "path handling", etc.

Why not just e.g. "In case the file does not exist, ResolvePath still resolves it, by resolving its directory path and keeping the file name." ?