[systemd-devel] [PATCHi v2] systemctl: add add-wants and add-requires verbs

Lukáš Nykrýn lnykryn at redhat.com
Thu Sep 25 01:09:40 PDT 2014


Zbigniew Jędrzejewski-Szmek píše v St 24. 09. 2014 v 18:15 +0200:
> 
> I don't understand the name "method_add_install_dependency_unit_files".
> Why not just "method_add_dependencies"? After all, this is not like install,
> and does not work on the level of unit files, but units.
> 
> Similarly for the dbus method name "AddInstallDependencyUnitFiles".
>   
This works with unit files, similarly as 'enable', so I would really
like to keep the "UnitFiles" part there. And the same for "Install"
part, I wanted to make it obvious that this is somehow an alternative to
[Install] section in unit file.

> > -                        r = unit_file_load(c, info, path, root_dir, allow_symlink);
> > +                        if (load)
> > +                                r = unit_file_load(c, info, path, root_dir, allow_symlink);
> > +                        else
> > +                                r = access(path, F_OK) ? -errno : 0;
> Wouldn't it be nicer to push the 'load' parameter into
> unit_file_load(), and do the check there? I think that with
> your patch the support for root_dir is missing.

Yep you are right, this will not work with --root, but I don't think
that we want to have function unit_file_load(...., bool really_load).
Maybe it would be better to add function unit_file_exists.

> > +               "                                  based on preset configuration\n"
> Extra line now? ;)

for (int i=0; i<100; i++) puts("Will double check my patches before
sending them");

> Zbyszek


But really thanks for review.

Lukas



More information about the systemd-devel mailing list