Don't change your buildbot to not use it. That's an error.
packages/Python/lldbsuite/test/dotest *must* be imported, as it depends on
its __init__.py being run. If your buildbot doesn't use it anymore, then I
think the patch I just submitted (r251819) will break your buildbot,
because I added this code:
if __name__ == "__main__":
print(__file__ + " is for use as a module only. It should not be run
as a standalone script.")
sys.exit(-1)
to packages/Python/lldbsuite/test/dotest/dotest.py
On Mon, Nov 2, 2015 at 11:22 AM Pavel Labath <lab...@google.com> wrote:
> BTW, is lldb/test/dotest.py here to stay? I thought it was there just to
> avoid breaking anybody who runs dotest directly (instead of ninja
> check-lldb), and therefore we will remove it once everybody gets a chance
> to migrate (I have already changed our buildbots to not use it).
>
> Is that correct or I am misunderstanding something?
>
> pl
>
>
> On 2 November 2015 at 11:11, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> I looked into this some more, and unfortunately I can't reproduce it.
>> That being said, I'm not convinced this patch fixes anything. The reason
>> is that when you import something, whether it be through the `__import__`
>> function or the `import` statement, the module itself is cached in
>> `sys.modules`. Whether the *name* for that module is introduced globally
>> (as is done in this patch) or locally (as is done when you use an `import`
>> statement) is irrelevant. Because the next time someone else imports it,
>> it will still find the same instance of the module in `sys.modules` and
>> just create a new name. It won't import it anew.
>>
>> If this patch does actually fix something, I think it must be a
>> coincidence. That said, I do have an idea as to what the problem might
>> be. Or at the very least, I know of a problem that would definitely lead
>> to strange behavior.
>>
>> `lldbsuite` is now a package, and it relies on the code in its
>> `__init__.py` being run. If you run
>> `packages/python/lldbsuite/test/dotest.py` manually, then `__init__.py`
>> doesn't get run, and `lldbExec` doesn't get initialized properly.
>>
>> Of course, this isn't what you're doing, but it *is* what `dosep` does
>> internally. `dosep` manually constructs a path to
>> `packages/python/lldbsuite/test/dotest.py`
>> and execs it. I have a patch that fixes this and makes `dosep` exec
>> `lldb/test/dotest.py` instead, which will then lead to the package being
>> imported, and `__init__.py` being run, and everything being initialized
>> properly.
>>
>> I'm going to commit that patch by itself, and then I will submit a
>> followup patch that reverts the change from this patch (since I can't
>> reproduce this problem, I can't check whether or not my patch fixes it).
>> So if my revert breaks you again, feel free to revert the revert. Although
>> if there's any way you can investigate a bit to understand what's going on
>> a little bit more, but I would be very grateful. In particular, I wonder
>> about the following things:
>>
>> 1) When lldbExec is not initialized properly, is this in the same process
>> instance that you ran from the command line, or is it in the
>> multiprocessing fork?
>> 2) If you add code to `lldbsuite/__init__.py` to print the process id and
>> the value of `lldb_root`, and then add code in `dotest.py` to print the
>> process id and the value of lldbExec, what does the output look like?
>> (Each line should be printed up to twice, due to the multiprocessing fork).
>>
>> Anyway, I'll get those 2 patches submitted to fix the dosep issue and
>> revert this change, and see what happens.
>>
>> On Fri, Oct 30, 2015 at 1:53 PM Jim Ingham <jing...@apple.com> wrote:
>>
>>> Note, the other important step was that you had to have an lldb
>>> installed in /usr/bin/lldb that FAILED this test. If you have a more
>>> recent lldb there, the test will succeed, and you won't notice you aren't
>>> testing your newly built sources.
>>>
>>> Jim
>>>
>>> > On Oct 30, 2015, at 1:25 PM, Enrico Granata via lldb-commits <
>>> lldb-commits@lists.llvm.org> wrote:
>>> >
>>> > I think what I was doing is be in lldb/test and do
>>> >
>>> > $ ./dotest.py ../packages/python/lldbsuite/functionalities/completion
>>> >
>>> >> On Oct 30, 2015, at 12:22 PM, Zachary Turner <ztur...@google.com>
>>> wrote:
>>> >>
>>> >> Can you give me a command line which will reproduce the original
>>> problem? Because I ran through the entire test suite and nothing failed,
>>> so I want to make sure we're doing the same thing. I'm still a little
>>> confused about how this happens, but I plan to look into it when I'm back
>>> on Monday and see if I can understand it better to identify a better fix.
>>> >>
>>> >> On Fri, Oct 30, 2015 at 11:58 AM Enrico Granata <egran...@apple.com>
>>> wrote:
>>> >>> On Oct 29, 2015, at 11:31 PM, Zachary Turner <ztur...@google.com>
>>> wrote:
>>> >>>
>>> >>> Wow. That's a weird problem. Thanks for finding it!
>>> >>>
>>> >>> Would it work if we move the definition of the `lldbtest_config`
>>> class into lldbsuite/test/__init__.py? This way the configuration should
>>> be part of the global package state of the lldbsuite.test package, which
>>> all the tests are already members of the same package, so they wouldn't
>>> even need to import anything (I think).
>>> >>>
>>> >>
>>> >> I think the problem is exactly that we want lldbtest_config to be
>>> *global* state and not package local state.
>>> >> Honestly, I think if we are not content with the fix as it stands,
>>> the right way would be to change the way unittest2 imports test cases as to
>>> use the package-level global scope instead of the global global state as it
>>> is now.
>>> >>
>>> >>> On Oct 30, 2015, at 8:32 AM, Zachary Turner <ztur...@google.com>
>>> wrote:
>>> >>>
>>> >>> I'm also still a little confused why this worked before my patch.
>>> How is unittest2 importing the individual tests in a way that behaves
>>> differently when dotest is a package (now) versus a standalone script
>>> (before)?
>>> >>>
>>> >>
>>> >> That is a good question. One to which “because Python” is the only
>>> answer I can think of. I suspect scripts live at the global scope anyway,
>>> so we were just getting lucky with those imports making it through
>>> correctly.
>>> >>
>>> >>> On Thu, Oct 29, 2015 at 6:12 PM Enrico Granata via lldb-commits <
>>> lldb-commits@lists.llvm.org> wrote:
>>> >>> Author: enrico
>>> >>> Date: Thu Oct 29 20:09:54 2015
>>> >>> New Revision: 251678
>>> >>>
>>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=251678&view=rev
>>> >>> Log:
>>> >>> Some test cases that need the lldbExec path were failing because
>>> lldbExec was turning out to be None even though it was being validly set by
>>> dotest.py
>>> >>>
>>> >>> It turns out that lldbtest_config was being imported locally to
>>> "lldbsuite.test" instead of globally, so when the test cases got
>>> individually brought by a global import via __import__ by unittest2, they
>>> did not see the lldbtest_config import, and ended up importing a new
>>> separate copy of it, with lldbExec unset
>>> >>>
>>> >>> This is a simple hackaround that brings lldbtest_config to global
>>> visibility and makes sure the configuration data is correctly shared
>>> >>>
>>> >>>
>>> >>> 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=251678&r1=251677&r2=251678&view=diff
>>> >>>
>>> ==============================================================================
>>> >>> --- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
>>> >>> +++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Thu Oct 29
>>> 20:09:54 2015
>>> >>> @@ -21,6 +21,10 @@ 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
>>> >>>
>>> >>> @@ -42,7 +46,6 @@ import test_results
>>> >>> from test_results import EventBuilder
>>> >>> import inspect
>>> >>> import unittest2
>>> >>> -import lldbtest_config
>>> >>> import test_categories
>>> >>>
>>> >>> import six
>>> >>>
>>> >>>
>>> >>> _______________________________________________
>>> >>> lldb-commits mailing list
>>> >>> lldb-commits@lists.llvm.org
>>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>> >>
>>> >>
>>> >> Thanks,
>>> >> - Enrico
>>> >> 📩 egranata@.com ☎️ 27683
>>> >>
>>> >
>>> >
>>> > Thanks,
>>> > - Enrico
>>> > 📩 egranata@.com ☎️ 27683
>>> >
>>> > _______________________________________________
>>> > 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
>>
>>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
