On Thu, Feb 8, 2018 at 12:22 PM Kenneth Porter <sh...@sewingwitch.com> wrote:
> On 2/8/2018 4:35 AM, Daniel Shahaf wrote: > > So, just to be clear, the problem is that svn/fs.py is not py3 > > compatible, and having the 'builtins' module under py2 merely > > exposes that? I.e., we have no reason to suspect a bug in the 'future' > > package's implementation of builtins.open() under py2. > > That's my interpretation. As I said at the start of the thread, it was > never clear why the internal temporary file was opened in text mode and > update mode when the code was first written. > > I committed the fix to the bindings in https://svn.apache.org/viewvc?view=revision&revision=1823802 . In addition to Kenneth's suggestion of opening in binary mode, I switched the imports so that the python2-future's implementation would not get inadvertently pulled in. Everything looked fine with the how python2-future's open worked (since it did in fact use the Python 3's open() semantics), but I think it best that on the intended modules are included. I also added a test which duplicated the issue (with python2-future installed at least), and confirms the fix. This is a relatively isolated change, but fixes surprising behavior (as Kenneth can attest to), does something like this make sense to propose for the 1.10 branch? Troy