[systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=

[Lennart Poettering]

On Fri, 07.11.14 14:24, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:

> On Fri, Nov 07, 2014 at 01:06:41PM +0100, Lennart Poettering wrote:
> > On Fri, 07.11.14 09:49, Jan Synacek (jsynacek at redhat.com) wrote:
> > 
> > > Lennart Poettering <lennart at poettering.net> writes:
> > > > On Thu, 06.11.14 10:49, Jan Synacek (jsynacek at redhat.com) wrote:
> > > >
> > > >> I think that this patch might be a bit ineffective, as it calls
> > > >> unit_file_load() again just to get an InstallContext. I wasn't sure
> > > >> how to get Also= targets in any other way.
> > > >> 
> > > >> If such change makes sense, this patch should probably be considered a
> > > >> preview rather than something to be committed right away.
> > > >
> > > > Hmm, wouldn't it be nicer to introduce a new UnitFileState enum value
> > > > for this?
> > > >
> > > > Maybe UNIT_FILE_ALSO or so? 
> > > >
> > > > I am not sure I like the idea of implicitly following the Also= setting here, due
> > > > to the awkwarndess if multiple units are listed and how to map exotic
> > > > states of that other unit back to ours...
> > > >
> > > > Would that make sense?
> > > >
> > > > Lennart
> > > 
> > > Yes, that makes sense. What should a string representation of
> > > UNIT_FILE_ALSO be? I don't think that reporting 'also' would feel
> > > right.
> Maybe I'm missing something, but wouldn't be enough to report is as 'enabled'?
> For some units adding another name from Also= really enables the unit,
> and for other units the name from Also= is mostly cosmetic. What I'm
> trying to say is that having or not the Also= name enabled is only approximate
> information and does not always tell us if the unit will be started.

Also= isn't necessarily symmetric. If you have a service A and a
service B, then it might very well be that B has Also=A.service, but
not the other way round. I think there are only two strategies here:

a) if a unit has Also= set, determine the state of the unit files
   listed in it, any propagate that. This is what Jan's patch did
   originally. But I am not sure I like this propagation since it
   leaves so many open questions regarding correct propagation the
   precise state and what to do if multiple units are listed.

or

b) just report that Also= is set, which is what I was proposing. This
   might not be particularly self-explanatory, but maybe we can
   explain this in the man page or so...

> 
> I'd prefer to redefine enabled/disabled/static as "this unit has at
> least on of the declared links in the filesystem/the unit does not
> have any defined links in the filesystem/this unit does not declare
> any links in the filesystem", which is something that we can actually
> check.

So, are you proposing to follow strategy a) then?

Lennart

-- 
Lennart Poettering, Red Hat