[Piglit] [PATCH] grouptools.py: Silently ignore mixed os.path.join usage on Windows too.

[Dylan Baker]

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 <baker.dylan.c at gmail.com>
> >
> > 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
> >> Piglit at lists.freedesktop.org
> >> 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=
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150311/7fd5e4dd/attachment.sig>