-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Only fail after chmod error if permissions differ (e.g. on config file) #8771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or 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 another operation for the path where |
Also very reasonable. Pushing that change – tested and working fine as well. |
e713088
to
4d72458
Compare
@imsodin Want to have another look if the current changes are what you desired? |
calmh
approved these changes
Feb 20, 2023
imsodin
approved these changes
Feb 20, 2023
calmh
added a commit
to imsodin/syncthing
that referenced
this pull request
Mar 10, 2023
* main: (46 commits) build: Update dependencies lib/api: Expose `blocksHash` in file info (syncthing#8810) gui, man, authors: Update docs, translations, and contributors lib/discover: Don't leak relay-tokens to discovery (syncthing#8762) gui, man, authors: Update docs, translations, and contributors gui: Add Croatian (hr) translation template (syncthing#8801) build(deps): bump github.com/quic-go/quic-go from 0.32.0 to 0.33.0 (syncthing#8800) cmd/stupgrades: Cache should apply to HEAD as well as GET build: Add more GitHub Actions gui: Remove non-existent customicons.css file reference (fixes syncthing#8797) (syncthing#8798) Only fail after chmod error if permissions differ (e.g. on config file) (syncthing#8771) gui, man, authors: Update docs, translations, and contributors build: Use Go 1.19.6 for Windows build: Update dependencies gui, man, authors: Update docs, translations, and contributors gui: Remove duplicate Spanish (Spain) translation (fixes syncthing#8781) (syncthing#8782) gui: Add xattr filter editor (fixes syncthing#8660) (syncthing#8734) gui: Switch to Weblate for translations (syncthing#8777) all: Use new Go 1.19 atomic types (syncthing#8772) gui, man, authors: Update docs, translations, and contributors ...
calmh
added a commit
to tomasz1986/syncthing
that referenced
this pull request
Mar 18, 2023
* main: (424 commits) gui, man, authors: Update docs, translations, and contributors lib/protocol: Cache expensive key operations (fixes syncthing#8599) (syncthing#8820) gui: Add Persian (fa) translation template (syncthing#8822) lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563) gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539) build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804) build: Update dependencies (syncthing#8821) lib/api: Expose `blocksHash` in file info (syncthing#8810) gui, man, authors: Update docs, translations, and contributors lib/discover: Don't leak relay-tokens to discovery (syncthing#8762) gui, man, authors: Update docs, translations, and contributors gui: Add Croatian (hr) translation template (syncthing#8801) build(deps): bump github.com/quic-go/quic-go from 0.32.0 to 0.33.0 (syncthing#8800) cmd/stupgrades: Cache should apply to HEAD as well as GET build: Add more GitHub Actions gui: Remove non-existent customicons.css file reference (fixes syncthing#8797) (syncthing#8798) Only fail after chmod error if permissions differ (e.g. on config file) (syncthing#8771) gui, man, authors: Update docs, translations, and contributors build: Use Go 1.19.6 for Windows build: Update dependencies ...
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
frozen-due-to-age
Issues closed and untouched for a long time, together with being locked for discussion
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.
Purpose
Given:
In this case, Syncthing cannot chmod the configuration file
config.xml
while saving it:That is because only the owner, not the group, can change permissions. Observed on Linux.
Since the config file is first atomically written, and then
chmod
attempted, the config file still gets written. But theSave
function logs and returns this error and therefore also shows an error dialog in the GUI. To avoid running into this, my changeonly calls[UPDATE] only fails after aChmod(...)
if permissions differChmod(...)
error if desired permissions really differ.Testing
Ran Syncthing server before and after the change. The fix avoids any errors while saving the config file, given my scenario described above.