Bug 756787

Summary: RFE: allow overwrite options included with .include
Product: [Fedora] Fedora Reporter: Petr Lautrbach <plautrba>
Component: systemdAssignee: systemd-maint
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: hhorak, james, johannbg, johannbg, k.georgiou, lpoetter, metherid, mschmidt, notting, plautrba, systemd-maint
Target Milestone: ---Keywords: FutureFeature
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-02-25 21:44:28 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 784611    

Description Petr Lautrbach 2011-11-24 09:37:36 EST
Description of problem:

If I want to change only some directives in a default unit and I don't want to loose track of package updates, I should create new unit in /etc/systemd/system with .include of original unit. But This doesn't work if I need change option which can be specified more than once.

E.g. /etc/systemd/system/local-sshd.service
.include /lib/systemd/system/sshd.service
[Service]
Execstart=/usr/local/sbin/sshd -D $OPTIONS

will end with error message:

systemd[1]: sshd.service has more than one ExecStart setting, which is only allowed for Type=oneshot services. Refusing.

Ideas:
- introduce .include_overwrite
- allow reset option with empty value:
ExecStart=
Execstart=/usr/local/sbin/sshd -D $OPTIONS
Comment 1 Jóhann B. Guðmundsson 2011-11-24 10:18:40 EST
I personally had though my self to file an RFE for an overwrite switch as in

.include
OverWrite=Yes
Exec$foo=bla 

But decided not to because once you start overwriting the default as opposed to be adding something to the unit you beat the purpose of .include and might just as well just copy the whole unit and edit it instead of adding several lines of codes trying to determine which one of multiple ExecStart/Pre/Post/ the end user is trying owerwrite.
Comment 2 Michal Schmidt 2011-11-24 11:07:47 EST
> - allow reset option with empty value:
> ExecStart=
> Execstart=/usr/local/sbin/sshd -D $OPTIONS

I like this one because it was actually I who gave the idea to Petr :-)

Jóhann, I disagree with your objection. There's no need to determine anything. "ExecStart=" would simply mean "forget all ExecStart lines defined up to this point".
Comment 3 Michal Schmidt 2011-11-24 11:17:55 EST
I would also find it nice to be able to remove selected values from lists.
For example, getty@.service has:

After=dev-%i.device systemd-user-sessions.service plymouth-quit-wait.service
After=rc-local.service

Wouldn't it be useful to be able to say:
.include /lib/systemd/system/getty@.service
[Unit]
After != rc-local.service

in order to remove one ordering dependency from the unit and preserve all the others? (I don't insist on the exact syntax)
Comment 4 Jóhann B. Guðmundsson 2011-11-24 11:32:35 EST
If used in conjunction with After= Before wont you risk messing up ordering along with wants requires etc?


And in service section how are you going to solve which Exec$foo lines are going to be overwritten if there are more then one line?
Comment 5 Michal Schmidt 2011-11-24 12:08:06 EST
(In reply to comment #4)
> If used in conjunction with After= Before wont you risk messing up ordering
> along with wants requires etc?

There's always a risk of messing things up when reconfiguring anything. You have to know what you're doing. I don't see why having the ability to remove selected dependencies would be particularly dangerous.

> And in service section how are you going to solve which Exec$foo lines are
> going to be overwritten if there are more then one line?

With the "ExecStart=" syntax? As I tried to explain in comment #2, all of them. Example:

ExecStart=foo
ExecStart=bar
ExecStart=
ExecStart=baz

The result would be the same as:
ExecStart=baz
Comment 6 Michal Schmidt 2012-11-19 11:16:24 EST
*** Bug 878088 has been marked as a duplicate of this bug. ***
Comment 7 Michal Schmidt 2013-01-08 08:40:31 EST
*** Bug 891748 has been marked as a duplicate of this bug. ***
Comment 8 Michal Schmidt 2013-01-15 07:21:57 EST
*** Bug 874559 has been marked as a duplicate of this bug. ***
Comment 9 Lennart Poettering 2013-01-15 17:37:43 EST
I like the ExecStart= syntax with an empty assignment. I think that's the way to go. I am sold.
Comment 10 James Withers 2013-01-16 03:16:54 EST
Being able to override ExecStart is particularly useful for applications like MySQL or PostgreSQL; where config parameters can be passed on run. It allows for portable modifications that do not override core code and are not [at risk of being] lost in upgrades
Comment 11 Jóhann B. Guðmundsson 2013-01-16 05:29:58 EST
Just ensure you wont wipe all Exec$foo= lines once this is implemented. 

Administrators might just want to replace one of those lines not all..
Comment 12 Lennart Poettering 2013-01-16 20:49:43 EST
Implemented in git.
Comment 13 Fedora Update System 2013-01-28 14:13:18 EST
systemd-197-1.fc18.2 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/systemd-197-1.fc18.2
Comment 14 Fedora Update System 2013-01-29 19:35:37 EST
Package systemd-197-1.fc18.2:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing systemd-197-1.fc18.2'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-1590/systemd-197-1.fc18.2
then log in and leave karma (feedback).
Comment 15 Jóhann B. Guðmundsson 2013-01-30 02:16:34 EST
Lennart how exactly did this get implemented?

Specifically what happens if I have for example two ExecStart lines and only want to replace one of them ( not all of them or everything there after )?
Comment 16 Michal Schmidt 2013-01-30 05:56:51 EST
> Specifically what happens if I have for example two ExecStart lines and only
> want to replace one of them ( not all of them or everything there after )?

There's no way to do that. You'll have to clear them all and then re-add the ones you want.

ExecStart=
ExecStart=/usr/bin/foo
ExecStart=/usr/bin/bar
Comment 17 Jóhann B. Guðmundsson 2013-01-30 12:31:43 EST
(In reply to comment #16)
> > Specifically what happens if I have for example two ExecStart lines and only
> > want to replace one of them ( not all of them or everything there after )?
> 
> There's no way to do that. You'll have to clear them all and then re-add the
> ones you want.
> 
> ExecStart=
> ExecStart=/usr/bin/foo
> ExecStart=/usr/bin/bar

That kinda defines this purpose does in it because you can just as well skip the .include line and copy the whole unit and edit it if comes down to that.

But I thought this could be achieving that by matching the exact line you want to 
replace ( search replace ) and just replace that one as in 

Or if you could define if many that it should only replace the first line with ExecStart and replace that or the second line containing ExecStart

Heck even saying replace line 5 in the unit with this line would be better then just blindly dismiss what ever comes after the line you want to replace
Comment 18 Michal Schmidt 2013-01-30 13:50:52 EST
(In reply to comment #17)
> That kinda defines this purpose does in it because you can just as well skip
> the .include line and copy the whole unit and edit it if comes down to that.

Not quite. You can still inherit all other settings.

> But I thought this could be achieving that by matching the exact line you
> want to 
> replace ( search replace ) and just replace that one as in

You mean some sed-like capability? s/old/new/? 

> Or if you could define if many that it should only replace the first line
> with ExecStart and replace that or the second line containing ExecStart

I'm afraid this would be too fragile to use.

> Heck even saying replace line 5 in the unit with this line would be better
> then just blindly dismiss what ever comes after the line you want to replace

And this even more so.
Comment 19 Jóhann B. Guðmundsson 2013-01-30 13:59:57 EST
(In reply to comment #18)
> (In reply to comment #17)
> > That kinda defines this purpose does in it because you can just as well skip
> > the .include line and copy the whole unit and edit it if comes down to that.
> 
> Not quite. You can still inherit all other settings.

Unless I'm misunderstanding something that will not happen in type oneshot units which often have more then one instance of the same ExecFoo line the same thing apply with other types that have multiple ExecStartPre/Post lines ( if you would replace either of those. 

> 
> > But I thought this could be achieving that by matching the exact line you
> > want to 
> > replace ( search replace ) and just replace that one as in
> 
> You mean some sed-like capability? s/old/new/? 

Yes 

Replace ExecStart=/bin/foo -a -b -c with ExecStart=/bin/foo -x- y -z which is the most common use case for this ( one line change ) the other is to add Restart=foo but I expect we will decide how we go forward with that @BRNO there are certain bits missing before we can default to that ( which leaves out only the one line changes )
Comment 20 Fedora Update System 2013-02-22 20:02:40 EST
Package initscripts-9.42.2-1.fc18, systemd-197-1.fc18.2:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing initscripts-9.42.2-1.fc18 systemd-197-1.fc18.2'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-1590/initscripts-9.42.2-1.fc18,systemd-197-1.fc18.2
then log in and leave karma (feedback).
Comment 21 Fedora Update System 2013-02-25 21:44:32 EST
initscripts-9.42.2-1.fc18, systemd-197-1.fc18.2 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.