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

From: David Turner <dturner@twopensource.com>
Date: Apr 20, 2016, 01:04 UTC
Message-ID: <1461114267.5540.161.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)

I'll add in code to log errors. I'm not sure where it would make sense to report the errors. Generally, for errors during a client operation, we would like to report them to the client, but the client might have already disconnected. I guess in that case it's OK if they just go to the log? The client could warn on a timeout while waiting for index -helper and direct people to the log.