[systemd-devel] [PATCH] sysv-generator: Handle .sh suffixes when translating Provides:

[Martin Pitt]

Hey Lennart,

Lennart Poettering [2015-01-27 23:09 +0100]:
> Hmm, we already had code that checks this in place, didn#t we?

We only previously had this .sh stripping in sysv_translate_name(),
but that translates and init script name like "foo.sh" to
"foo.service". But that's not what we need in
sysv_translate_facility() to filter out the Provides: item which
refers to itself.

> I mean sysv_translate_facility() already filters out the case where
> the service name is identical to the provided name. Hence, why do you
> need a second check for this?

It's not really a new check, we just fixed the existing one:

-        } else if (filename && streq(name, filename))
+        } else if (filename && streq(name, filename_no_sh))

I. e. if filename ends with ".sh", the streq wouldn't previously match
as Provides: names don't have the ".sh" suffixes. I. e. this is the
case where you encounter the item in Provides: which refers to its own
name, not to other scripts. We should not create an alias symlink for
that, as it would refer to itself.

> I wonder if the simple fix could just be to change this:
> 
>         } else if (filename && streq(name, filename))
> 
> to this
> 
>         } else if (filename && streq(n, filename))
> 
> Or so, in that function?

That particularly wouldn't help, as both "n" and "name" are "foo" here
in that context. I. e. n and name are both the value of the currently
processed "Provides:" list item (just that n strips off $, but that's
irrelevant here). But "filename" has the .sh suffix while "Provides:"
names never have .sh suffixes, so that comparison fails and without
the patch it would go on and create a "foo.service -> foo.service"
alias symlink.

This case has been mitigated in two other places, so you need to
revert both 77354c7e and the extra sanity check in add_alias() of
commit 29e0e6d to see it again. So you could say that we don't need
the filename_no_sh comparison in sysv_translate_facility() as the new
check in add_alias() as well as 29e0e6d will save us too, but this
would still feel unclean to me.

Thanks,

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)