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

[Pavel Labath via lldb-commits]

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