On Sat, Feb 10, 2018 at 8:28 PM Troy Curtis Jr <troycurtisjr_at_gmail.com>
wrote:
> On Thu, Feb 8, 2018 at 12:22 PM Kenneth Porter <shiva_at_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?
>
>
https://ci.apache.org/builders/svn-windows-ra/builds/1998
It did occur to me at one point, "I wonder how well this works on Windows",
since it shells out to external 'diff' command. I presume fs.FileDiff has
never worked on Windows (well presumably it would if a compatible 'diff'
command was in the PATH), but there was never a test for it previously.
I believe that Subversion has an internal diff generation utility, could
someone point me in the right direction? I think just changing the
implementation to using an internal diff would make this more portable.
Troy
Received on 2018-02-11 04:36:45 CET