[lldb-dev] Refactoring dotest as a Python package

Wed Oct 28 22:48:40 PDT 2015

It looks like `lldbsuite.lldb_root` is not getting set correctly for you.
Line 1070 of lldbsuite/test/dotest.py looks like this:

    lldbRootDirectory = lldbsuite.lldb_root

`lldbsuite.lldb_root` is set in `packages/Python/lldbsuite/__init__.py`
whenever someone writes `import lldbsuite`.  The import must have been
successful otherwise you would have gotten a stacktrace pointing to this
line with a message like "unknown package lldbsuite", so I guess we need to
figure out why the code in __init__.py is not working correctly.  Can you
add a print statement in __init__.py after it sets lldb_root, and another
print statement at the above line of code to see what the value of
lldb_root is in both cases?

You might also try applying http://reviews.llvm.org/D14157 and see if that
makes things better.

On Wed, Oct 28, 2015 at 10:40 PM Zachary Turner <zturner at google.com> wrote:

> Can you give me a stack trace?
>
> On Wed, Oct 28, 2015 at 10:37 PM Zachary Turner <zturner at google.com>
> wrote:
>
>> Shoot, I'm at the LLVM Developers Conference tomorrow and Friday as
>> well.  Let me eyeball the code and see if I can figure out what might be
>> wrong.
>>
>>
>> On Wed, Oct 28, 2015 at 4:25 PM Greg Clayton <gclayton at apple.com> wrote:
>>
>>> Zach: this no longer works:
>>>
>>> % ./dotest.py -A x86_64 -C clang -v -t
>>> /.../packages/Python/lldbsuite/test/functionalities/completion
>>>
>>> fill "..." in with your path to your lldb root.
>>>
>>> It is unable to find lldb because lldbtest_config.lldbExec isn't set
>>> correctly...
>>>
>>>
>>> > On Oct 28, 2015, at 12:21 PM, Zachary Turner via lldb-dev <
>>> lldb-dev at lists.llvm.org> wrote:
>>> >
>>> > Committed r251544 to try to fix this.
>>> >
>>> > On Wed, Oct 28, 2015 at 12:18 PM Zachary Turner <zturner at google.com>
>>> wrote:
>>> > It's saying "import: command not found".  Do I need to put a
>>> `#!/usr/bin/env python` or something at the top?  I thought by virtue of
>>> having a .py extension this would be handled, but maybe not.
>>> >
>>> > On Wed, Oct 28, 2015 at 12:16 PM Zachary Turner <zturner at google.com>
>>> wrote:
>>> > We can set executable permissions on
>>> packages/Python/lldbsuite/test/dotest.py for now.  I suppose that's
>>> actually necessary for now since it does tget executed indirectly by dosep.
>>> >
>>> > I plan to remove the need for this, and at the same time make it fail
>>> if you try to run that file directly.  Will that fix this problem?  It's
>>> not clear to me from looking at this log if that is the problem
>>> >
>>> > On Wed, Oct 28, 2015 at 11:50 AM Oleksiy Vyalov <ovyalov at google.com>
>>> wrote:
>>> > Linux build bot is failing
>>> http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/7895
>>> :
>>> >
>>> >
>>> /lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../llvm/tools/lldb/test/dotest.py
>>> --executable
>>> /lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../build/bin/lldb -A i386
>>> -C clang-3.5 -s logs-clang-3.5-i386 -u CXXFLAGS -u CFLAGS --channel
>>> "gdb-remote packets" --channel "lldb all"
>>> >
>>> /lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../llvm/tools/lldb/test/dotest.py:
>>> line 1: import: command not found
>>> >
>>> /lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../llvm/tools/lldb/test/dotest.py:
>>> line 3: import: command not found
>>> >
>>> /lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../llvm/tools/lldb/test/dotest.py:
>>> line 5: syntax error: unexpected end of file
>>> >
>>> > Should we run packages/Python/lldbsuite/test/dotest.py instead of
>>> test/dotest.py?
>>> > If yes, can we set executable permissions on
>>> packages/Python/lldbsuite/test/dotest.py?
>>> >
>>> > On Wed, Oct 28, 2015 at 10:54 AM, Zachary Turner via lldb-dev <
>>> lldb-dev at lists.llvm.org> wrote:
>>> > This is in right now (without my proposed change from previous email,
>>> although I can make that as a followup since it's just cleanup)
>>> >
>>> > In any case, let me know if anything blows up.  It took 35 minutes
>>> just to commit, so hopefully any problems that arise can be fixed with
>>> localized patches instead of revert.  Although I'm still open to that if
>>> it's really the only way to get things fixed.
>>> >
>>> > On Tue, Oct 27, 2015 at 8:12 PM Zachary Turner <zturner at google.com>
>>> wrote:
>>> > Todd, I have one question.  If I understand correctly, we currently
>>> run dotest.py as a script, which imports dosep and calls some method in
>>> dosep, and dosep then again exec's dotest.
>>> >
>>> > Can you think of a pythonic way to make this work under the new
>>> layout?  To be clear, I have it working, just not in a pythonic way.  The
>>> problem is that if we put code in lldbsuite's __init__.py, then this code
>>> won't be run when we exec dotest, because we'll be running it as a script
>>> instead of importing it as a package.  __init__.py is a very handy way to
>>> run some per-package initialization, so I'd like to be able to take
>>> advantage of it.
>>> >
>>> > The way I currently have it working is just write `import lldbsuite`
>>> at the top of dotest.py, but that seems a little arbitrary that a file
>>> can't be exec'ed unless it imports the package that it's supposed to be
>>> contained in.
>>> >
>>> > So to re-iterate: the problem is that under the new layout the user
>>> will run lldb/test/dotest.py, but dosep will try to exec
>>> lldb/packages/Python/lldbsuite/test/dotest.py, which is intended to be
>>> imported as a package.
>>> >
>>> > What if we have dosep instead do this:
>>> >
>>> > # Execute the same script that the user originall ran from the command
>>> line,
>>> > # which under this new system will be lldb/test/dotest.py, even though
>>> dosep
>>> > # and the *real* dotest now reside in
>>> lldb/packages/Python/lldbsuite/test
>>> > import __main__ as main
>>> > exec main.__file__
>>> >
>>> > Can you think of any problem with this?  Another option is to use
>>> sys.argv[0].  Not sure if there's any practical difference between the two
>>> methods.
>>> >
>>> > On Tue, Oct 27, 2015 at 7:29 PM Zachary Turner <zturner at google.com>
>>> wrote:
>>> > Ok, I'll do it tomorrow.  Since it's a big code move I was a little
>>> worried it would break someone's bot or the Xcode build, but I guess we can
>>> deal with issues that pop up afterwards.
>>> >
>>> > On Tue, Oct 27, 2015 at 5:14 PM Jim Ingham <jingham at apple.com> wrote:
>>> > It seems like everybody is okay with the idea of this, so I don't see
>>> the need for a review of the details of this stage.  If you think there's
>>> anything tricky call it out in words, otherwise I say just commit it.
>>> >
>>> > Jim
>>> >
>>> >
>>> > > On Oct 27, 2015, at 4:30 PM, Zachary Turner via lldb-dev <
>>> lldb-dev at lists.llvm.org> wrote:
>>> > >
>>> > > I have the first part of the patch in, and the second part of the
>>> patch (which is essentially just a whole-folder rename with a couple of
>>> fixups) ready to go.  What's the best way to have this reviewed?  Uploading
>>> a 7MB patch to Phabricator probably isn't going to work out very well.
>>> > >
>>> > > On Tue, Oct 27, 2015 at 1:40 PM Zachary Turner <zturner at google.com>
>>> wrote:
>>> > > I think I have a way to split this into two smaller CLs.  I'm
>>> testing this at the moment, if so it will allow the first CL to be most of
>>> the preparation for the rename without the rename, and then the second CL
>>> should literally just be a straight move with only 1-2 line code change.
>>> So I'll try to put this first CL up for review shortly.
>>> > >
>>> > > On Tue, Oct 27, 2015 at 12:49 PM Zachary Turner <zturner at google.com>
>>> wrote:
>>> > > I've got a patch locally to make all of our Python stuff part of an
>>> lldb package called `lldbsuite`.  Currently we've got a bunch of standalone
>>> scripts that live in various directories such as `lldb/test`, or
>>> `lldb/scripts`, and possibly some  other locations, and this organization
>>> makes it hard to share code because it is incompatible with Python's
>>> built-in code reuse mechanism, which is its package system.
>>> > >
>>> > > The problem is, this patch is *big*.  Functionally there weren't
>>> many major changes, but it renames the entire test directory.  To be clear,
>>> it still leaves `test/dotest.py` in place, so nobody has to change their
>>> workflow or do anything differently.  If you used to write "cd test &&
>>> dotest.py" you can still do that.  dotest.py is now just a 2 line wrapper
>>> around the package, so it looks like:
>>> > >
>>> > > import lldbsuite.test
>>> > > lldbsuite.test.run_suite()
>>> > >
>>> > > the advantage of this method is that lldbsuite can contain
>>> subpackages.  It already contains one subpackage, which is lldbsuite.test,
>>> and I plan to move some of the Python code in `lldb/scripts` there as well,
>>> so that we have lldbsuite.scripts.  Then we can add a third submodule,
>>> lldbsuite.shared, and now dotest can share code with scripts, and it gives
>>> us a nice place to put stuff that previously had been copied all around.
>>> > >
>>> > > It also gives us a nice way to perform module-wide initialization
>>> that we don't have to repeat in every single test, such as writing "import
>>> lldb_shared" at the top of every file, since that can be done as part of
>>> lldbsuite/__init__.py.
>>> > >
>>> > >
>>> > > In any case, I have this all working on my machine, but I would like
>>> to see if someone can try my patch out on other platforms.  The size of the
>>> patch presents a problem though - it's over 7MB since it renames a very
>>> large directory.
>>> > >
>>> > > As usual, comments / concerns also welcome.
>>> > > _______________________________________________
>>> > > lldb-dev mailing list
>>> > > lldb-dev at lists.llvm.org
>>> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>> >
>>> >
>>> > _______________________________________________
>>> > lldb-dev mailing list
>>> > lldb-dev at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> > Oleksiy Vyalov | Software Engineer | ovyalov at google.com
>>> > _______________________________________________
>>> > lldb-dev mailing list
>>> > lldb-dev at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151029/defc0a8d/attachment-0001.html>