[PATCH 2 of 2 STABLE] py3: use native strings when forming email headers in patchbomb

[Denis Laxalde]

Yuya Nishihara a écrit :
> On Mon, 28 Oct 2019 15:16:49 -0400, Augie Fackler wrote:
> > On Mon, Oct 28, 2019 at 11:57:38PM +0900, Yuya Nishihara wrote:
> > > On Mon, 28 Oct 2019 10:09:22 +0100, Denis Laxalde wrote:
> > > > Yuya Nishihara a écrit :
> > > > > On Fri, 25 Oct 2019 15:02:11 +0200, Denis Laxalde wrote:
> > > > > > # HG changeset patch
> > > > > > # User Denis Laxalde <denis at laxalde.org>
> > > > > > # Date 1572008488 -7200
> > > > > > #      Fri Oct 25 15:01:28 2019 +0200
> > > > > > # Branch stable
> > > > > > # Node ID 6058175493d062dbaec2cb0e897e49da4596d147
> > > > > > # Parent  17fad3f1b4742d75db23e17a04c85223bcc239a2
> > > > > > py3: use native strings when forming email headers in patchbomb
> > > > >
> > > > > I feel this change is too big to be in stable.
> > > > >
> > > > > > -        # Fix up all headers to be native strings.
> > > > > > -        # TODO(durin42): this should probably be cleaned up above in the future.
> > > > > > -        if pycompat.ispy3:
> > > > > > -            for hdr, val in list(m.items()):
> > > > >
> > > > > Can't we somehow get around the issue here?
> > > >
> > > > Well, I'm not sure I'd be interested in implementing a workaround,
> > > > especially here since the change aims at removing this TODO while making
> > > > the code Python 3 compatible. Fixing the issue is a side effect somehow.
> > >
> > > Okay, then let's move the series to default branch if you aren't interested
> > > in minimizing the stable changes.
> > >
> > > There are too much py3 changes after the freeze.
> > 
> > I don't love this for stable, but I think I'd rather have it (in 5.2.1
> > maybe?) as part of our "works on py3" release than punt it to 5.3.
> 
> I looked this patch more closely, and I'm getting to think patchbomb works
> on py3 without this patch. Most (all?) header values are "encoded", which
> should be ASCII and strurl() wouldn't crash.
> 
> So, -1 for this change. Can you at least add some tests that would fail
> without this change?

The test introduced in patch 1 of the series fails on Python 3 without
this change (a subject header with non-ascii characters). This crashes
in strurl(). On second thought, I can fix that with minimal changes, so
I'll send a follow up on patch 1.

Nevertheless, I still think this change here is worth. I'm fine if it
goes into default or if it waits after the release. Just let me know the
plan.