Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 541880 - dev-vcs/git-2.3.1: QA Notice: systemd units using /etc/conf.d detected
Summary: dev-vcs/git-2.3.1: QA Notice: systemd units using /etc/conf.d detected
Status: CONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Development (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Robin Johnson
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-02 10:43 UTC by Ilya Gordeev
Modified: 2016-05-08 23:07 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Gordeev 2015-03-02 10:43:25 UTC
Installing dev-vcs/git-2.3.1 emerge prints this message:
QA: other
QA Notice: systemd units using /etc/conf.d detected:
usr/lib/systemd/system/git-daemon@.service:EnvironmentFile=/etc/conf.d/git-daemon
See: https://wiki.gentoo.org/wiki/Project:Systemd/conf.d_files

Reproducible: Always
Comment 1 William Hubbs gentoo-dev 2015-07-29 19:32:18 UTC
The same applies for 2.4.6.
Comment 2 Mike Gilbert gentoo-dev 2016-05-08 20:01:26 UTC
commit 843954e66426a8e87c42a2c3d3d99ca6fa7d4744
Author: Mike Gilbert <floppym@gentoo.org>
Date:   Sun May 8 15:59:36 2016 -0400

    dev-vcs/git: for systemd, don't source /etc/conf.d/git-daemon
    
    Package-Manager: portage-2.2.28_p97

 dev-vcs/git/files/git-daemon_at.service | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Comment 3 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2016-05-08 21:45:04 UTC
@floppym, systemd:
Thanks, you just at the last disabled syslog for everybody that used systemd git-daemon, and at worst broke their git daemons (eg if they used --listen) and potentially exposed.

There should be some alternative config file at the least with the same configuration in it.
Comment 4 Mike Gilbert gentoo-dev 2016-05-08 22:24:40 UTC
(In reply to Robin Johnson from comment #3)
> @floppym, systemd:
> Thanks, you just at the last disabled syslog for everybody that used systemd
> git-daemon, and at worst broke their git daemons (eg if they used --listen)
> and potentially exposed.

git-daemon@.service passes the --inetd option to git-daemon.

Quoting the git-daemon manual:

       --inetd
           Have the server run as an inetd service. Implies --syslog. Incompatible with --detach,
           --port, --listen, --user and --group options.

So, --syslog is already enabled implicitly.

The --listen option is incompatible with socket-activation.

So no, I don't think I broke anything here.
Comment 5 Mike Gilbert gentoo-dev 2016-05-08 22:33:57 UTC
(In reply to Robin Johnson from comment #3)
> There should be some alternative config file at the least with the same
> configuration in it.

That's really not systemd units are designed to work. If someone wants to customize the git-daemon command line, they already have a couple options:

1. Copy the unit to /etc/systemd/system and edit the ExecStart line.

2. Create a drop-in config under /etc/systemd/system/git-daemon@.service.d/ .

EnvironmentFile is supposed to be used for setting environment variables, not for adding options to ExecStart.
Comment 6 Richard Freeman gentoo-dev 2016-05-08 22:51:08 UTC
Mike, the main problem with those approaches is that if you want to modify execstart you basically have to fork the entire unit, which means that you no longer benefit from any Gentoo changes in /usr on subsequent updates.

With an environment variable some kind of configuration item that is commonly adjusted can be changed without having to fork the unit.

Sure, it would be nice if upstream allowed all settings to be passed in the environment and not just on the command line, but that isn't always the case.

I guess you could still put an environment variable in the unit's execstart and then just use an add-on to set it.  I'm not sure what that really buys you beyond what you'd get from using the conf.d file as an environment file, and it makes the configuration non-portable across the two service managers.
Comment 7 Mike Gilbert gentoo-dev 2016-05-08 22:59:19 UTC
commit df94d5913967eb5527fc5b1b8af8434659c794af
Author: Mike Gilbert <floppym@gentoo.org>
Date:   Sun May 8 18:56:31 2016 -0400

    dev-vcs/git: reapply systemd unit changes in ~arch only
    
    Package-Manager: portage-2.2.28_p97

 dev-vcs/git/files/git-daemon_at-r1.service              | 12 ++++++++++++
 dev-vcs/git/{git-9999-r3.ebuild => git-2.8.2-r1.ebuild} |  2 +-
 dev-vcs/git/git-9999-r1.ebuild                          |  2 +-
 dev-vcs/git/git-9999-r2.ebuild                          |  2 +-
 dev-vcs/git/git-9999-r3.ebuild                          |  2 +-
 dev-vcs/git/git-9999.ebuild                             |  2 +-
 6 files changed, 17 insertions(+), 5 deletions(-)

commit e3d1d95bd9a6935fddfb0f4253d784c1b72564d3
Author: Mike Gilbert <floppym@gentoo.org>
Date:   Sun May 8 18:47:28 2016 -0400

    Revert "dev-vcs/git: for systemd, don't source /etc/conf.d/git-daemon"
    
    This reverts commit 843954e66426a8e87c42a2c3d3d99ca6fa7d4744.

 dev-vcs/git/files/git-daemon_at.service | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 8 Mike Gilbert gentoo-dev 2016-05-08 23:07:47 UTC
(In reply to Richard Freeman from comment #6)
> Mike, the main problem with those approaches is that if you want to modify
> execstart you basically have to fork the entire unit, which means that you
> no longer benefit from any Gentoo changes in /usr on subsequent updates.

We are talking about a simple declarative config file, not a shell script that might be changed between versions. You are not really losing much by forking the file locally.

> With an environment variable some kind of configuration item that is
> commonly adjusted can be changed without having to fork the unit.

systemd upstream is very much opposed to this kind of usage. systemd units are not supposed to change behavior like that.

> I guess you could still put an environment variable in the unit's execstart
> and then just use an add-on to set it.  I'm not sure what that really buys
> you beyond what you'd get from using the conf.d file as an environment file,
> and it makes the configuration non-portable across the two service managers.

We have a policy against using /etc/conf.d in EnvironmentFile mainly because those files are allowed to contain arbitrary shell code. There is no standard format for those files and they just happen to work by accident in many cases.

I realize that EnvironmentFile=/etc/conf.d/... works in 90% of cases, but I would prefer to do things the "upstream way" when possible.