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

Yuya Nishihara yuya at tcha.org
Tue Oct 29 19:29:10 EDT 2019


On Tue, 29 Oct 2019 18:09:11 +0100, Denis Laxalde wrote:
> 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.

Ah, right. The test fails because of --test, which disables headencode().
Then, I think s/pycompat.strurl(val)/encoding.strfromlocal(val)/ is the
simplest fix.

> 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.

I also want to remove this TODO, but I don't think inserting many
strfromlocal()s is the best way to do that. Maybe we'll need some helper
to cope with py3's new mail API?


More information about the Mercurial-devel mailing list