Re: [PATCH v2 3/8] checkout-index: add parallel checkout support

[Matheus Tavares Bernardino <matheus.bernardino@usp.br>]

On Sat, May 1, 2021 at 2:08 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Fri, Apr 30, 2021 at 11:40 PM Matheus Tavares
> <matheus.bernardino@usp.br> wrote:
> >
> > Note: previously, `checkout_all()` would not return on errors, but
>
> s/Note: previously/Previously/
>
> > instead call `exit()` with a non-zero code. However, it only did that
> > after calling `checkout_entry()` for all index entries, thus not
> > stopping on the first error, but attempting to write the maximum number
> > of entries possible. In order to maintain this behavior we now propagate
> > `checkout_all()`s error status to `cmd_checkout_index()`, so that it can
> > call `run_parallel_checkout()` and attempt to write the queued entries
> > before exiting with the error code.
> >
> > Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
>
> > @@ -142,11 +143,7 @@ static void checkout_all(const char *prefix, int prefix_length)
> >         }
> >         if (last_ce && to_tempfile)
> >                 write_tempfile_record(last_ce->name, prefix);
> > -       if (errs)
> > -               /* we have already done our error reporting.
> > -                * exit with the same code as die().
> > -                */
> > -               exit(128);
>
> So when there were errors in checkout_all(), we used to exit() with
> error code 128 (same as die())...
>
> > @@ -275,12 +277,16 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
> >                 strbuf_release(&buf);
> >         }
> >
> > +       if (all)
> > +               err |= checkout_all(prefix, prefix_length);
> > +
> > +       if (pc_workers > 1)
> > +               err |= run_parallel_checkout(&state, pc_workers, pc_threshold,
> > +                                            NULL, NULL);
> > +
> >         if (err)
> >                 return 1;
>
> ...but now it looks like we will exit with error code 1. I see that
> you already answered this comment in the previous round of review, but
> you didn't add the explanations to the commit message.

Oops, good catch! I'll add the explanation for v3. Thanks.