[Mesa-dev] [PATCH mesa 6/6] scons: use python3-compatible string-check

[Ilia Mirkin]

On Tue, Sep 26, 2017 at 7:07 AM, Jose Fonseca <jfonseca at vmware.com> wrote:
> On 25/09/17 14:30, Eric Engestrom wrote:
>>
>> I pushed the rest of the series.
>> See below for discussion on this patch.
>>
>>
>> On Wednesday, 2017-09-20 17:05:21 +0000, Jose Fonseca wrote:
>>>
>>> On 19/09/17 15:14, Eric Engestrom wrote:
>>>>
>>>> Signed-off-by: Eric Engestrom <eric.engestrom at imgtec.com>
>>>> ---
>>>>    scons/custom.py | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/scons/custom.py b/scons/custom.py
>>>> index 0767ba936d410167116d..978ee5f9ec7c23a74cb9 100644
>>>> --- a/scons/custom.py
>>>> +++ b/scons/custom.py
>>>> @@ -257,7 +257,7 @@ def parse_source_list(env, filename, names=None):
>>>>        sym_table = parser.parse(src.abspath)
>>>>        if names:
>>>> -        if isinstance(names, basestring):
>>>> +        if isinstance(names, str):
>>>>                names = [names]
>>>>            symbols = names
>>>>
>>>
>>> I'm not sure if this won't give the wrong results for unicode strings,
>>> but
>>> at any rate, I don't think that should ever happen in practice.
>>
>>
>> Are you replying to Ilia [1] here?
>>
>> He left a comment on this patch, saying:
>>>
>>> This might be python3-compatible, but it's not the same thing. str !=
>>> unicode. Not sure where "names" can come from, but if it can come in
>>> as a unicode string, this won't work.
>>
>>
>> My knowledge of python is quite basic, so I'd rather you discuss between
>> you two rather than me trying to forward a conversation with each of you
>> :P
>>
>> [1]
>> https://lists.freedesktop.org/archives/mesa-dev/2017-September/170130.html
>>
>
> I just checked all uses of ParseSourceList and it's never used with unicode
> strings.  It's always used with plain strings or lists of plain strings.
>
> So on 2nd thought, I think this patch should be safe.  Ilia, would you
> agree?

So really this whole thing is about whether names is a list or not.
You can't check if it's iterable since strings are also iterable. What
lists are passed in here? If it's all list-type lists (and not
tuples/etc), we can change this to be

if not isinstance(names, list)

Or we want to be cheeky,

if type(names) == type(names[0])

For a list of strings, that won't be the case, but for a string, it
should be, since a single char has the same type as a string. At least
in python2.

I don't use scons, or know how all this stuff is used, so you guys can
do whatever. In my experience, unicode strings spring up from the most
unexpected places.

  -ilia