Re: [PATCH v5 06/15] index-helper: add --detach

From: David Turner <dturner@twopensource.com>
Date: Apr 25, 2016, 20:53 UTC
Message-ID: <1461617586.24444.4.camel@twopensource.com>
In-Reply-To: <CACsJy8Bcb=-V0wc8En2SCSz8jPefEL4qibxJsLsTB-c94x9y3Q@mail.gmail.com>

On Wed, 2016-04-20 at 06:50 +0700, Duy Nguyen wrote:

Show 18 quoted lines

> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <
> dturner@twopensource.com> wrote:
> > @@ -317,6 +320,8 @@ int main(int argc, char **argv)
> >         if (fd < 0)
> >                 die_errno(_("could not set up index-helper
> > socket"));
> > 
> > +       if (detach && daemonize(&daemonized))
> > +               die_errno(_("unable to detach"));
> 
> At the least, I think we need to redirect both stdout and stderr to a
> file, so we can catch errors. The watchman patch uses warning() to
> report errors, for example. And there is always a chance of die().
> 
> Then we need to report the errors back. I faced the same problem with
> daemonizing git-gc, but I'm not sure if we can do exactly the same
> here like in commit 329e6e8 (gc: save log from daemonized gc --auto
> and print it next time - 2015-09-19)

On reflection, I decided not to do a complicated system for replaying warnings. Here are my reasons why:

1. A user will not be expecting to see warnings from previous git
commands.

2. It's not super-important that users see most of these warnings.  GC
is different because it's critical to the health of a repository so it
really matters that users learn about issues.

3. It involves many complications to the (presently very simple)
protocol. We would need to distinguish between messages, warnings, and
previous-session warnings.

4. There are only a few cases where errors will happen, and none seem
to be exciting to clients.

Instead, I'll just log errors.