[systemd-devel] [PATCH] sysv-generator: handle Provides: for non-virtual facility names

[Zbigniew Jędrzejewski-Szmek]

On Mon, Jan 05, 2015 at 10:30:11AM +0100, Michael Biebl wrote:
> See attached patch.
> 
> The commit message reads:
> 
> The list of provided facility names as specified via Provides: in the
> LSB header was originally implemented by adding those facilities to the
> Names= property via unit_add_name().
> 
> In commit 95ed3294c632f5606327149f10cef1eb34422862 the internal SysV
> support was replaced by a generator and support for parsing the Names=
> option had been removed from the unit file parsing in v186.
> As a result, Provides: for non-virtual facility was dropped when
> introducing the sysv-generator.
> 
> Since quite a few SysV init scripts still use that functionality (at
> least in distros like Debian which have a large body of SysV init
> scripts), add back support by making those facility names available via
> symlinks to the unit filename to ensure correct orderings between
> SysV init script which use those facility names.
> 
> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=774335
> 
> 
> -- 
> Why is it that all of the instruments seeking intelligent life in the
> universe are pointed away from Earth?

> From a1bc03cabf26d05aeb725fc549a74312f634eaba Mon Sep 17 00:00:00 2001
> From: Michael Biebl <biebl at debian.org>
> Date: Mon, 5 Jan 2015 09:49:44 +0100
> Subject: [PATCH] sysv-generator: handle Provides: for non-virtual facility
>  names
> 
> The list of provided facility names as specified via Provides: in the
> LSB header was originally implemented by adding those facilities to the
> Names= property via unit_add_name().
> 
> In commit 95ed3294c632f5606327149f10cef1eb34422862 the internal SysV
> support was replaced by a generator and support for parsing the Names=
> option had been removed from the unit file parsing in v186.
> As a result, Provides: for non-virtual facility was dropped when
> introducing the sysv-generator.
> 
> Since quite a few SysV init scripts still use that functionality (at
> least in distros like Debian which have a large body of SysV init
> scripts), add back support by making those facility names available via
> symlinks to the unit filename to ensure correct orderings between
> SysV init script which use those facility names.
> 
> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=774335
> ---
>  src/sysv-generator/sysv-generator.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
> index 6730843..f78ddeb 100644
> --- a/src/sysv-generator/sysv-generator.c
> +++ b/src/sysv-generator/sysv-generator.c
> @@ -112,6 +112,27 @@ static int add_symlink(const char *service, const char *where) {
>          return 1;
>  }
>  
> +static int add_alias(const char *service, const char *alias) {
> +        _cleanup_free_ char *link = NULL;
> +        int r;
> +
> +        assert(service);
> +        assert(alias);
> +
> +        link = strjoin(arg_dest, "/", alias, NULL);
> +        if (!link)
> +                return log_oom();
path_join()?

> +
> +        r = symlink(service, link);
Those will be dangling links, no? They will work with current implementation,
but it would be nicer to create a working symlink.


> +        if (r < 0) {
> +                if (errno == EEXIST)
> +                        return 0;
> +                return -errno;
> +        }
> +
> +        return 1;
> +}
> +
>  static int generate_unit_file(SysvStub *s) {
>          char **p;
>          _cleanup_fclose_ FILE *f = NULL;
> @@ -457,7 +478,9 @@ static int load_sysv(SysvStub *s) {
>                                          if (r == 0)
>                                                  continue;
>  
> -                                        if (unit_name_to_type(m) != UNIT_SERVICE) {
> +                                        if (unit_name_to_type(m) == UNIT_SERVICE) {
> +                                                r = add_alias(s->name, m);
> +                                        } else {
>                                                  /* NB: SysV targets
>                                                   * which are provided
>                                                   * by a service are

Looks reasonable otherwise.

Zbyszek