Option to follow directory junctions / symbolic links?

Is the fs layer used to access the synchronized directories only? Or is it used for all disk related tasks, like reading the configuration, for temporary files etc.? In the second case, couldn’t the change cause any troubles?

Do you know, in advance, how would sync behave when a directory junction is corrupted? Would it immediately treat the directory as empty (and delete it from other devices too), or as unchanged (keeping it on other devices til the junction is fixed)? I do not plan to implement any checks for directory junction validity, it will fail at the os level on attempt to read or write its content. I.e. the same situation as if a directory or file is corrupted.

It’s used for all operations in synced folders, but not the database or config.

I expect the effect of a broken junction will be the same as a missing mount in Unix - the files are gone. We already have that caveat on those systems so I think it’s fine.

Is this problem related to

?

1 Like

That’s the same thing yeah.

One more question: Are paths handled by fs (e.g. by fs.Lstat()) somehow normalized, or can they be arbitrary, e.g. UNC paths? If they can be completely general, I would have to “normalize”/“fix” them before passing them to kernel32.dll, which would be a little more difficult.

The methods get a folder relative path which they convert to an absolute one:

I suppose they might be UNC paths if so configured. We also used to do the \\?\C:\... thing to get the long file name API, not sure if we still do.

I copy plenty of code from Golang internal packages, or from unexported functions. It’s an unnecessary duplication of code, if there’s a way how to call those normally inaccessible functions/packages. Is there? In particular, there is the rather long function that is used to “fix” the paths before calling kernel32.dll. If there’s no way, I would have to extract and copy the function into fs.

I’d be very surprised if there is no information indicating if it’s a junction or a symlink in os.FileInfo.Sys(). Stuff like inodes are definately there.

I’ve tried that yesterday, and Golang treats symlinks and junctions the same way. Internally, it has the information available, but it does not export it. That’s why I have to call the kerne32.dll myself. If you prove me wrong, then fine.

You are right, there is now way to know anything else other that is it’s a reparse point. You have to read the reparse point to figure out whats underneath it.

Some dinosaurs to potentially help you:

Yes, I do that in a similar way. The problem I’ve been talking about is in the line

ptr, err := syscall.UTF16PtrFromString(path)

In https://golang.org/src/os/stat_windows.go they use

namep, err := syscall.UTF16PtrFromString(fixLongPath(name))

instead. The fixLongPath() function is the function I was talking about. There’s probably a reason why they use it there, though I don’t know it. That’s why I was asking whether the paths need “fixing”. But the answer was that probably yes.

The function is rather complex, and I do not know how to call it, as it is not exported. Hence I will have to copy it.

You probably have to copy it.

Can you give me an advice?

Lstat() returns *os.fileStat as fs.FileInfo. I need to convert this to os.FileInfo, so that I can pass it to the function os.SameFile(os.FileInfo, os.FileInfo). This should be possible, because inside Lstat(), *os.fileStat was already converted to os.FileInfo (before being converted to fs.FileInfo). However, both commands

fi, ok := info.(os.FileInfo)
fi, ok := info.(interface{}).(os.FileInfo)

set ok=false. (Here, info is the pointer of type fs.FileInfo I need to convert.)

What is the correct way to perform the conversion? When I create a simplified test case, I am able to perform the conversion via the second command above.

info contains:

{%!s(*os.fileStat=&{Sync 16 {3806948330 30794625} {664948339 30810303} {664948339 30810303} 0 0 0 0 {0 0} \\?\D:\Users\xarx\Sync 0 0 0 false})}

I suspect you should do all this in the lstat implementation before it gets returned as an fs.FileInfo?

No, I have the Lstat() already modified and working. Now I created the “infinite recursion prevension” in (*fs.walkFilesystem).Walk(), but it doesn’t work because the conversion doesn’t work.

Edit: Or was your question meant as an answer?

The conversion doesn’t work because fs.FileInfo does not implement os.FileInfo (no Sys() interface{} method). To get the os.FileInfo something like info.(basicFileInfo).FileInfo would work, but you can’t use that in walkFs, as it must work with fs.Filesystem, not an implementation of that like fs.BasicFilesystem. If you need SameFile in walkFs, you’ll likely have to add that to the fs.Filesystem interface. Or more generally: You shouldn’t use os outside of BasicFilesystem, that package should be abstracted by filesystem.go.

From the end:

I can implement fs.SameFile() in BasicFilesystem, but this wouldn’t solve my problem. I would have the same conversion problem there, I suppose. In Walk() I use the conversion in a safe way, i.e. if the conversion cannot be done, I do perform a fallback behaviour.

Edit: info.(basicFileInfo).FileInfo can be compiled, sorry. Actually the conversion does something, thank you!

I need to log errors into the Syncthing log, in walkFilesystem. How should I do that?

The logger l that is available in the fs package has error/fatal/panic methods omitted. Should I add them to the logger.Logger interface? Why are they omitted?

The logger in fs uses the same interface as all the other packages, so it has (for example) Warnln and Warnf for logging errors to the user and the corresponding Info... methods for more informational things. That said, do we really want errors to the user from inside fs?

About the SameFile stuff, I suggest adding some sort of unique file identifier to our basicFileInfo that could be used for this purpose.

type basicFileInfo struct {
  // ...
  UniqueIdentifier interface{}
}

For example the inode in Unixes, or file ID in Windows. If we stored it as interface{} we could add a typed value from the actual implementation, such as a type unixInode int64 and then someInfo.UniqueIdentifier = unixInode(42) or in the Windows case ... = windowsFileIdentifier(1234). These values can be compared with == in fs.SameFile and that’ll only match if the type and value is identical. Also works with s3BucketHask("abcd1234") and such in the future.

No need then for gnarly type assertions in SameFile. And, possibly, no need for SameFile at all as it just compiles to an == and we could do that directly on the UniqueIdentifier instead.

I’ve got that already implemented and working. When I’ll send a pull request, you can review whether you want some changes to the code. I’m not completely satisfied with my solution, because I’m limited with my knowledge of Golang, and this requires more than a basic knowledge of Golang. But it works, and it is not ugly :-).

Actually, I need to use assertions. But it seems that there is no such thing in Golang. Hence, I want - at least - to log the situations when something doesn’t behave as expected. So that they are not silently ignored, they should - at least - occur in the log. Currently I use fmt.Printf().

1 Like