[lldb-dev] Refactoring dotest as a Python package

[Todd Fiala via lldb-dev]

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.
>
>
Hey Zachary,

Sorry I missed this.  I *think* you have this all worked out, right?

I think the way to fix this is:

* top-level dotest.py just imports whatever is needed and calls it
(something from the package, that is basically the contents of the old
dotest.py).

* The new dotest.py (inside the package) does what it used to do, calling
dosep.py when it is a concurrent run.

* Dosep.py would call the top level dotest.py when execing, with the right
flags as before to indicate that the new run is a dotest.py inferior.

I think that would do it and still get your initialization.

-Todd


> 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
>>>
>>>


-- 
-Todd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151029/2b8a2303/attachment-0001.html>