Skip to content

Conversation

DanielO
Copy link

@DanielO DanielO commented Jul 16, 2019

Purpose

Allow setting of umask directly which simplifies startup scripts.

Testing

Light testing on a FreeBSD server, verified new files are created with the expected permissions.

Documentation

syncthing/docs#460

Authorship

Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.

@AudriusButkevicius
Copy link
Member

Again, you can set this in the shell, so I think this is a needless addition

@AudriusButkevicius
Copy link
Member

It's cheap and easy to add these options, but incredibly hard to remove, so I am personally against this. Lets see what others say.

@DanielO
Copy link
Author

DanielO commented Jul 16, 2019

I found it quite difficult to work out how to get the umask set correctly in the init script due to the multiple levels of escaping required.

I don't necessarily disagree with you though but the end result is this lets me reliably set the umask which is critical to having permissions set correctly.

@AudriusButkevicius
Copy link
Member

Sure, but you could make the same argument about environment variables, and we should add -env, because of multiple levels of escaping. Multiple levels of escaping is sort of not our problem and us having to work around it seems wrong.

Again, lets see what others have to say.

@calmh
Copy link
Member

calmh commented Jul 16, 2019

I agree that while there is nothing technically
wrong with the implementation here this is not something we want to add. The umask is better set in a small shell script or as an option in the service manager, whichever such is used. It would also be odd to add something that doesn’t make sense on Windows and is silently ignored there.

@calmh calmh closed this Jul 16, 2019
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Jul 16, 2020
@syncthing syncthing locked and limited conversation to collaborators Jul 16, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants