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

[Jose Fonseca]

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.

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=