all: Support syncing ownership (fixes #1329) #8434
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds support for syncing ownership on Unixes and on Windows. The scanner always picks up ownership information, but it is not applied unless the new folder option "Sync Ownership" is set. Ownership data is stored in a new FileInfo field called "OS data". This is intended to hold further OS specific data in the future (specifically, extended attributes), which is why the whole design is a bit overkill for just ownership.
* main: gui, man, authors: Update docs, translations, and contributors cmd/syncthing/cli: Add show discovery command (fixes syncthing#8007) (syncthing#8378) gui, man, authors: Update docs, translations, and contributors lib/osutil: Only announce address of interfaces which are up (fixes syncthing#7458) (syncthing#8422) gui: Fix missing span end tag and missing nbsp semicolon in HTML (syncthing#8419)
calmh
commented
Jul 21, 2022
Comment on lines
+151
to
+154
| message PlatformData { | ||
| UnixData unix = 1 [(gogoproto.nullable) = true]; | ||
| WindowsData windows = 2 [(gogoproto.nullable) = true]; | ||
| } |
There was a problem hiding this comment.
This will grow with more entries soon, to enable xattr support.
There was a problem hiding this comment.
Should this be called ownership then/oppose to unix/windows?
Or is that going to go under *Data?
There was a problem hiding this comment.
I was planning to stuff windows xattrs in windows platform data, and invent new linux/freebsd/macOS/etc platform data fields for xattrs for those platforms. The reason being that even if the apis are similar the xattrs for one don't apply to the other. (And also add a non-wire-sent inode modtime field to detect changes.)
calmh
changed the title
|
Deepsource seems confused about which "problems" are added in this PR, and anyway somewhat annoying... |
|
I'm out till Monday/Tuesday, but hopefully not forget to have a look once I am back. |
AudriusButkevicius
previously approved these changes
Jul 25, 2022
Comment on lines
+151
to
+154
| message PlatformData { | ||
| UnixData unix = 1 [(gogoproto.nullable) = true]; | ||
| WindowsData windows = 2 [(gogoproto.nullable) = true]; | ||
| } |
There was a problem hiding this comment.
Should this be called ownership then/oppose to unix/windows?
Or is that going to go under *Data?
| //go:build !windows | ||
| // +build !windows | ||
|
|
||
| package model |
There was a problem hiding this comment.
Do we really want to have folder_<foldertype>_<thing>_<os>.go ?
Seems like folder_<foldertype>_<os>.go would already result in quite a big matrix.
calmh
added a commit
to calmh/syncthing
that referenced
this pull request
Jul 26, 2022
* main: all: Support syncing ownership (fixes syncthing#1329) (syncthing#8434) gui, man, authors: Update docs, translations, and contributors
calmh
added a commit
to calmh/syncthing
that referenced
this pull request
Jul 28, 2022
* main: cmd/syncthing, lib/config: Remove restartOnWakeup option & functionality (fixes syncthing#8448) (syncthing#8449) gui: Remove blank meta tags (syncthing#8362) gui: Add device sync status (fixes syncthing#7981) (syncthing#8401) gui: Fix detailed staggered versioning information in folder info (ref syncthing#8348) (syncthing#8433) all: Support syncing ownership (fixes syncthing#1329) (syncthing#8434) gui, man, authors: Update docs, translations, and contributors lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes syncthing#8296) (syncthing#8427) gui: Move filesystem watcher explanation from tooltip to help block (syncthing#8432) gui: Use discovered IDs from cache when adding a new remote device (syncthing#8382) build: Update goleveldb (syncthing#8440) gui, man, authors: Update docs, translations, and contributors cmd/syncthing/cli: Add show discovery command (fixes syncthing#8007) (syncthing#8378) gui, man, authors: Update docs, translations, and contributors lib/osutil: Only announce address of interfaces which are up (fixes syncthing#7458) (syncthing#8422) gui: Fix missing span end tag and missing nbsp semicolon in HTML (syncthing#8419)
Martchus
pushed a commit
to Martchus/syncthing
that referenced
this pull request
Aug 4, 2022
This adds support for syncing ownership on Unixes and on Windows. The scanner always picks up ownership information, but it is not applied unless the new folder option "Sync Ownership" is set. Ownership data is stored in a new FileInfo field called "platform data". This is intended to hold further platform-specific data in the future (specifically, extended attributes), which is why the whole design is a bit overkill for just ownership.
st-review
added
the
frozen-due-to-age
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for syncing ownership on Unixes and on Windows. The scanner always picks up ownership information, but it is not applied unless the new folder option "Sync Ownership" is set.
Ownership data is stored in a new FileInfo field called "Platform". This is intended to hold further OS specific data in the future (specifically, extended attributes).
Also refactors the FileInfo comparison which started to grow a lot of inscrutable boolean arguments, now instead taking a struct with the comparison parameters. (Perhaps the comparison function should be a method on the comparison struct?)
(This is an alternative implementation to #8415, eschewing maps...)