Patchwork [1,of,3] import-checker: refactor sys.path prefix check (issue4129)

# HG changeset patch
# User Chris Jerdonek <chris.jerdonek@gmail.com>
# Date 1387750226 28800
#      Sun Dec 22 14:10:26 2013 -0800
# Node ID a4bfbeba9838ef33ed72942d98ddad0cbf608b2d
# Parent  4274eda143cb1025be1130ffdaaf62370a2a6961
import-checker: refactor sys.path prefix check (issue4129)

This patch refactors the logic in contrib/import-checker.py responsible for
checking the beginnings of the paths in sys.path.  In particular, it adds a
variable that defines the set of allowed prefixes.

The primary purpose of this change is to make it easier to add more allowed
prefixes.  This will be useful in resolving issue4129, which involves making
the function list_stdlib_modules() work when run from a virtualenv.

On Dec 23, 2013, at 12:34 AM, Chris Jerdonek <chris.jerdonek@gmail.com> wrote:

> # HG changeset patch
> # User Chris Jerdonek <chris.jerdonek@gmail.com>
> # Date 1387750226 28800
> #      Sun Dec 22 14:10:26 2013 -0800
> # Node ID a4bfbeba9838ef33ed72942d98ddad0cbf608b2d
> # Parent  4274eda143cb1025be1130ffdaaf62370a2a6961
> import-checker: refactor sys.path prefix check (issue4129)
> 
> This patch refactors the logic in contrib/import-checker.py responsible for
> checking the beginnings of the paths in sys.path.  In particular, it adds a
> variable that defines the set of allowed prefixes.
> 
> The primary purpose of this change is to make it easier to add more allowed
> prefixes.  This will be useful in resolving issue4129, which involves making
> the function list_stdlib_modules() work when run from a virtualenv.
> 
> diff --git a/contrib/import-checker.py b/contrib/import-checker.py
> --- a/contrib/import-checker.py
> +++ b/contrib/import-checker.py
> @@ -48,11 +48,14 @@
>     for m in 'ctypes', 'email':
>         yield m
>     yield 'builtins' # python3 only
> +    stdlib_prefixes = set([sys.prefix, sys.exec_prefix])
>     for libpath in sys.path:
> -        # We want to walk everything in sys.path that starts with
> -        # either sys.prefix or sys.exec_prefix.
> -        if not (libpath.startswith(sys.prefix)
> -                or libpath.startswith(sys.exec_prefix)):
> +        # We want to walk everything in sys.path that starts with something
> +        # in stdlib_prefixes.
> +        for prefix in stdlib_prefixes:
> +            if libpath.startswith(prefix):
> +                break
> +        else:
>             continue

This file already depends on 2.6isms (the ast module), so perhaps we could use any() here and avoid the slightly-awkward-to-me for/else? I think it might read more clearly.

>         if 'site-packages' in libpath:
>             continue

On Dec 24, 2013, at 5:45 PM, Augie Fackler <raf@durin42.com> wrote:

> 
> On Dec 23, 2013, at 12:34 AM, Chris Jerdonek <chris.jerdonek@gmail.com> wrote:
> 
>> 
>>    for libpath in sys.path:
>> -        # We want to walk everything in sys.path that starts with
>> -        # either sys.prefix or sys.exec_prefix.
>> -        if not (libpath.startswith(sys.prefix)
>> -                or libpath.startswith(sys.exec_prefix)):
>> +        # We want to walk everything in sys.path that starts with something
>> +        # in stdlib_prefixes.
>> +        for prefix in stdlib_prefixes:
>> +            if libpath.startswith(prefix):
>> +                break
>> +        else:
>>            continue
> 
> This file already depends on 2.6isms (the ast module), so perhaps we could use any() here and avoid the slightly-awkward-to-me for/else? I think it might read more clearly.

After looking at the rest of the series, I'm crewing it with my own follow-up patch to use any() on a genexp instead of the for/break/else/continue trick. Thanks for following up on this!

On Tue, Dec 24, 2013 at 2:52 PM, Augie Fackler <raf@durin42.com> wrote:
>
> On Dec 24, 2013, at 5:45 PM, Augie Fackler <raf@durin42.com> wrote:
>
>>
>> On Dec 23, 2013, at 12:34 AM, Chris Jerdonek <chris.jerdonek@gmail.com> wrote:
>>
>>>
>>>    for libpath in sys.path:
>>> -        # We want to walk everything in sys.path that starts with
>>> -        # either sys.prefix or sys.exec_prefix.
>>> -        if not (libpath.startswith(sys.prefix)
>>> -                or libpath.startswith(sys.exec_prefix)):
>>> +        # We want to walk everything in sys.path that starts with something
>>> +        # in stdlib_prefixes.
>>> +        for prefix in stdlib_prefixes:
>>> +            if libpath.startswith(prefix):
>>> +                break
>>> +        else:
>>>            continue
>>
>> This file already depends on 2.6isms (the ast module), so perhaps we could use any() here and avoid the slightly-awkward-to-me for/else? I think it might read more clearly.
>
> After looking at the rest of the series, I'm crewing it with my own follow-up patch to use any() on a genexp instead of the for/break/else/continue trick. Thanks for following up on this!

Great, thanks, Augie!

--Chris