On Wed, Mar 11, 2015 at 08:19:42PM +0000, Jose Fonseca wrote: > Thanks Dylan. > > Yeah, this is meant mostly as short term stop gap. Feel free to revert > this when you have a better solution. > > '|' sounds alright. Another alternative would be to use a non-printable > character (something nobody ever consider using in a test name) -- then > replace it with '/' a __str__/__repr__ methods. Eitherway, if you make > the separate a constant variable, you can always tweak it later.
Turns out that glean uses a lot of special characters in test names, I'm down to using somethign more exotic like @, which isn't a binary operator. I assume that windows terminal can print the @ chracter? > > Jose > > > On 11/03/15 18:42, Dylan Baker wrote: > > Also sent a reply to this from the wrong account, I think the best > > solution is to replace '/' with some other character. Maybe '|'? > > > > I'll work on patches for a different separator, in the mean time: > > > > Reviewed-by: Dylan Baker <[email protected]> > > > > On Wed, Mar 11, 2015 at 11:24:36AM +0000, Jose Fonseca wrote: > >> grouptools currently acts like a time-bomb: spite the good intention, it > >> currently ignores when developers mistakedly use os.path.join with > >> grouptools on Posix; then it explodes when the same code is used on > >> Windows. > >> > >> This makes grouptools behavior on Windows the same as Linux, ie., > >> silently ignore when os.path.join is mixed. > >> > >> Another solution would be to use a different separator character on > >> Linux, but that would be a much more complex change than I can afford to > >> make at this moment. > >> --- > >> framework/grouptools.py | 25 +++++++++++++++++++++++++ > >> 1 file changed, 25 insertions(+) > >> > >> diff --git a/framework/grouptools.py b/framework/grouptools.py > >> index 3d26bbc..1000a51 100644 > >> --- a/framework/grouptools.py > >> +++ b/framework/grouptools.py > >> @@ -30,6 +30,7 @@ posix paths they may not start with a leading '/'. > >> """ > >> > >> import posixpath > >> +import os.path > >> > >> __all__ = [ > >> 'join', > >> @@ -42,6 +43,22 @@ __all__ = [ > >> 'from_path' > >> ] > >> > >> + > >> +def _normalize(group): > >> + """Helper to normalize group paths on Windows. > >> + > >> + Although grouptools' heart is in the right place, the fact is that it > >> fails > >> + to spot when developers mistakedly use os.path.join for groups on > >> Posix > >> + systems. > >> + > >> + So until this is improved somehow, make grouptools behavior on Windows > >> + match Linux, ie, just silently ignore mixed use of grouptools and > >> os.path. > >> + """ > >> + if os.path.sep != '/': > >> + group = group.replace(os.path.sep, '/') > >> + return group > >> + > >> + > >> def _assert_illegal(group): > >> """Helper that checks for illegal characters in input.""" > >> assert isinstance(group, (str, unicode)), 'Type must be string or > >> unicode' > >> @@ -61,6 +78,7 @@ def testname(group): > >> Analogous to os.path.basename > >> > >> """ > >> + group = _normalize(group) > >> _assert_illegal(group) > >> > >> return posixpath.basename(group) > >> @@ -76,6 +94,7 @@ def groupname(group): > >> Analogous to os.path.dirname > >> > >> """ > >> + group = _normalize(group) > >> _assert_illegal(group) > >> > >> return posixpath.dirname(group) > >> @@ -83,6 +102,7 @@ def groupname(group): > >> > >> def splitname(group): > >> """Split a group name, Returns tuple "(group, test)".""" > >> + group = _normalize(group) > >> _assert_illegal(group) > >> > >> return posixpath.split(group) > >> @@ -90,6 +110,7 @@ def splitname(group): > >> > >> def commonprefix(args): > >> """Given a list of groups, returns the longest common leading > >> component.""" > >> + args = [_normalize(group) for group in args] > >> for group in args: > >> _assert_illegal(group) > >> > >> @@ -103,6 +124,7 @@ def join(*args): > >> '\\' in them, as these are groups not paths. > >> > >> """ > >> + args = [_normalize(group) for group in args] > >> for group in args: > >> assert isinstance(group, (str, unicode)), \ > >> 'Type must be string or unicode' > >> @@ -121,6 +143,8 @@ def relgroup(large, small): > >> start is longer than the group then '' is returned. > >> > >> """ > >> + large = _normalize(large) > >> + small = _normalize(small) > >> for element in {large, small}: > >> _assert_illegal(element) > >> > >> @@ -138,6 +162,7 @@ def split(group): > >> If input is '' return an empty list > >> > >> """ > >> + group = _normalize(group) > >> _assert_illegal(group) > >> if group == '': > >> return [] > >> -- > >> 2.1.0 > >> > >> _______________________________________________ > >> Piglit mailing list > >> [email protected] > >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_piglit&d=AwIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=SsqcjtAtzTdE_vBy1rhD_WiZrV0ZdU_dE5uIAzr47F4&s=wQGaXZ5nxvOyc6gdSsbDQr_POe2MSYH6VbLv17cBbQk&e= >
signature.asc
Description: Digital signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
