Re: [PATCH 2/5] fast-import: support 'encoding' commit header

[Elijah Newren <newren@gmail.com>]

Hi Eric,

On Thu, Apr 25, 2019 at 1:37 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, Apr 25, 2019 at 11:51 AM Elijah Newren <newren@gmail.com> wrote:
> > Since git supports commit messages with an encoding other than utf-8,
> > allow fast-import to import such commits.  This may be useful for folks
> > who do not want to reencode commit messages from an external system, and
> > may also be useful to achieve reversible history rewrites (e.g. sha1sum
> > <-> sha256sum transitions or subtree work) with git repositories that
> > have used specialized encodings in their commit history.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> > diff --git a/fast-import.c b/fast-import.c
> > @@ -2607,6 +2608,9 @@ static void parse_new_commit(const char *arg)
> >         if (!committer)
> >                 die("Expected committer but didn't get one");
> > +       if (skip_prefix(command_buf.buf, "encoding ", &encoding)) {
> > +               read_next_command();
> > +       }
>
> Style nit: unnecessary braces
>
> > @@ -2670,9 +2674,13 @@ static void parse_new_commit(const char *arg)
> >         strbuf_addf(&new_data,
> >                 "author %s\n"
> > -               "committer %s\n"
> > -               "\n",
> > +               "committer %s\n",
> >                 author ? author : committer, committer);
> > +       if (encoding)
> > +               strbuf_addf(&new_data,
> > +                       "encoding %s\n",
> > +                       encoding);
> > +       strbuf_addf(&new_data, "\n");
>
> Alternately:
>
>     strbuf_addch(&new_data, '\n');

Thanks for taking a look.  I'll fix both of these items you
highlighted and the test_config item you pointed out in 1/5 in the
next re-roll.

> > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> > @@ -3299,4 +3299,24 @@ test_expect_success !MINGW 'W: get-mark & empty orphan commit with erroneous thi
> > +test_expect_success 'X: handling encoding' '
> > +       test_tick &&
> > +       [...]
> > +       git cat-file -p encoding | grep $(printf "\360") &&
> > +       git log -1 --format=%B encoding | grep $(printf "\317\200")
>
> This script is already full of instances of Git commands upstream of
> pipes, so this usage is consistent (despite recent work to eliminate
> such situations). Okay.