-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix #2279: File permissions are kept upon save #2281
Conversation
Good job! Give it some time. If there is no build tomorrow, try restarting. Then @g-mos should give it a try and see if it keeps permissions. |
I would be happy to test whatever is needed. Since I am new to this, inform me and tell me, when and what I need to download. |
@tobiasdiez Could you create a branch at the JabRef main repo?
|
@koppor I pushed to a branch in the main rep (https://github.com/JabRef/jabref/tree/filePermissions) but no build appears under http://builds.jabref.org/. |
A test failed at circleCI... I've restarted the build: https://circleci.com/gh/JabRef/jabref/8328 |
Thanks @matthiasgeiger, it worked! @g-mos could you please install the development version from http://builds.jabref.org/filePermissions/ and test if the permission issue is indeed resolved. Thank you! |
LGTM 👍 |
I can confirm that the permissions of the files are kept. Therefore the issue at hand is fixed. |
@@ -4,7 +4,7 @@ This project **does not** adhere to [Semantic Versioning](http://semver.org/). | |||
This file tries to follow the conventions proposed by [keepachangelog.com](http://keepachangelog.com/). | |||
Here, the categories "Changed" for added and changed functionality, | |||
"Fixed" for fixed functionality, and | |||
"Removed" for removed functionality is used. | |||
"Removed" for removed functionality are used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "are"?
Functionality is singular
Either you let the is or you use functionalities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the object is "categories" which is plural.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the moment I think we can merge it in.
We should think about/creating a new PR for checking if the copy is really necessary
@g-mos Thanks for your feedback (and reporting the issue in the first place). |
@tobiasdiez I agree for the security layer. My idea is |
@g-mos Due to a problem on windows I slightly had to change the code. Follow up is in #2286 |
Unfortunately @Siedlerchr, this new version returns to the original problem. The permissions are completely destroyed. rw for user and no access to everybody else. It is also changing of user/group ownership to the user that shaved the jabref file. Sorry for the bad news. |
@g-mos Okay, thanks for the feedback. I will investigate this further. In the meantime you can revert back to the version before. |
Fixes #2279.
JabRef does not directly writes to the bib-file upon save but creates a temporary file which is then copied over the original one. In this process the original file permissions get lost. Should be fixed with this PR (I have not tried it...).
I pushed the code to the branch https://github.com/JabRef/jabref/tree/filePermissions but for some reason the setup files do not show up on builds.jabref.org
gradle localizationUpdate
?