Patchwork import-checker: exclude dist-packages tree from stdlib as well

login
register
mail settings
Submitter Yuya Nishihara
Date July 3, 2015, 1:48 p.m.
Message ID <506956b94ca90eec47df.1435931326@mimosa>
Download mbox | patch
Permalink /patch/9884/
State Changes Requested
Headers show

Comments

Yuya Nishihara - July 3, 2015, 1:48 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1435929436 -32400
#      Fri Jul 03 22:17:16 2015 +0900
# Node ID 506956b94ca90eec47dfd7d9746e129bc3ccf884
# Parent  d3d2ba265e17af1d9700e3e99de4d012b1b31ab8
import-checker: exclude dist-packages tree from stdlib as well

On Debian, packaged modules are installed into dist-packages, not to
site-packages. Because of this, if we have mercurial installed, the test
failed as 'mercurial.node' was listed in stdlib_modules:

  testpackage/latesymbolimport.py relative import of stdlib module

https://wiki.debian.org/Python
Augie Fackler - July 3, 2015, 3 p.m.
> On Jul 3, 2015, at 9:48 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1435929436 -32400
> #      Fri Jul 03 22:17:16 2015 +0900
> # Node ID 506956b94ca90eec47dfd7d9746e129bc3ccf884
> # Parent  d3d2ba265e17af1d9700e3e99de4d012b1b31ab8
> import-checker: exclude dist-packages tree from stdlib as well
> 
> On Debian, packaged modules are installed into dist-packages, not to
> site-packages. Because of this, if we have mercurial installed, the test
> failed as 'mercurial.node' was listed in stdlib_modules:

Hmm. I think this means that the import-checker is running with too many things on its sys.path - does that make sense to you?

I’m not sure what we should do about it, but it definitely seems a bit undesirable to pick things up from diet-packages when we’re trying to analyze a working tree. Thoughts?

> 
>  testpackage/latesymbolimport.py relative import of stdlib module
> 
> https://wiki.debian.org/Python
> 
> diff --git a/contrib/import-checker.py b/contrib/import-checker.py
> --- a/contrib/import-checker.py
> +++ b/contrib/import-checker.py
> @@ -192,7 +192,7 @@ def list_stdlib_modules():
>         # of any().
>         if not any(libpath.startswith(p) for p in stdlib_prefixes): # no-py24
>             continue
> -        if 'site-packages' in libpath:
> +        if 'site-packages' in libpath or 'dist-packages' in libpath:
>             continue
>         for top, dirs, files in os.walk(libpath):
>             for name in files:
> @@ -202,7 +202,7 @@ def list_stdlib_modules():
>                         or name.endswith('.pyd')):
>                     continue
>                 full_path = os.path.join(top, name)
> -                if 'site-packages' in full_path:
> +                if 'site-packages' in full_path or 'dist-packages' in full_path:
>                     continue
>                 rel_path = full_path[len(libpath) + 1:]
>                 mod = dotted_name_of_path(rel_path)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Yuya Nishihara - July 3, 2015, 3:24 p.m.
On Fri, 3 Jul 2015 11:00:50 -0400, Augie Fackler wrote:
> > On Jul 3, 2015, at 9:48 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1435929436 -32400
> > #      Fri Jul 03 22:17:16 2015 +0900
> > # Node ID 506956b94ca90eec47dfd7d9746e129bc3ccf884
> > # Parent  d3d2ba265e17af1d9700e3e99de4d012b1b31ab8
> > import-checker: exclude dist-packages tree from stdlib as well
> > 
> > On Debian, packaged modules are installed into dist-packages, not to
> > site-packages. Because of this, if we have mercurial installed, the test
> > failed as 'mercurial.node' was listed in stdlib_modules:
> 
> Hmm. I think this means that the import-checker is running with too many
> things on its sys.path - does that make sense to you?

import-checker.py does list too much packages with the system Python:

  print len(stdlib_modules)

  % /usr/bin/python contrib/import-checker.py
  8841
  % /home/yuya/opt/python-2.7.8/bin/python contrib/import-checker.py
  1491

> I’m not sure what we should do about it, but it definitely seems a bit
> undesirable to pick things up from diet-packages when we’re trying to
> analyze a working tree. Thoughts?

I have no idea about the clean way to list only the standard modules, but I
think this patch is the easiest fix for the buildbot failure, running on Ubuntu.

http://hgbuildbot.kublai.com/builders/hg%20tests/builds/592/steps/run-tests.py%20%28python2.7%29/logs/stdio
Gregory Szorc - July 3, 2015, 4:57 p.m.
On Fri, Jul 3, 2015 at 8:00 AM, Augie Fackler <raf@durin42.com> wrote:

>
> > On Jul 3, 2015, at 9:48 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> >
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1435929436 -32400
> > #      Fri Jul 03 22:17:16 2015 +0900
> > # Node ID 506956b94ca90eec47dfd7d9746e129bc3ccf884
> > # Parent  d3d2ba265e17af1d9700e3e99de4d012b1b31ab8
> > import-checker: exclude dist-packages tree from stdlib as well
> >
> > On Debian, packaged modules are installed into dist-packages, not to
> > site-packages. Because of this, if we have mercurial installed, the test
> > failed as 'mercurial.node' was listed in stdlib_modules:
>
> Hmm. I think this means that the import-checker is running with too many
> things on its sys.path - does that make sense to you?
>
> I’m not sure what we should do about it, but it definitely seems a bit
> undesirable to pick things up from diet-packages when we’re trying to
> analyze a working tree. Thoughts?
>

Having recently touched this code, I believe the only place where "is
stdlib module" comes into play is when checking for mixed imports between
mercurial modules and stdlib modules on the same line. Instead of worrying
about sys.path, site-packages, etc, how about simply excluding
"mercurial.*" and "hgext.*" modules from the stdlib modules list?
Augie Fackler - July 3, 2015, 9:27 p.m.
> On Jul 3, 2015, at 12:57 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> 
> On Fri, Jul 3, 2015 at 8:00 AM, Augie Fackler <raf@durin42.com> wrote:
> 
> > On Jul 3, 2015, at 9:48 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> >
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1435929436 -32400
> > #      Fri Jul 03 22:17:16 2015 +0900
> > # Node ID 506956b94ca90eec47dfd7d9746e129bc3ccf884
> > # Parent  d3d2ba265e17af1d9700e3e99de4d012b1b31ab8
> > import-checker: exclude dist-packages tree from stdlib as well
> >
> > On Debian, packaged modules are installed into dist-packages, not to
> > site-packages. Because of this, if we have mercurial installed, the test
> > failed as 'mercurial.node' was listed in stdlib_modules:
> 
> Hmm. I think this means that the import-checker is running with too many things on its sys.path - does that make sense to you?
> 
> I’m not sure what we should do about it, but it definitely seems a bit undesirable to pick things up from diet-packages when we’re trying to analyze a working tree. Thoughts?
> 
> Having recently touched this code, I believe the only place where "is stdlib module" comes into play is when checking for mixed imports between mercurial modules and stdlib modules on the same line. Instead of worrying about sys.path, site-packages, etc, how about simply excluding "mercurial.*" and "hgext.*" modules from the stdlib modules list?
> 

That works for me. Yuya, does that work for you?
Yuya Nishihara - July 4, 2015, 2:32 a.m.
On Fri, 3 Jul 2015 17:27:46 -0400, Augie Fackler wrote:
> 
> > On Jul 3, 2015, at 12:57 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> > 
> > On Fri, Jul 3, 2015 at 8:00 AM, Augie Fackler <raf@durin42.com> wrote:
> > 
> > > On Jul 3, 2015, at 9:48 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > >
> > > # HG changeset patch
> > > # User Yuya Nishihara <yuya@tcha.org>
> > > # Date 1435929436 -32400
> > > #      Fri Jul 03 22:17:16 2015 +0900
> > > # Node ID 506956b94ca90eec47dfd7d9746e129bc3ccf884
> > > # Parent  d3d2ba265e17af1d9700e3e99de4d012b1b31ab8
> > > import-checker: exclude dist-packages tree from stdlib as well
> > >
> > > On Debian, packaged modules are installed into dist-packages, not to
> > > site-packages. Because of this, if we have mercurial installed, the test
> > > failed as 'mercurial.node' was listed in stdlib_modules:
> > 
> > Hmm. I think this means that the import-checker is running with too many things on its sys.path - does that make sense to you?
> > 
> > I’m not sure what we should do about it, but it definitely seems a bit undesirable to pick things up from diet-packages when we’re trying to analyze a working tree. Thoughts?
> > 
> > Having recently touched this code, I believe the only place where "is stdlib module" comes into play is when checking for mixed imports between mercurial modules and stdlib modules on the same line. Instead of worrying about sys.path, site-packages, etc, how about simply excluding "mercurial.*" and "hgext.*" modules from the stdlib modules list?
> > 
> 
> That works for me. Yuya, does that work for you?

Worked fine. I'll send V2 soon.

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -192,7 +192,7 @@  def list_stdlib_modules():
         # of any().
         if not any(libpath.startswith(p) for p in stdlib_prefixes): # no-py24
             continue
-        if 'site-packages' in libpath:
+        if 'site-packages' in libpath or 'dist-packages' in libpath:
             continue
         for top, dirs, files in os.walk(libpath):
             for name in files:
@@ -202,7 +202,7 @@  def list_stdlib_modules():
                         or name.endswith('.pyd')):
                     continue
                 full_path = os.path.join(top, name)
-                if 'site-packages' in full_path:
+                if 'site-packages' in full_path or 'dist-packages' in full_path:
                     continue
                 rel_path = full_path[len(libpath) + 1:]
                 mod = dotted_name_of_path(rel_path)