Re: [PATCH] daemon: add --no-syslog to undo implicit --syslog

From: Lucas Werkmeister <mail@lucaswerkmeister.de>
Date: Jan 23, 2018, 22:06 UTC
Message-ID: <18ef807d-a642-17b2-fc32-af0c3b963a71@lucaswerkmeister.de>
In-Reply-To: <xmqqh8rcig0x.fsf@gitster.mtv.corp.google.com>

Thanks for your responses!

On 23.01.2018 01:00, Ævar Arnfjörð Bjarmason wrote:

Show 7 quoted lines

> This patch looks good, but I wonder if with the rise of systemd
> there's a good reason to flip the default around to not having other
> stuff imply --syslog, and have users specify this implictly, then we
> won't need a --no-syslog option.
> 
> But maybe that'll break too much stuff.
>

That seems risky to me – even with systemd, the StandardError directive by default inherits StandardOutput, so if you set StandardOutput=socket without StandardError=journal, log output in stderr clobbers regular output. (Also, stderr is apparently closed with --detach, see below.)

On 23.01.2018 19:30, Junio C Hamano wrote:

Show 12 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
>> On Mon, Jan 22 2018, Lucas Werkmeister jotted:
>> 
>>> Several options imply --syslog, without there being a way to
>>> disable it again. This commit adds that option.
>> 
>> Just two options imply --syslog, --detach & --inetd, unless I've
>> missed something, anyway 2 != several, so maybe just say "The
>> --detach and --inetd options imply --syslog ...".
> 
> Correct.

I respectfully disagree on “2 != several”, but sure, I can repeat the two options in the message instead :)

Show 5 quoted lines

> Moreover, --detach completely dissociates the process from the
> original set of standard file descriptors by first closing them and
> then connecting it to "/dev/null", so it will be nonsense to use this
> new option with it.
>

Ah, I wasn’t aware of that – so with --detach, --no-syslog would be better described as “disables all logging” rather than “log to stderr instead”. IMHO it would still make sense to have the --no-syslog option (then mainly for use with --inetd) as long as its interaction with --inetd is properly documented… do you agree? If yes, I’ll be glad to submit another version of the patch.

Best regards, Lucas