Re: Git repo problem?

On Thu, Jun 13, 2019 at 6:05 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> Greetings,
>
> * Tatsuo Ishii (ishii(at)sraoss(dot)co(dot)jp) wrote:
> > > That sounds a lot like the bug that Stephen bumped into during pgcon,
> and
> > > that we haven't fully tracked down yet. It only makes the commit email
> > > break, everything else should work as usual, so it's not that bad, but
> it
> > > should be fixed of course.
> > >
> > > Did that push by any chance include a merge commit? That's our current
> > > theory on what's triggering the problem...
> >
> > Yes. I pushed:
> >
> https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=b9ee8417ea74798c108498849d453e693246c3a1
> > and
> >
> https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=d1a2e366afeee87493db3463baa6660f9671e5d6
> >
> > The latter is a merge commit.
>
> It turns out that the issue seems to be when there is no diffstat for a
> given commit and the last line read is appended back to the list:
>
> if not l.startswith(" "):
> # If there is no starting space, it means there were no stats
> rows,
> # and we're already looking at the next commit. Put this line
> back
> # on the list and move on
> lines.append(l)
> break
>
> This ends up adding a regular str (and not a byte string) to the list,
> which blows up later when we try to do a 'decode' on it, in the next
> pass.
>
> Instead, I believe this should be:
>
> if not l.startswith(" "):
> # If there is no starting space, it means there were no stats
> rows,
> # and we're already looking at the next commit. Put this line
> back
> # on the list and move on
> lines.append(l.encode('utf-8'))
> break
>
> Local testing showed that to fix it, but I'd like to give Magnus an
> opportunity to review and confirm that this fix makes sense before
> changing things on the git server.
>

Nice spot. I think you have at least found the issue, but I think it may
not be the best fix. Given that when we decode the string we do it with
errors=ignore, we might loose data. Does the attached patch fix it in your
tests as well? Instead of encoding/recoding, it just sticks the old line
back on the list (which also matches the comment).

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>