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

Jan Synacek jsynacek at redhat.com
Fri Nov 7 06:26:28 PST 2014


Lennart Poettering <lennart at poettering.net> writes:
> 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.
>
> We should always keep the enum name and the string it translates to in
> sync. I can see that reporting "also" might be confusing, note sure
> what we could name it better though. But if we use a different string
> we should also rename its enum really.
>
> Maybe "indirect"? "other"? Hmm, "see-also" could work? With the
> counterpart UNIT_FILE_SEE_ALSO?
>
>> Also, is there a better way to find out if unit has any Also= targets
>> than how I did it?
>
> I think the best way would be to extend InstallInfo to get a new "also"
> strv field, that is upated by config_parse_also(). Then
> unit_file_can_install() can find it and return the fact that the list
> is not empty in an extra parameter.

Thanks for the pointers! I've resubmitted the patch and went with the
"indirect" version, as it felt the best to me.

-- 
Jan Synacek
Software Engineer, Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20141107/70694096/attachment.sig>


More information about the systemd-devel mailing list