Option to follow directory junctions / symbolic links?

xarx · April 30, 2020, 3:25pm

Good point. I didn’t see a reason why to keep track of ordinary directories.

calmh · April 30, 2020, 3:28pm

Btw the better place to start is probably to copy out Lstat() from basicfs.go into the unix and windows specializations and there have the windows version do the extra syscall etc. Then you pass the extra flags or whatnot to the basicFileInfo which already has a windows specialization in basicfs_fileinfo_windows.go that does hacks around symlinks… Drop the symlink bit there if you know it’s a junction, and that’s probably all you need to do.

xarx · April 30, 2020, 3:31pm

This behaviour should be dependent on the current Advanced configuration of the current directory. Is fs the right place? I.e. is the current fs layer behaviour already determined by the Advanced configuration?

calmh · April 30, 2020, 3:33pm

There are no configurable things in there today, no. Are we sure this needs an off switch? I mean, there are multiple options of creating one, if so. One would be to make this a separate filesystem type. Today we have “basic” (this) and “fake” (in-memory). “junction-following” could be a subtype of “basic”.

xarx · April 30, 2020, 3:34pm

I do not need the switch. But there are other users too, and they may have a different opinion on that.

calmh · April 30, 2020, 3:35pm

And no doubt they will voice them, and then we’ll know the reasons. I’m generally pro avoiding knobs and switches until they’re obviously needed. This is one of those things where I think I’m fine with just having the one behavior until the opposite is proven.

xarx · April 30, 2020, 4:00pm

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.

calmh · April 30, 2020, 4:04pm

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.

xarx · April 30, 2020, 4:07pm

Is this problem related to

?

calmh · April 30, 2020, 4:14pm

That’s the same thing yeah.

xarx · April 30, 2020, 4:35pm

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.

calmh · April 30, 2020, 5:02pm

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

github.com

syncthing/syncthing/blob/bdb25f9ba5b781ad309a888d50ed22ddf38df377/lib/fs/basicfs.go#L144


func (f *BasicFilesystem) MkdirAll(path string, perm FileMode) error {
	path, err := f.rooted(path)
	if err != nil {
		return err
	}


	return f.mkdirAll(path, os.FileMode(perm))
}


func (f *BasicFilesystem) Lstat(name string) (FileInfo, error) {
	name, err := f.rooted(name)
	if err != nil {
		return nil, err
	}
	fi, err := underlyingLstat(name)
	if err != nil {
		return nil, err
	}
	return basicFileInfo{fi}, err
}

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.

xarx · April 30, 2020, 5:28pm

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.

AudriusButkevicius · April 30, 2020, 5:44pm

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.

xarx · April 30, 2020, 5:46pm

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.

AudriusButkevicius · April 30, 2020, 6:03pm

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:

github.com

syncthing/syncthing/blob/32a76901a91ff0f663db6f0830e0aedec946e4d0/internal/symlinks/symlink_windows.go

// Copyright (C) 2014 The Syncthing Authors.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at http://mozilla.org/MPL/2.0/.

// +build windows

package symlinks

import (
	"os"
	"path/filepath"

	"github.com/syncthing/protocol"
	"github.com/syncthing/syncthing/internal/osutil"

	"syscall"
	"unicode/utf16"
	"unsafe"

This file has been truncated. show original

xarx · April 30, 2020, 6:13pm

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.

AudriusButkevicius · April 30, 2020, 6:16pm

You probably have to copy it.

xarx · May 3, 2020, 9:07pm

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})}

calmh · May 3, 2020, 9:10pm

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