<div dir="ltr">As an alternative, since this is breaking your buildbot, I can check in the lion's share of the work (which should fix about 95% of the fixups), and you can do any remaining ones that pop up on the linux buildbot after I'm gone. Do you prefer that?<div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 2, 2015 at 5:40 PM Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Yes, but it's a VERY large mechanical code change. For example, everywhere we were doign something like this:<div><br></div><div>import lldbutil</div><div><br></div><div>from one of the tests, we have to rewrite this as:</div><div><br></div><div>import lldbsuite.test.lldbutil as lldbutil</div><div><br></div><div>And we have to catch all the different ways of importing, like:</div><div><br></div><div>from lldbutil import x</div><div>import lldb, lldbutil</div><div><br></div><div>and there are about 20 or 30 modules other than lldbutil I have to fix up as well.</div><div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 2, 2015 at 5:37 PM Pavel Labath <<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Good news Zachary, I'm glad we finally got to the bottom of this.<br>
<br>
I've tested your proposed changes locally, and on our buildbot (which,<br>
for whatever reason, did not like Enrico's workaround). Could we get<br>
this in as soon as possible?<br>
<br>
cheers,<br>
pl<br>
<br>
<br>
On 2 November 2015 at 16:43, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
> +Enrico Granata<br>
><br>
><br>
> On Mon, Nov 2, 2015 at 4:42 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
>><br>
>> Ok, I think I figured out the root of all the problems.<br>
>><br>
>> First, in setupSysPath we have this:<br>
>><br>
>> sys.path.insert(0, scriptPath)<br>
>><br>
>> This basically adds `lldb/packages/Python/lldbsuite/test` to sys.path.<br>
>><br>
>> Then, in the individual tests we write this:<br>
>><br>
>> from lldbtest import *<br>
>><br>
>> It finds lldbtest at the above path, and imports it. But this is *not*<br>
>> the same copy of lldbtest that has already been imported, because that copy<br>
>> was imported as a member of the lldbsuite.test package. So now we have this<br>
>> module in sys.modules twice. Once as lldbtest, found by looking in<br>
>> lldbsuite/test since it was in sys.path. And once as<br>
>> lldbsuite.test.lldbtest, found because lldbsuite was in sys.path, using<br>
>> subpackage resolution.<br>
>><br>
>> I think the fix for all of these problems is to *remove* scriptPath from<br>
>> sys.path. We shouldn't be mucking with sys.path for stuff in our own test<br>
>> suite. We should resolve the references explicitly with subpackage<br>
>> references. For example, even if I don't write __package__ =<br>
>> "lldbsuite.test" in TestMultithreaded.py, I can still fix the problem with<br>
>> the below patch:<br>
>><br>
>> - from lldbtest import *<br>
>> + from lldbsuite.test.lldbtest import *<br>
>><br>
>> because now we're referencing the module that has already been imported.<br>
>> This is the "correct" way to do it, so if we tighten up sys.path, nobody<br>
>> should be able to make this mistake again.<br>
>><br>
>> On Mon, Nov 2, 2015 at 4:32 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
>>><br>
>>> I think what is happening is that when we go through unittest2, the<br>
>>> package link is being broken.<br>
>>><br>
>>> Inside dotest.py : __package__ = lldbsuite.test<br>
>>> Inside unittest2.loadTestsFromName : __package__ = unittest2<br>
>>> Inside TestMultithreaded.py : __package__ = None<br>
>>><br>
>>> Restoring the link by writing<br>
>>><br>
>>> __package__ = "lldbsuite.test"<br>
>>><br>
>>> at the top of TestMultithreaded.py seems to fix the problem, although<br>
>>> that really bothersome to have to do that in every single file. I'm still<br>
>>> trying to figure out the proper fix.<br>
>>><br>
>>> On Mon, Nov 2, 2015 at 3:41 PM Pavel Labath via lldb-commits<br>
>>> <<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br>
>>>><br>
>>>> Author: labath<br>
>>>> Date: Mon Nov 2 17:39:09 2015<br>
>>>> New Revision: 251862<br>
>>>><br>
>>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=251862&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=251862&view=rev</a><br>
>>>> Log:<br>
>>>> Revert "Remove the __import__ hack of lldbtest_config."<br>
>>>><br>
>>>> The hack still seems to be necessary. Putting it back in until we figure<br>
>>>> out why.<br>
>>>><br>
>>>> Modified:<br>
>>>> lldb/trunk/packages/Python/lldbsuite/test/dotest.py<br>
>>>><br>
>>>> Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py<br>
>>>> URL:<br>
>>>> <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=251862&r1=251861&r2=251862&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=251862&r1=251861&r2=251862&view=diff</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> --- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)<br>
>>>> +++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Mon Nov 2<br>
>>>> 17:39:09 2015<br>
>>>> @@ -19,11 +19,14 @@ for available options.<br>
>>>> """<br>
>>>><br>
>>>> from __future__ import print_function<br>
>>>> +# this module needs to have global visibility, otherwise test cases<br>
>>>> +# will import it anew in their local namespace, essentially losing<br>
>>>> access<br>
>>>> +# to all the configuration data<br>
>>>> +globals()['lldbtest_config'] = __import__('lldbtest_config')<br>
>>>><br>
>>>> import use_lldb_suite<br>
>>>> -import lldbsuite<br>
>>>><br>
>>>> -import lldbtest_config<br>
>>>> +import lldbsuite<br>
>>>><br>
>>>> import atexit<br>
>>>> import commands<br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> lldb-commits mailing list<br>
>>>> <a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
</blockquote></div></blockquote></div>