Re: [PATCH 2/3] archive: implement protocol v2 archive command

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 13, 2018, 18:45 UTC
Message-ID: <87k1npkzh5.fsf@evledraar.gmail.com>
In-Reply-To: <CAGZ79kZOTsUH=zQX3rLXvuSOx1vp8C98maSn47ssfca8c-BrBQ@mail.gmail.com>

On Wed, Sep 12 2018, Stefan Beller wrote:

Show 16 quoted lines

> On Tue, Sep 11, 2018 at 10:36 PM Josh Steadmon <steadmon@google.com> wrote:
>> +                */
>> +               status = packet_reader_read(&reader);
>> +       }
>> +       if (status != PACKET_READ_DELIM)
>> +               die(_("upload-archive: expected delim packet"));
>
> This is upload-archive, which is a low level plumbing command
> (see the main man page of git for an explanation of that category),
> so we do not translate the error/die() calls. Besides, this is executed
> on the server, which might have a different locale than the requesting
> client?
>
> Would asking for a setlocale() on the server side be an unreasonable
> feature request for the capabilities (in a follow up patch, and then not
> just for archive but also fetch/push, etc.)?

This would be very nice to have, but as you suggest in some follow-up change.

I think though that instead of doing setlocale() it would be better to pass some flag saying we're operating in a machine-readable mode, and then we'd (as part of the protocol defintion) say we're going to emit GIT_ERR_UPLOAD_ARCHIVE_EXPECTED_DELIM_PACKET or whatever.

Advantages of doing that over a server-side setlocale():

 1) Purely for translation purposes, users can update to a newer client
    to get new translations, even though they're talking to an old
    server.

 2) Again, only for translation purposes, servers may not have the
    appropriate locales generated and/or linked to libgettext.

 3) Ditto, some clients that aren't git.git may want/need to emit
    different translation messages to their consumers than what we have,
    think some GUI client / Emacs magit etc. whose UI is different from
    ours.

 4) Aside from translation purposes, getting a machine-readable
    "push/pull" etc. mode would be very handy. E.g. now you need to
    parse stderr to see why exactly your push failed (hook denied, or
    non-fast-forward, or non-fast-forward where there was a lock race
    condition? ...).

I also wonder if something like #4 wouldn't compliment something like the proposed structured logging[1]. I.e. even though we'd like to run git.git, and present exactly the message to the user we do now, we might want to run in such a machine-readable mode under the hood when talking to the server so we can log exactly how the push went / how it failed for the purposes of aggregation.

1. https://public-inbox.org/git/20180713165621.52017-2-git@jeffhostetler.com/