I have to sync several Windows machines where some directory have many files that have the HIDDEN attribute bit set. On the destination machines, after the synchronization is done, the attribute is lost, so I must resort to use some scripts to keep the files synced with this attribute. I see that the HIDDEN attribute is already used in the Windows version of Synchthing, for hiding the temporary files under sync, but then it is removed on final rename. Having this attribute enforced in this platform will be very beneficial, at least for who use this very useful tool in this os platform that is very popular. Using v0.14.47, Windows (64 bit)
Yeah currently the hidden attribute is not synced. It would be a fair feature request to do so, I think. Mind filing it on GitHub?
Yes, I will file it on GitHub. Thanks Jakob
@calmh , in https://github.com/syncthing/syncthing/issues/4935 you mentioned that SYSTEM attribute isn’t grabbed as well. I’ve just started to learn about syncthing project and its source code, but it seems to me that no attributes are taken care of at all. Correct me if I’m wrong, but I think that currently we are specifying some file features when calling OpenFile(), which uses Posix convention. (so, basically, access modes and some flags). Hence it’s a bit confusing that you pointed out only SYSTEM attribute.
My idea for solution is:
- in folder_sendrecv.go: add FileAttributes field to sharedPullerState struct, and fill it with syscall.GetFileAttributes() if and only if we are currently using Windows OS. (otherwise it won’t be used at all since it doesn’t make sense on Unix…?).
- sharedpullerstate.go: inside finalClose(), instead of calling Unhide(), call setFileAttributes with appropriate attributes (remembered in sharedPullerState.FileAttributes field).
Of course every OS-specifc call would be hidden behind functions from Filesystem interface. Please, let me know if it seems right so I can implement it.
This is equivalent on how we handle permissions. Mode attribute in the protocol has spare bits, you’d need reserve them in the protocol and then handle them somewhere in the scanner and finisher routine on the model/puller.
What are the other attributes? I mentioned
system because I only know of
hidden, that, and
archive (which I think is meaningless to handle). But I don’t know this stuff, so if there’s other attributes that we should handle then by all means do so.
Depending on how many we could indeed shovel them above the regular bits of the permission field, or we could add a field for it. I guess it depends on what they are and where they fit. If they don’t have generally accepted bit values we should probably make up our own and define them as Audrius says. These aren’t exactly “permissions” as I understand them.
For me, the only useful flags to honor are the hidden and system flags:
- Hidden flag make some windows utilities to not show it in their interface (for example Explorer);
- System flag mark a file as part of operative system;
Both flags can be useful in the Windows world.
The Archive flag stands for ‘needs archive’ and resetting it can trick some backup utilities that use this flag to decide whether backup the file or not.
Given the volatile nature of the archive bit (i.e., it getting set on each file write) I don’t think it’s useful to sync. There are however a couple of additional ones:
- Compressed: When set, Windows compresses the hosting file upon storage. For more information, see NTFS § File compression.
- Encrypted: When set, Windows encrypts the hosting file upon storage to prevent unauthorized access. For more information, see NTFS § Encryption.
- not content-Indexed: When set, Indexing Service or Windows Search do not include the hosting file in their indexing operation.
I can see arguments for syncing these, and also the opposite (that they are a local concern and should not be set by Syncthing).
I’ve raised and already fixed the issue with missing file attributes in golang.
Given all of the them are available, I think it would be a nice feature to sync all of them. I guess it implies adding additional field for storing FILE_ATTRIBUTES_* value.
I think it makes no sense to sync some file attributes and some not, but I’ll be happy to discuss that.
I think we have a few spare bits in the FileMode bit mask which should be enough to sync the most reasonable ones. Syncing some others (such as directory, reparse point, virtual) makes no sense.
Yes, this is a trivial addition that needs a contributor.
This could be a minefield. Files and folders will inherit their parents’ EFS encryption flag (if the parent is set to do so), which is enough to ensure EFS files are handled correctly (they are here). Otherwise you could end up syncing files with a user account that hasn’t used EFS yet, which could mean data loss if the user hasn’t backed up their cert, could cause files to be encrypted by the wrong cert if Syncthing is running as a service or other user account. Worst of all, if a receiver receives files to a Windows edition / file system that doesn’t support EFS, it’s plausible that EFS flag being “not encrypted” could trickle up to the sender which would cause unwanted decryption.
I really don’t like this approach since it messes up two separate features - permissions and attributes. It would require additional work on every use on the permission field - from the top of my head, when using chmod, we would need to clear bits from ‘features’ part; when extracting features - we would need to convert them from our ‘custom’ representation to the widely-accepted ones. Of course it’s not infeasible - I just find it inelegant.
I agree that there are some unused bits in ‘permission’ field, but is saving space for one int worth this whole mess? It just feels like against all good practices. Also, there are some shifts performed on the permission values (protocol/bep.pb.go). I cannot tell without closer look if it would forbid using ‘unused’ bits in permission field, but it surely indicates the need for investigation if we decided to use them.
Last but not least, which file attributes would you like to have synced since you don’t want all of them?
Its not in the permissions field. It’s in the mode field. Go itself exposes permissions and file flags as a single int. I think we should only sync things we can resonably set (for example we might not be able to turn a file to a directory by just setting a flag, nor we might able to set system flag).
After discussing this issue with @calmh on IRC we decided that additional “attributes” field should be present in the protocol in order not to mix permissions with attributes as they are logically separated.
I have a problem with changing a protocol: When I try to create a bep.pb.go from bep.prot, I receive an error “github.com/gogo/protobuf/gogoproto/gogo.proto: File not found. syncthing/syncthing/lib/protocol/bep.proto: Import “github.com/gogo/protobuf/gogoproto/gogo.proto” was not found or had errors.” I have cloned gogo/protobuf and put it in $GOPATH dir. Once I managed to somehow generate a .pb.go file, but it was invalid. How to generate this file properly?
Are you using
go run build.go proto to do the building,
go generate or something else? The first command is the one I use, and should in theory at least work with just the vendored proto files. You might also need a
go install ./github.com/syncthing/syncthing/vendor/github.com/gogo/protobuf/protoc-gen-gogofast to get the compiler.
Expanding on the above about permissions / attributes fields, we take the current permissions field and do things like
fs.Chmod(file.Name, file.Permissions) without looking closer at the bits in the permissions field. Hence we don’t want to add random high bits there as it’s likely existing versions will do the wrong thing with them. A new
Attributes field will be cleanly ignored by old versions, and we can specify exactly which bits mean what and that any non-understood bits
MUST be ignored.