subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Troy Curtis <troycurti...@apache.org>
Subject Re: [swig-py3][patch] interfacing bytes object instead of str
Date Thu, 27 Dec 2018 19:09:44 GMT
On 12/27/18 12:42 PM, Yasuhito FUTATSUKI wrote:
> On 12/27/18 1:01 PM, Troy Curtis wrote:
>   > The only real functionality changes I'm concerned with on the 
> changes are dropping str() conversion on a couple of the API inputs 
> since previously providing any object with a suitable __str__ method 
> would work, but would now be broken. But a simple way of supporting Py2 
> and Py3 isn't a "gimme", so didn't want to block this patch on that.
> 
> I see. So I've revise it and str/bytes conversion in unit tests as a first
> step of followup to r1849784.

Oh my, so I've been silly having you look at this. I just realized that 
both usages of dropping str() were within the tests! SMH! Since those 
usages are completely internal, there is no danger of being provided 
some other value. I apologize for the misdirection.

> 
> [in subversion/bindings/swig/python/tests]
> 
> * client.py (SubversionClientTestCase.test_update4): not to encode
>   os.path.sep if it is already bytes object
> 
> * core.py (SubversionCoreTestCase.test_stream_write): not to encode
>   fname if it is already bytes object
> 
> * delta.py (DeltaTestCase.testTxWindowHandler_stream_IF,
>   DeltaTestCase.testTxWindowHandler_Stream_IF): not to encode fname if it
>   is already bytes object
> 
> * fs.py (SubversionFSTestCase.setUp): not to encode self.tmpfile if it
>   is already bytes object
> 
> * trac/versioncontrol/main.py (Node.__init__): revise change to strip
>   str() on r1849784; if path is already bytes, use it for self.path.
>   othewise ensure self.path is str, and if it is not bytes (i.e. in case
>   of py3), encode it to bytes
> 
> * trac/versioncontrol/svn_fs.py (SubversionNode.__init__): don't use
>   IS_PY3 condition, just store a unicode string in the exception object.
>   - remove unnecessary variable IS_PY3 and therefore unnecessary import
>    of sys module
>   - (SubversionNode.getproperties): revise change to strip str() on
>    r1849784; if value is already bytes, use it for prop[name].  othewise
>    ensure value is str, and if it is not bytes (i.e. in case of py3),
>    encode it to bytes for consistency of the type of prop[name]
> 
> * utils.py (Temper.alloc_empty_dir, Temper.alloc_empty_repo): ensure
>   suffix arg for tempfile.mkdtemp() is unicode (for py3 < 3.5)
> 
> Cheers,

LGTM, committed as r1849804, but without the unnecessary gymnastics to 
continue supporting str() in those two cases mentioned above. I also 
tweaked the log message to use complete sentences (and removed 
references to the code that I didn't include).

Troy


Mime
View raw message