[Lldb-commits] [lldb] r251862 - Revert "Remove the __import__ hack of lldbtest_config."

[Zachary Turner via lldb-commits]

Yes, but it's a VERY large mechanical code change.  For example, everywhere
we were doign something like this:

import lldbutil

from one of the tests, we have to rewrite this as:

import lldbsuite.test.lldbutil as lldbutil

And we have to catch all the different ways of importing, like:

from lldbutil import x
import lldb, lldbutil

and there are about 20 or 30 modules other than lldbutil I have to fix up
as well.

I'm in the process of doing this right now, and I've almost got all the
instances found and fixed.  But I have to leave in about 15 minutes.  If I
don't get it in tonight, it will be in tomorrow morning first thing.

On Mon, Nov 2, 2015 at 5:37 PM Pavel Labath <labath at google.com> wrote:

> Good news Zachary, I'm glad we finally got to the bottom of this.
>
> I've tested your proposed changes locally, and on our buildbot (which,
> for whatever reason, did not like Enrico's workaround). Could we get
> this in as soon as possible?
>
> cheers,
> pl
>
>
> On 2 November 2015 at 16:43, Zachary Turner <zturner at google.com> wrote:
> > +Enrico Granata
> >
> >
> > On Mon, Nov 2, 2015 at 4:42 PM Zachary Turner <zturner at google.com>
> wrote:
> >>
> >> Ok, I think I figured out the root of all the problems.
> >>
> >> First, in setupSysPath we have this:
> >>
> >>     sys.path.insert(0, scriptPath)
> >>
> >> This basically adds `lldb/packages/Python/lldbsuite/test` to sys.path.
> >>
> >> Then, in the individual tests we write this:
> >>
> >> from lldbtest import *
> >>
> >> It finds lldbtest at the above path, and imports it.  But this is *not*
> >> the same copy of lldbtest that has already been imported, because that
> copy
> >> was imported as a member of the lldbsuite.test package.  So now we have
> this
> >> module in sys.modules twice.  Once as lldbtest, found by looking in
> >> lldbsuite/test since it was in sys.path.  And once as
> >> lldbsuite.test.lldbtest, found because lldbsuite was in sys.path, using
> >> subpackage resolution.
> >>
> >> I think the fix for all of these problems is to *remove* scriptPath from
> >> sys.path.  We shouldn't be mucking with sys.path for stuff in our own
> test
> >> suite.  We should resolve the references explicitly with subpackage
> >> references.  For example, even if I don't write __package__ =
> >> "lldbsuite.test" in TestMultithreaded.py, I can still fix the problem
> with
> >> the below patch:
> >>
> >> - from lldbtest import *
> >> + from lldbsuite.test.lldbtest import *
> >>
> >> because now we're referencing the module that has already been imported.
> >> This is the "correct" way to do it, so if we tighten up sys.path, nobody
> >> should be able to make this mistake again.
> >>
> >> On Mon, Nov 2, 2015 at 4:32 PM Zachary Turner <zturner at google.com>
> wrote:
> >>>
> >>> I think what is happening is that when we go through unittest2, the
> >>> package link is being broken.
> >>>
> >>> Inside dotest.py : __package__ = lldbsuite.test
> >>> Inside unittest2.loadTestsFromName : __package__ = unittest2
> >>> Inside TestMultithreaded.py : __package__ = None
> >>>
> >>> Restoring the link by writing
> >>>
> >>> __package__ = "lldbsuite.test"
> >>>
> >>> at the top of TestMultithreaded.py seems to fix the problem, although
> >>> that really bothersome to have to do that in every single file.  I'm
> still
> >>> trying to figure out the proper fix.
> >>>
> >>> On Mon, Nov 2, 2015 at 3:41 PM Pavel Labath via lldb-commits
> >>> <lldb-commits at lists.llvm.org> wrote:
> >>>>
> >>>> Author: labath
> >>>> Date: Mon Nov  2 17:39:09 2015
> >>>> New Revision: 251862
> >>>>
> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=251862&view=rev
> >>>> Log:
> >>>> Revert "Remove the __import__ hack of lldbtest_config."
> >>>>
> >>>> The hack still seems to be necessary. Putting it back in until we
> figure
> >>>> out why.
> >>>>
> >>>> Modified:
> >>>>     lldb/trunk/packages/Python/lldbsuite/test/dotest.py
> >>>>
> >>>> Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
> >>>> URL:
> >>>>
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=251862&r1=251861&r2=251862&view=diff
> >>>>
> >>>>
> ==============================================================================
> >>>> --- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
> >>>> +++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Mon Nov  2
> >>>> 17:39:09 2015
> >>>> @@ -19,11 +19,14 @@ for available options.
> >>>>  """
> >>>>
> >>>>  from __future__ import print_function
> >>>> +# this module needs to have global visibility, otherwise test cases
> >>>> +# will import it anew in their local namespace, essentially losing
> >>>> access
> >>>> +# to all the configuration data
> >>>> +globals()['lldbtest_config'] = __import__('lldbtest_config')
> >>>>
> >>>>  import use_lldb_suite
> >>>> -import lldbsuite
> >>>>
> >>>> -import lldbtest_config
> >>>> +import lldbsuite
> >>>>
> >>>>  import atexit
> >>>>  import commands
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> lldb-commits mailing list
> >>>> lldb-commits at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151103/f129b086/attachment-0001.html>