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

[Zbigniew Jędrzejewski-Szmek]

On Thu, Sep 25, 2014 at 10:09:40AM +0200, Lukáš Nykrýn wrote:
> 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.
OK.

> > > -                        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.
I don't see this extra parameter as a problem. Getting the root_dir
semantics right has proven to be really hard, so it is important to
keep the number of places where we calculcate the path to minimum.
If you have two functions, you'll have to add an if to pick which one
to call in multiple places, and duplicate the logic in both functions,
but the whole point was to avoid that.

> > > +               "                                  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