git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] midx: use buffered I/O to talk to pack-objects

From
René Scharfe <l.s.r@web.de>
Date
Aug 4, 2020, 04:31 UTC
Message-ID
<90b839cd-29e0-3d20-8661-a2a73eff9ced@web.de>
In-Reply-To
<fdf66f66-b7ed-f733-22d0-23ec17209f40@web.de>
Am 04.08.20 um 00:27 schrieb René Scharfe:
Show 24 quoted lines
> Am 03.08.20 um 20:10 schrieb Johannes Sixt:
>> Am 02.08.20 um 18:11 schrieb Chris Torek:
>>> On Sun, Aug 2, 2020 at 7:40 AM René Scharfe <l.s.r@web.de> wrote:
>>>> @@ -1443,10 +1446,15 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
>>>>                         continue;
>>>>
>>>>                 nth_midxed_object_oid(&oid, m, i);
>>>> -               xwrite(cmd.in, oid_to_hex(&oid), the_hash_algo->hexsz);
>>>> -               xwrite(cmd.in, "\n", 1);
>>>> +               fprintf(cmd_in, "%s\n", oid_to_hex(&oid));
>>>> +       }
>>>> +
>>>> +       if (fclose(cmd_in)) {
>>>> +               error_errno(_("could not close stdin of pack-objects"));
>>>> +               result = 1;
>>>> +               finish_command(&cmd);
>>>> +               goto cleanup;
>>>>         }
>>>> -       close(cmd.in);
>>>>
>>>>         if (finish_command(&cmd)) {
>>>>                 error(_("could not finish pack-objects"));
>>>> --
>>>> 2.28.0
Show 8 quoted lines
>> We need this explicit test after each fprintf anyway because SIGPIPE may
>> be ignored, and then writing fails with EPIPE. On Windows, this is
>> doubly important because we do not have SIGPIPE at all (and always see
>> EPIPE), but we see EPIPE only on the first failed write; subsequent
>> writes produce EINVAL.
>
> Why is this important?  The current code doesn't care about it, at
> least.  It does care about EINTR, though.

Ah, that's the point, right? You want to *ignore* EPIPE, because the failed pack-objects process at the other end will have died with a (hopefully) useful error message already.

OK, so we also need a fprintf() wrapper that retries on EINTR, ignores EPIPE and exits early if the error indicator is set?

Somehow staying with write(2) and its friends and just adding strbuf based buffering looks attractive to me now. :-/

René
Previous: René ScharfeNext: Junio C Hamano
Message 5 of 14 in “midx: use buffered I/O to talk to pack-objects”
  1. midx: use buffered I/O to talk to pack-objectsRené Scharfe, Aug 2, 2020
  2. Chris TorekAug 2, 2020
  3. Johannes SixtAug 3, 2020
  4. René ScharfeAug 3, 2020
  5. René ScharfeAug 4, 2020
  6. Junio C HamanoAug 4, 2020
  7. Derrick StoleeAug 3, 2020
  8. René ScharfeAug 11, 2020
  9. Derrick StoleeAug 11, 2020
  10. midx: use buffered I/O to talk to pack-objectsRené Scharfe, Aug 12, 2020
  11. Junio C HamanoAug 12, 2020
  12. Junio C HamanoAug 12, 2020
  13. Jeff KingAug 13, 2020
  14. Jeff KingAug 13, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.