Skip to content

contrib: replace deprecated PermissionsStartOnly in systemd init #16994

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.

Sign up for GitHub

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

Jump to bottom
Closed
setpill wants to merge 1 commit into from
Closed

contrib: replace deprecated PermissionsStartOnly in systemd init #16994

setpill wants to merge 1 commit into from

Conversation

@setpill
Copy link
Contributor

@setpill setpill commented Sep 30, 2019

PermissionsStartOnly is deprecated (but not yet removed); its
functionality replaced by special executable prefixes. The ! prefix
allows the prefixed command to be run with unrestricted User and Group.
This is necessary to ensure group ownership is set correctly to the
configuration directory.

Followup on @hebasto's comment on #16556

@laanwj laanwj changed the title Systemd replace deprecated PermissionsStartOnly contrib: replace deprecated PermissionsStartOnly in systemd init Sep 30, 2019
@hebasto
Copy link
Member

hebasto commented Sep 30, 2019 •
edited
Loading

ACK 36030e0a07b8adac990b38a0b544f54cb150904f, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged. Concept ACK. Will test.

See: Special executable prefixes

PermissionsStartOnly is deprecated since systemd v240.

Systemd versions:

  • Debian:
    • jessie: 215-17+deb8u13
    • stretch (via backports): 241-5~bpo9+1
    • buster: 241-7~deb10u1
  • Ubuntu:
    • xenial: 229-4ubuntu21.21
    • bionic: 237-3ubuntu10.29
  • Fedora 30: 241-12.git1e19bcd.fc30

hebasto
hebasto reviewed Sep 30, 2019
View reviewed changes
@setpill
Systemd replace deprecated PermissionsStartOnly
bd8d659 
PermissionsStartOnly is deprecated (but not yet removed); its
functionality replaced by special executable prefixes. The `!` prefix
allows the prefixed command to be run with unrestricted User and Group.
This is necessary to ensure group ownership is set correctly to the
configuration directory.
@setpill setpill force-pushed the fix-deprecated-permissionsstartonly branch from 36030e0 to bd8d659 Compare September 30, 2019 12:32
@hebasto
Copy link
Member

hebasto commented Sep 30, 2019

ACK bd8d659, tested on Linux Mint 19.2

hebasto@linux-511:~$ systemctl start bitcoind.service
hebasto@linux-511:~$ stat /etc/bitcoin | grep id
Access: (0710/drwx--x---)  Uid: (    0/    root)   Gid: ( 1002/ bitcoin)

@ryanofsky
Copy link
Contributor

ryanofsky commented Sep 30, 2019

Do we know what version the prefix syntax was introduced, and if there are operating systems where the current file works but would be broken by this change?

systemd/systemd#10802 (comment) says they don't have plans to drop support for PermissionsStartOnly

@hebasto
Copy link
Member

hebasto commented Sep 30, 2019 •
edited
Loading

Do we know what version the prefix syntax was introduced..?

systemd/systemd#6577, since v235.

So, jessie and xenial do not comply.

@setpill
Copy link
Contributor Author

setpill commented Oct 1, 2019

Since this file is mostly a suggestion for packagers/sysadmins, would a comment suffice to address pre-v235 situations?

@setpill
Copy link
Contributor Author

setpill commented Oct 1, 2019

Then again, if there's no immediate plans for systemd to drop PermissionsStartOnly, perhaps it's better to revisit this PR when pre-v235 systems are EOL.

@laanwj
Copy link
Member

laanwj commented Oct 1, 2019

Then again, if there's no immediate plans for systemd to drop PermissionsStartOnly, perhaps it's better to revisit this PR when pre-v235 systems are EOL.

Tend to agree here, if there's no hurry, and everything else is the same, staying with the old configuration option for now will result in the least surprises.

@laanwj
Copy link
Member

laanwj commented Oct 30, 2019

Then again, if there's no immediate plans for systemd to drop PermissionsStartOnly, perhaps it's better to revisit this PR when pre-v235 systems are EOL.

Can you close this until then, please?

@setpill setpill closed this Oct 30, 2019
@laanwj laanwj added this to the Future milestone Oct 30, 2019
@hebasto hebasto mentioned this pull request Jul 15, 2020
Closed
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
@maflcko maflcko removed this from the Future milestone Jul 23, 2025
@fanquake
Copy link
Member

fanquake commented Jul 23, 2025

PIcked up in #33044.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto left review comments

Assignees

No one assigned

Labels

Scripts and tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@setpill @hebasto @ryanofsky @laanwj @fanquake @maflcko