<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 27, 2017 at 6:54 AM, Yuya Nishihara <span dir="ltr"><<a href="mailto:yuya@tcha.org" target="_blank">yuya@tcha.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Sun, 26 Mar 2017 18:36:41 -0400, Augie Fackler wrote:<br>
> # HG changeset patch<br>
> # User Augie Fackler <<a href="mailto:raf@durin42.com">raf@durin42.com</a>><br>
> # Date 1490567324 14400<br>
> #      Sun Mar 26 18:28:44 2017 -0400<br>
> # Node ID e7fe3ab60132647a9c3b86b2960b38<wbr>5e61b9dcf0<br>
> # Parent  48144fe2d912b7d9fc300955d0c881<wbr>aceead6930<br>
> revlog: fix many file handle opens to explicitly use bytes mode<br>
><br>
> This broke on Python 3, but we've probably been getting lucky that<br>
> this hasn't caused EOL corruption on Windows or something for<br>
> years. :(<br>
><br>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py<br>
> --- a/mercurial/revlog.py<br>
> +++ b/mercurial/revlog.py<br>
> @@ -1467,8 +1467,8 @@ class revlog(object):<br>
><br>
>          dfh = None<br>
>          if not self._inline:<br>
> -            dfh = self.opener(self.datafile, "a+")<br>
> -        ifh = self.opener(self.indexfile, "a+", checkambig=self._checkambig)<br>
> +            dfh = self.opener(self.datafile, "a+b")<br>
> +        ifh = self.opener(self.indexfile, "a+b", checkambig=self._checkambig)<br>
<br>
</span>'b' shouldn't be needed since vfs adds 'b' unless text=True is explicitly<br>
specified.<br>
</blockquote><div><br></div><div>That "text" argument always seemed weird to me because all it does is reinvent the wheel for the "mode" argument to open(). I'd prefer we deprecate the argument and use explicit mode values so the I/O code more closely aligns with what Python does. Principal of least surprise.<br><br></div><div>That being said, it is a big BC and I'm not sure we can change it easily.<br><br>I do like Augie's patch to add "b" because it results in explicit code, even if it is redundant. Although, I can see how some call sites specifying "b" and others not with both having the same behavior (without "text=True") could be confusing. That's a nice corner we painted ourselves into :/<br></div></div></div></div>