JDevlieghere abandoned this revision.
JDevlieghere added a comment.

In https://reviews.llvm.org/D45332#1059429, @labath wrote:

> In https://reviews.llvm.org/D45332#1058976, @JDevlieghere wrote:
>
> > In https://reviews.llvm.org/D45332#1058970, @zturner wrote:
> >
> > > I don't think `sys.path` is set up correctly to be able to find the 
> > > lldbtest package from the `lldb/lit` folder.
> > >
> > > These things kind of evolved separately, and the `lldb/lit` folder was 
> > > created as a place to start iterating on LLVM-style lit / FileCheck 
> > > tests.  These kind of tests -- by definition -- don't really use the SB 
> > > API, so no work was ever done to set up paths correctly so that it could 
> > > write `import lldb` or to re-use any of the other stuff from 
> > > `packages/Python`.
> > >
> > > I'm not sure what the best thing to do is, but usually the canonical 
> > > structuring is to have the test files in the same tree as the lit 
> > > configuration.  So perhaps you could put a lit configuration file in 
> > > `lldb/packages/Python/lldbsuite` and have that be separate from 
> > > `lldb/lit`, with the goal of eventually (possibly) merging them.  Then 
> > > have a separate CMake target so you'd still have `check-lldb-lit` which 
> > > goes into the `lldb/lit` directory, and another one like 
> > > `check-lldb-lit-dotest` which starts from the 
> > > `lldb/packages/Python/lldbsuite` directory.
> > >
> > > On the other hand, if you want to see how `dotest.py` sets up its 
> > > `sys.path`, have a look at `lldb/test/dotest.py`  The magic is in this 
> > > `use_lldb_suite` function, which walks backwards through the tree until 
> > > it finds the root, then dives into the `lldbsuite` folder to manually add 
> > > it to `sys.path`.
> >
> >
> > Do you feel all that outweighs the alternative of just having the format in 
> > `llvm/Utils` as is the case in this diff? We already have some LLDB 
> > specific stuff there and I would argue that conceptually it makes (at least 
> > a little) sense to have all the format living together.
>
>
> I don't think it needs to be that complex.
>
> You already have LLDB_SOURCE_DIR from `lit.site.cfg.in`, so it should only be 
> a matter of doing something like this in `lit.cfg`:
>
>   sys.path.append(os.path.join(config.lldb_src_root, 
> "whereever_is_the_format_file")
>   from something import LLDBTest
>   config.test_format = LLDBTest(...)
>
>
> If we can make things as simple as this, then I think we should move this to 
> the lldb repo.


Thanks, that did the trick :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D45332



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to