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 <lab...@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 <ztur...@google.com> wrote: > > +Enrico Granata > > > > > > On Mon, Nov 2, 2015 at 4:42 PM Zachary Turner <ztur...@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 <ztur...@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@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@lists.llvm.org > >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits