Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

I think I have a pretty good handle on what the problems are.  I'm honestly
surprised the lit test suite ever worked, even before my patch that "broke"
it.  We were basically just picking up whatever the system PATH was and
just going with it, so a lot of the substitutions weren't actually
substitutions at all, they were just random executables found on path.
That's totally not how this is supposed to work at all.

The idea is to set up a hermetic environment where we we create
substitutions for everything we care about, those resolve to things in the
build tree (with suitable hooks for overriding the search path where that's
desired), and we actually delete the system path entirely so that it's
impossible to find anything on PATH.

So I'm currently working through getting all this working the way it's
supposed to, hopefully I can have something by tomorrow.

On Wed, Nov 14, 2018 at 9:21 AM Stella Stamenova 
wrote:

> Simplifying (and making things more robust in the process) sounds great to
> me. I think the current iteration of how parameters are passed to the tests
> is quite complicated and unclear, so this will be a step in the right
> direction and if there’s a need for gcc later, we can take the time to
> design the feature properly so that it works across the lit tests and the
> test suite both.
>
>
>
> Thanks,
>
> -Stella
>
>
>
> *From:* Zachary Turner 
> *Sent:* Tuesday, November 13, 2018 4:30 PM
>
>
> *To:* Stella Stamenova 
> *Cc:* reviews+d54009+public+0e164460da8f1...@reviews.llvm.org;
> pa...@labath.sk; chris.biene...@me.com; dccitali...@gmail.com;
> aleksandr.ura...@jetbrains.com; jdevliegh...@apple.com;
> abidh@gmail.com; teempe...@gmail.com; ki.s...@gmail.com;
> mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com;
> lldb-commits@lists.llvm.org; l...@inglorion.net
> *Subject:* Re: [PATCH] D54009: Refactor LLDB lit configuration files
>
>
>
> use_clang() already will fall back on searching the environment variable
> 'CLANG' to find a path to it.
>
>
>
> self.config.clang = self.use_llvm_tool(
>
> 'clang', search_env='CLANG', required=required)
>
>
>
> But we could make this environment variable a parameter to use_clang() if
> we wanted to.  As long as we can agree that we don't need to worry about
> gcc -- at least for now -- then it should all simplify down quite a bit.
> And AFAICT, there's nobody using gcc with the lit tests right now, so it
> just adds unnecessary complexity.  And if and when we do have people using
> it, there is even more work to be done.
>
>
>
> If someone only wants a clang that isn't the just-built clang (for example
> a release version to make sure the tests run faster), all they need to do
> is set the environment variable 'CLANG' and it should be fine.
>
>
>
> Since the lit suite is still very new and developing, I'm not too
> concerned about regressing a feature (especially one with zero users),
> because the important thing to me is that it's designed right so that the
> feature can grow in organically and not be "forced" in with a subpar
> implementation.
>
>
>
> On Tue, Nov 13, 2018 at 4:23 PM Stella Stamenova 
> wrote:
>
> The plan for the lit tests sounds reasonable to me. I would also remove
> LLDB_TEST_C/CXX_COMPILER entirely so that we can reduce confusion since
> they’re only used for the lit tests, right?
>
>
>
> My only concern is that I’ve been told that there are people who will
> build lldb with a different compiler than the tests – so the properties for
> LLDB_TEST_C/CXX_COMPILER might actually be used especially in cases where
> clang is not built alongside lldb.
>
>
>
> Thanks,
>
> -Stella
>
>
>
> *From:* Zachary Turner 
> *Sent:* Tuesday, November 13, 2018 4:16 PM
>
>
> *To:* Stella Stamenova 
> *Cc:* reviews+d54009+public+0e164460da8f1...@reviews.llvm.org;
> pa...@labath.sk; chris.biene...@me.com; dccitali...@gmail.com;
> aleksandr.ura...@jetbrains.com; jdevliegh...@apple.com;
> abidh@gmail.com; teempe...@gmail.com; ki.s...@gmail.com;
> mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com;
> lldb-commits@lists.llvm.org; l...@inglorion.net
> *Subject:* Re: [PATCH] D54009: Refactor LLDB lit configuration files
>
>
>
> Ok so for dotest, it seems to be ignoring the config.cc and config.cxx
> entirely.  So we can theoretically do whatever we want with it, or change
> around the directory structure so that it's more like:
>
>
>
> lldb
>
> * lit
>
> * * Dotest
>
> * * Unit
>
> * * Tests
>
>
>
> and put the config.cc / config.cxx logic under Tests.  That's a large
> change though and probably not worth making such a large change right away.
>
>
>
> dotest tests manually construct the command line directly in CMake via
> this `LLDB_DOTEST_ARGS_PROPERTY` global property, and then in
> lldb/lit/Suite/lit.cfg we have this line:
>
>
>
> dotest_cmd = [config.dotest_path, '-q']
>
> dotest_cmd.extend(config.dotest_args_str.split(';'))
>
>
>
>
>
> So pretty much everythign the parent lit file has done is totally
> 

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via lldb-commits]

Simplifying (and making things more robust in the process) sounds great to me. 
I think the current iteration of how parameters are passed to the tests is 
quite complicated and unclear, so this will be a step in the right direction 
and if there’s a need for gcc later, we can take the time to design the feature 
properly so that it works across the lit tests and the test suite both.

Thanks,
-Stella

From: Zachary Turner 
Sent: Tuesday, November 13, 2018 4:30 PM
To: Stella Stamenova 
Cc: reviews+d54009+public+0e164460da8f1...@reviews.llvm.org; pa...@labath.sk; 
chris.biene...@me.com; dccitali...@gmail.com; aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; abidh@gmail.com; teempe...@gmail.com; 
ki.s...@gmail.com; mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com; 
lldb-commits@lists.llvm.org; l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

use_clang() already will fall back on searching the environment variable 
'CLANG' to find a path to it.

self.config.clang = self.use_llvm_tool(
'clang', search_env='CLANG', required=required)

But we could make this environment variable a parameter to use_clang() if we 
wanted to.  As long as we can agree that we don't need to worry about gcc -- at 
least for now -- then it should all simplify down quite a bit.  And AFAICT, 
there's nobody using gcc with the lit tests right now, so it just adds 
unnecessary complexity.  And if and when we do have people using it, there is 
even more work to be done.

If someone only wants a clang that isn't the just-built clang (for example a 
release version to make sure the tests run faster), all they need to do is set 
the environment variable 'CLANG' and it should be fine.

Since the lit suite is still very new and developing, I'm not too concerned 
about regressing a feature (especially one with zero users), because the 
important thing to me is that it's designed right so that the feature can grow 
in organically and not be "forced" in with a subpar implementation.

On Tue, Nov 13, 2018 at 4:23 PM Stella Stamenova 
mailto:sti...@microsoft.com>> wrote:
The plan for the lit tests sounds reasonable to me. I would also remove 
LLDB_TEST_C/CXX_COMPILER entirely so that we can reduce confusion since they’re 
only used for the lit tests, right?

My only concern is that I’ve been told that there are people who will build 
lldb with a different compiler than the tests – so the properties for 
LLDB_TEST_C/CXX_COMPILER might actually be used especially in cases where clang 
is not built alongside lldb.

Thanks,
-Stella

From: Zachary Turner mailto:ztur...@google.com>>
Sent: Tuesday, November 13, 2018 4:16 PM

To: Stella Stamenova mailto:sti...@microsoft.com>>
Cc: 
reviews+d54009+public+0e164460da8f1...@reviews.llvm.org;
 pa...@labath.sk; 
chris.biene...@me.com; 
dccitali...@gmail.com; 
aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; 
abidh@gmail.com; 
teempe...@gmail.com; 
ki.s...@gmail.com; 
mgo...@gentoo.org; 
d...@su-root.co.uk; 
jfbast...@apple.com; 
lldb-commits@lists.llvm.org; 
l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

Ok so for dotest, it seems to be ignoring the config.cc and config.cxx 
entirely.  So we can theoretically do whatever we want with it, or change 
around the directory structure so that it's more like:

lldb
* lit
* * Dotest
* * Unit
* * Tests

and put the config.cc / config.cxx logic under Tests.  That's a large change 
though and probably not worth making such a large change right away.

dotest tests manually construct the command line directly in CMake via this 
`LLDB_DOTEST_ARGS_PROPERTY` global property, and then in lldb/lit/Suite/lit.cfg 
we have this line:

dotest_cmd = [config.dotest_path, '-q']
dotest_cmd.extend(config.dotest_args_str.split(';'))


So pretty much everythign the parent lit file has done is totally ignored.

With that in mind, **for the lit tests only** I propose dropping support for 
non-clang compilers and ignoring LLDB_TEST_C_COMPILER and 
LLDB_TEST_CXX_COMPILER (you can still have a custom path to clang executable 
via an environment variable, which is consistent with how clang's test suite 
works).

Note that when you run ninja check-lldb-lit you will now get messages that tell 
you the exact path to the clang executable, so you can see what the PATH 
resolution is doing.

On Tue, Nov 13, 2018 at 4:02 PM Zachary Turner 
mailto:ztur...@google.com>> wrote:
On Tue, Nov 13, 2018 at 3:47 PM Stella Stamenova 

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

use_clang() already will fall back on searching the environment variable
'CLANG' to find a path to it.

self.config.clang = self.use_llvm_tool(
'clang', search_env='CLANG', required=required)

But we could make this environment variable a parameter to use_clang() if
we wanted to.  As long as we can agree that we don't need to worry about
gcc -- at least for now -- then it should all simplify down quite a bit.
And AFAICT, there's nobody using gcc with the lit tests right now, so it
just adds unnecessary complexity.  And if and when we do have people using
it, there is even more work to be done.

If someone only wants a clang that isn't the just-built clang (for example
a release version to make sure the tests run faster), all they need to do
is set the environment variable 'CLANG' and it should be fine.

Since the lit suite is still very new and developing, I'm not too concerned
about regressing a feature (especially one with zero users), because the
important thing to me is that it's designed right so that the feature can
grow in organically and not be "forced" in with a subpar implementation.

On Tue, Nov 13, 2018 at 4:23 PM Stella Stamenova 
wrote:

> The plan for the lit tests sounds reasonable to me. I would also remove
> LLDB_TEST_C/CXX_COMPILER entirely so that we can reduce confusion since
> they’re only used for the lit tests, right?
>
>
>
> My only concern is that I’ve been told that there are people who will
> build lldb with a different compiler than the tests – so the properties for
> LLDB_TEST_C/CXX_COMPILER might actually be used especially in cases where
> clang is not built alongside lldb.
>
>
>
> Thanks,
>
> -Stella
>
>
>
> *From:* Zachary Turner 
> *Sent:* Tuesday, November 13, 2018 4:16 PM
>
>
> *To:* Stella Stamenova 
> *Cc:* reviews+d54009+public+0e164460da8f1...@reviews.llvm.org;
> pa...@labath.sk; chris.biene...@me.com; dccitali...@gmail.com;
> aleksandr.ura...@jetbrains.com; jdevliegh...@apple.com;
> abidh@gmail.com; teempe...@gmail.com; ki.s...@gmail.com;
> mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com;
> lldb-commits@lists.llvm.org; l...@inglorion.net
> *Subject:* Re: [PATCH] D54009: Refactor LLDB lit configuration files
>
>
>
> Ok so for dotest, it seems to be ignoring the config.cc and config.cxx
> entirely.  So we can theoretically do whatever we want with it, or change
> around the directory structure so that it's more like:
>
>
>
> lldb
>
> * lit
>
> * * Dotest
>
> * * Unit
>
> * * Tests
>
>
>
> and put the config.cc / config.cxx logic under Tests.  That's a large
> change though and probably not worth making such a large change right away.
>
>
>
> dotest tests manually construct the command line directly in CMake via
> this `LLDB_DOTEST_ARGS_PROPERTY` global property, and then in
> lldb/lit/Suite/lit.cfg we have this line:
>
>
>
> dotest_cmd = [config.dotest_path, '-q']
>
> dotest_cmd.extend(config.dotest_args_str.split(';'))
>
>
>
>
>
> So pretty much everythign the parent lit file has done is totally
> ignored.
>
>
>
> With that in mind, **for the lit tests only** I propose dropping support
> for non-clang compilers and ignoring LLDB_TEST_C_COMPILER and
> LLDB_TEST_CXX_COMPILER (you can still have a custom path to clang
> executable via an environment variable, which is consistent with how
> clang's test suite works).
>
>
>
> Note that when you run ninja check-lldb-lit you will now get messages that
> tell you the exact path to the clang executable, so you can see what the
> PATH resolution is doing.
>
>
>
> On Tue, Nov 13, 2018 at 4:02 PM Zachary Turner  wrote:
>
> On Tue, Nov 13, 2018 at 3:47 PM Stella Stamenova 
> wrote:
>
> I am not sure if that’s the right solution for a couple of reasons:
>
>1. As far as I can tell only clang calls use_clang (and only lld calls
>use_lld), while the other projects such as lld and llvm rely on the
>environment to be setup correctly
>2. Lld also has tests that invoke clang-cl and they pass – while the
>ones in LLDB do not, so the invocation of use_clang is not necessary for
>the tests to pass (maybe?)
>3. LLDB allows us to specify whether to use gcc or clang as well as
>the path and it can also have a test compiler specified via
>LLDB_USE_TEST_*_COMPILER, so we should first decide what scenarios we want
>to support before trying to make this work and possibly making it even more
>confusing and complicated
>
>
>
> Do you know what the answer for 3) is? What compilers are valid to specify
> for the lit/suite/unittests via the various parameters?
>
>
>
> For the unit tests, I don't think we ever specify a compiler, or we don't
> ever read the value.  Because a unit test shouldn't be compiling anything,
> it's a different kind of test.
>
>
>
> For the dotest suite, specifying the compiler is important and it can
> definitely be gcc, but I don't think this uses the same method of going
> through config.cc.  In fact, I'm not sure how it determines what compiler

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via lldb-commits]

The plan for the lit tests sounds reasonable to me. I would also remove 
LLDB_TEST_C/CXX_COMPILER entirely so that we can reduce confusion since they’re 
only used for the lit tests, right?

My only concern is that I’ve been told that there are people who will build 
lldb with a different compiler than the tests – so the properties for 
LLDB_TEST_C/CXX_COMPILER might actually be used especially in cases where clang 
is not built alongside lldb.

Thanks,
-Stella

From: Zachary Turner 
Sent: Tuesday, November 13, 2018 4:16 PM
To: Stella Stamenova 
Cc: reviews+d54009+public+0e164460da8f1...@reviews.llvm.org; pa...@labath.sk; 
chris.biene...@me.com; dccitali...@gmail.com; aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; abidh@gmail.com; teempe...@gmail.com; 
ki.s...@gmail.com; mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com; 
lldb-commits@lists.llvm.org; l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

Ok so for dotest, it seems to be ignoring the config.cc and config.cxx 
entirely.  So we can theoretically do whatever we want with it, or change 
around the directory structure so that it's more like:

lldb
* lit
* * Dotest
* * Unit
* * Tests

and put the config.cc / config.cxx logic under Tests.  That's a large change 
though and probably not worth making such a large change right away.

dotest tests manually construct the command line directly in CMake via this 
`LLDB_DOTEST_ARGS_PROPERTY` global property, and then in lldb/lit/Suite/lit.cfg 
we have this line:

dotest_cmd = [config.dotest_path, '-q']
dotest_cmd.extend(config.dotest_args_str.split(';'))


So pretty much everythign the parent lit file has done is totally ignored.

With that in mind, **for the lit tests only** I propose dropping support for 
non-clang compilers and ignoring LLDB_TEST_C_COMPILER and 
LLDB_TEST_CXX_COMPILER (you can still have a custom path to clang executable 
via an environment variable, which is consistent with how clang's test suite 
works).

Note that when you run ninja check-lldb-lit you will now get messages that tell 
you the exact path to the clang executable, so you can see what the PATH 
resolution is doing.

On Tue, Nov 13, 2018 at 4:02 PM Zachary Turner 
mailto:ztur...@google.com>> wrote:
On Tue, Nov 13, 2018 at 3:47 PM Stella Stamenova 
mailto:sti...@microsoft.com>> wrote:
I am not sure if that’s the right solution for a couple of reasons:

  1.  As far as I can tell only clang calls use_clang (and only lld calls 
use_lld), while the other projects such as lld and llvm rely on the environment 
to be setup correctly
  2.  Lld also has tests that invoke clang-cl and they pass – while the ones in 
LLDB do not, so the invocation of use_clang is not necessary for the tests to 
pass (maybe?)
  3.  LLDB allows us to specify whether to use gcc or clang as well as the path 
and it can also have a test compiler specified via LLDB_USE_TEST_*_COMPILER, so 
we should first decide what scenarios we want to support before trying to make 
this work and possibly making it even more confusing and complicated

Do you know what the answer for 3) is? What compilers are valid to specify for 
the lit/suite/unittests via the various parameters?

For the unit tests, I don't think we ever specify a compiler, or we don't ever 
read the value.  Because a unit test shouldn't be compiling anything, it's a 
different kind of test.

For the dotest suite, specifying the compiler is important and it can 
definitely be gcc, but I don't think this uses the same method of going through 
config.cc.  In fact, I'm not sure how it determines what compiler to use at the 
moment, as it's been a number of years since I've looked at the dotest suite.

For the lit tests, I'm inclined to say we should keep things simple and only 
support clang for now, and add support for new compilers such as gcc if and 
when someone actually wants it.  Otherwise YAGNI.

Definitely that time will come, but it doesn't make sense to support it 
immediately if nobody is using it today and nobody is planning to enable it 
immediately.

So I'm tempted to say that perhaps we should just call llvm_config.use_clang() 
and llvm_config.use_lld() and ignore LLDB_TEST_COMPILER, which in my experience 
has only been a source of unnecessary complexity that never actually gets used 
in practice.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

Ok so for dotest, it seems to be ignoring the config.cc and config.cxx
entirely.  So we can theoretically do whatever we want with it, or change
around the directory structure so that it's more like:

lldb
* lit
* * Dotest
* * Unit
* * Tests

and put the config.cc / config.cxx logic under Tests.  That's a large
change though and probably not worth making such a large change right away.

dotest tests manually construct the command line directly in CMake via this
`LLDB_DOTEST_ARGS_PROPERTY` global property, and then in
lldb/lit/Suite/lit.cfg we have this line:

dotest_cmd = [config.dotest_path, '-q']
dotest_cmd.extend(config.dotest_args_str.split(';'))


So pretty much everythign the parent lit file has done is totally ignored.

With that in mind, **for the lit tests only** I propose dropping support
for non-clang compilers and ignoring LLDB_TEST_C_COMPILER and
LLDB_TEST_CXX_COMPILER (you can still have a custom path to clang
executable via an environment variable, which is consistent with how
clang's test suite works).

Note that when you run ninja check-lldb-lit you will now get messages that
tell you the exact path to the clang executable, so you can see what the
PATH resolution is doing.

On Tue, Nov 13, 2018 at 4:02 PM Zachary Turner  wrote:

> On Tue, Nov 13, 2018 at 3:47 PM Stella Stamenova 
> wrote:
>
>> I am not sure if that’s the right solution for a couple of reasons:
>>
>>1. As far as I can tell only clang calls use_clang (and only lld
>>calls use_lld), while the other projects such as lld and llvm rely on the
>>environment to be setup correctly
>>2. Lld also has tests that invoke clang-cl and they pass – while the
>>ones in LLDB do not, so the invocation of use_clang is not necessary for
>>the tests to pass (maybe?)
>>3. LLDB allows us to specify whether to use gcc or clang as well as
>>the path and it can also have a test compiler specified via
>>LLDB_USE_TEST_*_COMPILER, so we should first decide what scenarios we want
>>to support before trying to make this work and possibly making it even 
>> more
>>confusing and complicated
>>
>>
>>
>> Do you know what the answer for 3) is? What compilers are valid to
>> specify for the lit/suite/unittests via the various parameters?
>>
>
> For the unit tests, I don't think we ever specify a compiler, or we don't
> ever read the value.  Because a unit test shouldn't be compiling anything,
> it's a different kind of test.
>
> For the dotest suite, specifying the compiler is important and it can
> definitely be gcc, but I don't think this uses the same method of going
> through config.cc.  In fact, I'm not sure how it determines what compiler
> to use at the moment, as it's been a number of years since I've looked at
> the dotest suite.
>
> For the lit tests, I'm inclined to say we should keep things simple and
> only support clang for now, and add support for new compilers such as gcc
> if and when someone actually wants it.  Otherwise YAGNI.
>
> Definitely that time will come, but it doesn't make sense to support it
> immediately if nobody is using it today and nobody is planning to enable it
> immediately.
>
> So I'm tempted to say that perhaps we should just call
> llvm_config.use_clang() and llvm_config.use_lld() and ignore
> LLDB_TEST_COMPILER, which in my experience has only been a source of
> unnecessary complexity that never actually gets used in practice.
>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

On Tue, Nov 13, 2018 at 3:47 PM Stella Stamenova 
wrote:

> I am not sure if that’s the right solution for a couple of reasons:
>
>1. As far as I can tell only clang calls use_clang (and only lld calls
>use_lld), while the other projects such as lld and llvm rely on the
>environment to be setup correctly
>2. Lld also has tests that invoke clang-cl and they pass – while the
>ones in LLDB do not, so the invocation of use_clang is not necessary for
>the tests to pass (maybe?)
>3. LLDB allows us to specify whether to use gcc or clang as well as
>the path and it can also have a test compiler specified via
>LLDB_USE_TEST_*_COMPILER, so we should first decide what scenarios we want
>to support before trying to make this work and possibly making it even more
>confusing and complicated
>
>
>
> Do you know what the answer for 3) is? What compilers are valid to specify
> for the lit/suite/unittests via the various parameters?
>

For the unit tests, I don't think we ever specify a compiler, or we don't
ever read the value.  Because a unit test shouldn't be compiling anything,
it's a different kind of test.

For the dotest suite, specifying the compiler is important and it can
definitely be gcc, but I don't think this uses the same method of going
through config.cc.  In fact, I'm not sure how it determines what compiler
to use at the moment, as it's been a number of years since I've looked at
the dotest suite.

For the lit tests, I'm inclined to say we should keep things simple and
only support clang for now, and add support for new compilers such as gcc
if and when someone actually wants it.  Otherwise YAGNI.

Definitely that time will come, but it doesn't make sense to support it
immediately if nobody is using it today and nobody is planning to enable it
immediately.

So I'm tempted to say that perhaps we should just call
llvm_config.use_clang() and llvm_config.use_lld() and ignore
LLDB_TEST_COMPILER, which in my experience has only been a source of
unnecessary complexity that never actually gets used in practice.

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

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via lldb-commits]

Actually, I take 2) back. Lld doesn’t call use_clang, but it only references 
clang-cl, it doesn’t expect it to execute.

From: Stella Stamenova
Sent: Tuesday, November 13, 2018 3:47 PM
To: 'Zachary Turner' 
Cc: reviews+d54009+public+0e164460da8f1...@reviews.llvm.org; pa...@labath.sk; 
chris.biene...@me.com; dccitali...@gmail.com; aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; abidh@gmail.com; teempe...@gmail.com; 
ki.s...@gmail.com; mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com; 
lldb-commits@lists.llvm.org; l...@inglorion.net
Subject: RE: [PATCH] D54009: Refactor LLDB lit configuration files

I am not sure if that’s the right solution for a couple of reasons:

  1.  As far as I can tell only clang calls use_clang (and only lld calls 
use_lld), while the other projects such as lld and llvm rely on the environment 
to be setup correctly
  2.  Lld also has tests that invoke clang-cl and they pass – while the ones in 
LLDB do not, so the invocation of use_clang is not necessary for the tests to 
pass (maybe?)
  3.  LLDB allows us to specify whether to use gcc or clang as well as the path 
and it can also have a test compiler specified via LLDB_USE_TEST_*_COMPILER, so 
we should first decide what scenarios we want to support before trying to make 
this work and possibly making it even more confusing and complicated

Do you know what the answer for 3) is? What compilers are valid to specify for 
the lit/suite/unittests via the various parameters?

Thanks,
-Stella


From: Zachary Turner mailto:ztur...@google.com>>
Sent: Tuesday, November 13, 2018 3:27 PM
To: Stella Stamenova mailto:sti...@microsoft.com>>
Cc: 
reviews+d54009+public+0e164460da8f1...@reviews.llvm.org;
 pa...@labath.sk; 
chris.biene...@me.com; 
dccitali...@gmail.com; 
aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; 
abidh@gmail.com; 
teempe...@gmail.com; 
ki.s...@gmail.com; 
mgo...@gentoo.org; 
d...@su-root.co.uk; 
jfbast...@apple.com; 
lldb-commits@lists.llvm.org; 
l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

I believe that is correct, and perhaps part of the problem.  In other projects 
we call llvm_config.use_clang(), and that actually explicitly creates a 
substitution so that if someone writes "clang" they'll get an error that says 
"use %clang instead".  Because we have the exact path, that isn't happening 
here.  Perhaps the proper fix is to add a keyword argument to 
llvm_config.use_clang() so that it can be called as 
llvm_config.use_clang(path=p).  This way we can get the proper substitutions 
created and we would find out about this.

I can play around with this some, but does it sound reasonable to you?  I'm not 
sure how to hit all the edge cases because everything is already working for me 
(for some reason), so it won't be obvious if I fix the problem for those people 
for whom it's broken.

On Tue, Nov 13, 2018 at 3:16 PM Stella Stamenova 
mailto:sti...@microsoft.com>> wrote:
I took a brief look and I have a question about the usage of clang (rather than 
clang-cl).

In general I would agree that we have an exact path of clang (or gcc) that we 
are trying to use and they’re specified by using %cc and %cxx in the test 
files, but there are a number of test files that simply use clang e.g.:

SymbolFile\DWARF\find-variable-dwo.cpp:3:// RUN: clang %s -g -gsplit-dwarf -c 
-emit-llvm -o - --target=x86_64-pc-linux -DONE

In this case, are we not going to pick up whatever clang happens to be in the 
path instead of one that was explicitly specified? Is this intentional?

Thanks,
-Stella

From: Zachary Turner mailto:ztur...@google.com>>
Sent: Tuesday, November 13, 2018 2:46 PM
To: 
reviews+d54009+public+0e164460da8f1...@reviews.llvm.org
Cc: Stella Stamenova mailto:sti...@microsoft.com>>; 
pa...@labath.sk; 
chris.biene...@me.com; 
dccitali...@gmail.com; 
aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; 
abidh@gmail.com; 
teempe...@gmail.com; 
ki.s...@gmail.com; 
mgo...@gentoo.org; 
d...@su-root.co.uk; 
jfbast...@apple.com; 
lldb-commits@lists.llvm.org; 
l...@inglorion.net

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via lldb-commits]

I am not sure if that’s the right solution for a couple of reasons:

  1.  As far as I can tell only clang calls use_clang (and only lld calls 
use_lld), while the other projects such as lld and llvm rely on the environment 
to be setup correctly
  2.  Lld also has tests that invoke clang-cl and they pass – while the ones in 
LLDB do not, so the invocation of use_clang is not necessary for the tests to 
pass (maybe?)
  3.  LLDB allows us to specify whether to use gcc or clang as well as the path 
and it can also have a test compiler specified via LLDB_USE_TEST_*_COMPILER, so 
we should first decide what scenarios we want to support before trying to make 
this work and possibly making it even more confusing and complicated

Do you know what the answer for 3) is? What compilers are valid to specify for 
the lit/suite/unittests via the various parameters?

Thanks,
-Stella


From: Zachary Turner 
Sent: Tuesday, November 13, 2018 3:27 PM
To: Stella Stamenova 
Cc: reviews+d54009+public+0e164460da8f1...@reviews.llvm.org; pa...@labath.sk; 
chris.biene...@me.com; dccitali...@gmail.com; aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; abidh@gmail.com; teempe...@gmail.com; 
ki.s...@gmail.com; mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com; 
lldb-commits@lists.llvm.org; l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

I believe that is correct, and perhaps part of the problem.  In other projects 
we call llvm_config.use_clang(), and that actually explicitly creates a 
substitution so that if someone writes "clang" they'll get an error that says 
"use %clang instead".  Because we have the exact path, that isn't happening 
here.  Perhaps the proper fix is to add a keyword argument to 
llvm_config.use_clang() so that it can be called as 
llvm_config.use_clang(path=p).  This way we can get the proper substitutions 
created and we would find out about this.

I can play around with this some, but does it sound reasonable to you?  I'm not 
sure how to hit all the edge cases because everything is already working for me 
(for some reason), so it won't be obvious if I fix the problem for those people 
for whom it's broken.

On Tue, Nov 13, 2018 at 3:16 PM Stella Stamenova 
mailto:sti...@microsoft.com>> wrote:
I took a brief look and I have a question about the usage of clang (rather than 
clang-cl).

In general I would agree that we have an exact path of clang (or gcc) that we 
are trying to use and they’re specified by using %cc and %cxx in the test 
files, but there are a number of test files that simply use clang e.g.:

SymbolFile\DWARF\find-variable-dwo.cpp:3:// RUN: clang %s -g -gsplit-dwarf -c 
-emit-llvm -o - --target=x86_64-pc-linux -DONE

In this case, are we not going to pick up whatever clang happens to be in the 
path instead of one that was explicitly specified? Is this intentional?

Thanks,
-Stella

From: Zachary Turner mailto:ztur...@google.com>>
Sent: Tuesday, November 13, 2018 2:46 PM
To: 
reviews+d54009+public+0e164460da8f1...@reviews.llvm.org
Cc: Stella Stamenova mailto:sti...@microsoft.com>>; 
pa...@labath.sk; 
chris.biene...@me.com; 
dccitali...@gmail.com; 
aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; 
abidh@gmail.com; 
teempe...@gmail.com; 
ki.s...@gmail.com; 
mgo...@gentoo.org; 
d...@su-root.co.uk; 
jfbast...@apple.com; 
lldb-commits@lists.llvm.org; 
l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

I think it must be related to setting up the environment in which to run clang. 
 In all other projects we call llvm_config.use_clang() which is in 
llvm/utils/lit/lit/llvm/config.py, but because here we have an exact path of a 
clang we are trying to use, we skip this function in LLDB's lit configuration 
files.  But there is also a lot of other logic in that function, so perhaps 
it's some of that logic that's necessary.

On Mon, Nov 12, 2018 at 9:02 AM Aleksandr Urakov via Phabricator 
mailto:revi...@reviews.llvm.org>> wrote:
aleksandr.urakov added a comment.

But all compiles without errors if I run this manually:

  clang-cl -m32 /Z7 /c /GS- 
C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp /o 
C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj


Repository:
  rLLDB LLDB

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

I believe that is correct, and perhaps part of the problem.  In other
projects we call llvm_config.use_clang(), and that actually explicitly
creates a substitution so that if someone writes "clang" they'll get an
error that says "use %clang instead".  Because we have the exact path, that
isn't happening here.  Perhaps the proper fix is to add a keyword argument
to llvm_config.use_clang() so that it can be called as
llvm_config.use_clang(path=p).  This way we can get the proper
substitutions created and we would find out about this.

I can play around with this some, but does it sound reasonable to you?  I'm
not sure how to hit all the edge cases because everything is already
working for me (for some reason), so it won't be obvious if I fix the
problem for those people for whom it's broken.

On Tue, Nov 13, 2018 at 3:16 PM Stella Stamenova 
wrote:

> I took a brief look and I have a question about the usage of clang (rather
> than clang-cl).
>
>
>
> In general I would agree that we have an exact path of clang (or gcc) that
> we are trying to use and they’re specified by using %cc and %cxx in the
> test files, but there are a number of test files that simply use clang e.g.:
>
>
>
> SymbolFile\DWARF\find-variable-dwo.cpp:3:// RUN: clang %s -g -gsplit-dwarf
> -c -emit-llvm -o - --target=x86_64-pc-linux -DONE
>
>
>
> In this case, are we not going to pick up whatever clang happens to be in
> the path instead of one that was explicitly specified? Is this intentional?
>
>
>
> Thanks,
>
> -Stella
>
>
>
> *From:* Zachary Turner 
> *Sent:* Tuesday, November 13, 2018 2:46 PM
> *To:* reviews+d54009+public+0e164460da8f1...@reviews.llvm.org
> *Cc:* Stella Stamenova ; pa...@labath.sk;
> chris.biene...@me.com; dccitali...@gmail.com;
> aleksandr.ura...@jetbrains.com; jdevliegh...@apple.com;
> abidh@gmail.com; teempe...@gmail.com; ki.s...@gmail.com;
> mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com;
> lldb-commits@lists.llvm.org; l...@inglorion.net
> *Subject:* Re: [PATCH] D54009: Refactor LLDB lit configuration files
>
>
>
> I think it must be related to setting up the environment in which to run
> clang.  In all other projects we call llvm_config.use_clang() which is in
> llvm/utils/lit/lit/llvm/config.py, but because here we have an exact path
> of a clang we are trying to use, we skip this function in LLDB's lit
> configuration files.  But there is also a lot of other logic in that
> function, so perhaps it's some of that logic that's necessary.
>
>
>
> On Mon, Nov 12, 2018 at 9:02 AM Aleksandr Urakov via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> aleksandr.urakov added a comment.
>
> But all compiles without errors if I run this manually:
>
>   clang-cl -m32 /Z7 /c /GS-
> C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp /o
> C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D54009
> 
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via lldb-commits]

I took a brief look and I have a question about the usage of clang (rather than 
clang-cl).

In general I would agree that we have an exact path of clang (or gcc) that we 
are trying to use and they’re specified by using %cc and %cxx in the test 
files, but there are a number of test files that simply use clang e.g.:

SymbolFile\DWARF\find-variable-dwo.cpp:3:// RUN: clang %s -g -gsplit-dwarf -c 
-emit-llvm -o - --target=x86_64-pc-linux -DONE

In this case, are we not going to pick up whatever clang happens to be in the 
path instead of one that was explicitly specified? Is this intentional?

Thanks,
-Stella

From: Zachary Turner 
Sent: Tuesday, November 13, 2018 2:46 PM
To: reviews+d54009+public+0e164460da8f1...@reviews.llvm.org
Cc: Stella Stamenova ; pa...@labath.sk; 
chris.biene...@me.com; dccitali...@gmail.com; aleksandr.ura...@jetbrains.com; 
jdevliegh...@apple.com; abidh@gmail.com; teempe...@gmail.com; 
ki.s...@gmail.com; mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com; 
lldb-commits@lists.llvm.org; l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files

I think it must be related to setting up the environment in which to run clang. 
 In all other projects we call llvm_config.use_clang() which is in 
llvm/utils/lit/lit/llvm/config.py, but because here we have an exact path of a 
clang we are trying to use, we skip this function in LLDB's lit configuration 
files.  But there is also a lot of other logic in that function, so perhaps 
it's some of that logic that's necessary.

On Mon, Nov 12, 2018 at 9:02 AM Aleksandr Urakov via Phabricator 
mailto:revi...@reviews.llvm.org>> wrote:
aleksandr.urakov added a comment.

But all compiles without errors if I run this manually:

  clang-cl -m32 /Z7 /c /GS- 
C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp /o 
C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009


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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

I think it must be related to setting up the environment in which to run
clang.  In all other projects we call llvm_config.use_clang() which is in
llvm/utils/lit/lit/llvm/config.py, but because here we have an exact path
of a clang we are trying to use, we skip this function in LLDB's lit
configuration files.  But there is also a lot of other logic in that
function, so perhaps it's some of that logic that's necessary.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

I think it must be related to setting up the environment in which to run
clang.  In all other projects we call llvm_config.use_clang() which is in
llvm/utils/lit/lit/llvm/config.py, but because here we have an exact path
of a clang we are trying to use, we skip this function in LLDB's lit
configuration files.  But there is also a lot of other logic in that
function, so perhaps it's some of that logic that's necessary.

On Mon, Nov 12, 2018 at 9:02 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:

> aleksandr.urakov added a comment.
>
> But all compiles without errors if I run this manually:
>
>   clang-cl -m32 /Z7 /c /GS-
> C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp /o
> C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D54009
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

But all compiles without errors if I run this manually:

  clang-cl -m32 /Z7 /c /GS- 
C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp /o 
C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

I also have some strange failures on Windows x86 (I run tests from x64_86 MSVC 
command line). If I locally revert this commit and 3 fix commits right after 
this one, then all seems to work. Here is the failure:

  C:\Work\llvm\build_x86\bin>llvm-lit.py -v 
C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB\enums-layout.test
  llvm-lit.py: C:/Work/llvm\utils\lit\lit\llvm\config.py:333: note: using 
C:/Work/llvm/build_x86/./bin/clang.exe: C:\Work\llvm\build_x86\bin\clang.exe
  llvm-lit.py: C:/Work/llvm\utils\lit\lit\llvm\config.py:333: note: using 
C:/Work/llvm/build_x86/./bin/clang++.exe: C:\Work\llvm\build_x86\bin\clang++.exe
  config.cc = C:\Work\llvm\build_x86\bin\clang.exe
  -- Testing: 1 tests, 1 threads --
  FAIL: LLDB :: SymbolFile/PDB/enums-layout.test (1 of 1)
   TEST 'LLDB :: SymbolFile/PDB/enums-layout.test' FAILED 

  Script:
  --
  : 'RUN: at line 2';   clang-cl -m32 /Z7 /c /GS- 
C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp /o 
C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj
  : 'RUN: at line 3';   link 
C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj
 /DEBUG /nodefaultlib /ENTRY:main 
/OUT:C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.exe
  : 'RUN: at line 4';   C:\Work\llvm\build_x86\bin\lldb-test.EXE symbols 
C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.exe
 | C:\Work\llvm\build_x86\bin\FileCheck.EXE 
C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB\enums-layout.test
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 2"
  $ "clang-cl" "-m32" "/Z7" "/c" "/GS-" 
"C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp" "/o" 
"C:\Work\llvm\build_x86\tools\lldb\lit\SymbolFile\PDB\Output/SimpleTypesTest.cpp.enums.obj"
  # command stderr:
  C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp(41,9) : 
 error: unknown type name 'char16_t'
  typedef char16_t WChar16Typedef;
  ^
  C:\Work\llvm\tools\lldb\lit\SymbolFile\PDB/Inputs/SimpleTypesTest.cpp(44,9) : 
 error: unknown type name 'char32_t'
  typedef char32_t WChar32Typedef;
  ^
  2 errors generated.
  
  error: command failed with exit status: 1
  
  --
  
  
  Testing Time: 0.33s
  
  Failing Tests (1):
  LLDB :: SymbolFile/PDB/enums-layout.test
  
Unexpected Failures: 1

I can't figure out yet what can be wrong with it. May be you have some ideas 
how to fix this?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

The builds don't specify the two options for LLDB_TEST_C/CXX_COMPILER, so they 
should be picking up the freshly build compilers. We don't have an option for 
clang-cl though - so it's never been explicitly specified.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Actually maybe it’s the other way around. Are you specifying
LLDB_TEST_COMPILER? If it’s picking up VS’s version, it would definitely be
able to find link.exe


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

Actually maybe it’s the other way around. Are you specifying
LLDB_TEST_COMPILER? If it’s picking up VS’s version, it would definitely be
able to find link.exe
On Wed, Nov 7, 2018 at 2:07 PM Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> wrote:

> stella.stamenova added a comment.
>
> I haven't verified this yet - but I suspect it is now picking up the
> clang-cl that comes with VS rather than the one that was just built.
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D54009
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

I haven't verified this yet - but I suspect it is now picking up the clang-cl 
that comes with VS rather than the one that was just built.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

It’s possible we lost some environment variable propagation, that would do
it. But I’m curious how it was finding the visual studio installation
before my patch.

It also looks like it’s failing finding link.exe (we really should make
lld-link the default). Another fix is to pass -fuse-ld=lld or split the
clang-cl line to separate compiler and linker invocations


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

It’s possible we lost some environment variable propagation, that would do
it. But I’m curious how it was finding the visual studio installation
before my patch.

It also looks like it’s failing finding link.exe (we really should make
lld-link the default). Another fix is to pass -fuse-ld=lld or split the
clang-cl line to separate compiler and linker invocations
On Wed, Nov 7, 2018 at 1:44 PM Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> wrote:

> stella.stamenova added a comment.
>
> In https://reviews.llvm.org/D54009#1290644, @zturner wrote:
>
> > I have not run the dotest suite recently, is that where you’re seeing the
> >  failures? I successfully ran the lit suite and unit tests though
>
>
> Yes, they are primarily in the lldb-suite but not only:
>
>
> http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio
>
> There are some other failures, but what I suspect happened after this
> change is that the tests are now not picking up clang-cl correctly:
>
>   $ "clang-cl" "/Zi"
> "E:\build_slave\lldb-x64-windows-ninja\llvm\tools\lldb\lit\Expr/Inputs/call-function.cpp"
> "-o"
> "E:\build_slave\lldb-x64-windows-ninja\build\tools\lldb\lit\Expr\Output\TestIRMemoryMapWindows.test.tmp"
>   # command stderr:
>   clang-cl: warning: unable to find a Visual Studio installation; try
> running Clang from a developer command prompt [-Wmsvc-not-found]
>
>   clang-cl: error: unable to execute command: program not executable
>
>   clang-cl: error: linker command failed with exit code 1 (use -v to see
> invocation)
>
> I will have some time to have a look in more detail tomorrow.
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D54009
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

In https://reviews.llvm.org/D54009#1290644, @zturner wrote:

> I have not run the dotest suite recently, is that where you’re seeing the
>  failures? I successfully ran the lit suite and unit tests though


Yes, they are primarily in the lldb-suite but not only:

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio

There are some other failures, but what I suspect happened after this change is 
that the tests are now not picking up clang-cl correctly:

  $ "clang-cl" "/Zi" 
"E:\build_slave\lldb-x64-windows-ninja\llvm\tools\lldb\lit\Expr/Inputs/call-function.cpp"
 "-o" 
"E:\build_slave\lldb-x64-windows-ninja\build\tools\lldb\lit\Expr\Output\TestIRMemoryMapWindows.test.tmp"
  # command stderr:
  clang-cl: warning: unable to find a Visual Studio installation; try running 
Clang from a developer command prompt [-Wmsvc-not-found]
  
  clang-cl: error: unable to execute command: program not executable
  
  clang-cl: error: linker command failed with exit code 1 (use -v to see 
invocation)

I will have some time to have a look in more detail tomorrow.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

I have not run the dotest suite recently, is that where you’re seeing the
failures? I successfully ran the lit suite and unit tests though


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

I have not run the dotest suite recently, is that where you’re seeing the
failures? I successfully ran the lit suite and unit tests though
On Wed, Nov 7, 2018 at 1:32 PM Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> wrote:

> stella.stamenova added a comment.
>
> Several of the windows tests that invoke clang-cl have started failing
> recently (I am not sure exactly when) and I suspect this change is the
> culprit. Were you able to run the tests successfully with this change?
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D54009
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

Several of the windows tests that invoke clang-cl have started failing recently 
(I am not sure exactly when) and I suspect this change is the culprit. Were you 
able to run the tests successfully with this change?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

In https://reviews.llvm.org/D54009#1284839, @zturner wrote:

> In https://reviews.llvm.org/D54009#1284827, @stella.stamenova wrote:
>
> > Looks good. The formatting in lit.cfg.py is a bit messy (indentations, 
> > especially), but as long as the tests pass, this is an improvement :). 
> > Thanks!
>
>
> Is there something like clang-format for Python?  Happy to fix it if so.  (I 
> don't do a lot of Python so it's hard for me to eyeball it and figure out 
> what's wrong, so I have to say it looks fine to me :))


There's no clang-format for Python that I know of. Since python cares about 
indentation, it's always a good idea to make sure that we use the same number 
of spaces for all of the 'if's and such as it will occasionally lead to hard to 
track down bugs. There are (in the final version) of lit.cfg.py a couple of 
places where two spaces make the indentation and everything else uses four. 
It's not a big deal because of where they are, but something to look out for.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a subscriber: stella.stamenova.
zturner added a comment.

Fix incoming, sorry about that.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via lldb-commits]

Fix incoming, sorry about that.

On Fri, Nov 2, 2018 at 2:57 PM Jonas Devlieghere via Phabricator <
revi...@reviews.llvm.org> wrote:

> JDevlieghere added a comment.
>
> Hi Zachary, looks like this broke GreenDragon:
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12087/console
>
> Can you have a look please?
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D54009
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Hi Zachary, looks like this broke GreenDragon: 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12087/console

Can you have a look please?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB346008: Refactor the lit configuration files (authored by 
zturner, committed by ).
Herald added subscribers: teemperor, abidh.

Changed prior to commit:
  https://reviews.llvm.org/D54009?vs=172254=172399#toc

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54009

Files:
  lit/Breakpoint/case-insensitive.test
  lit/Breakpoint/lit.local.cfg
  lit/CMakeLists.txt
  lit/Expr/TestIRMemoryMapWindows.test
  lit/Expr/lit.local.cfg
  lit/Quit/lit.local.cfg
  lit/Settings/lit.local.cfg
  lit/SymbolFile/NativePDB/lit.local.cfg
  lit/SymbolFile/PDB/ast-restore.test
  lit/SymbolFile/PDB/calling-conventions.test
  lit/SymbolFile/PDB/class-layout.test
  lit/SymbolFile/PDB/compilands.test
  lit/SymbolFile/PDB/enums-layout.test
  lit/SymbolFile/PDB/func-symbols.test
  lit/SymbolFile/PDB/function-level-linking.test
  lit/SymbolFile/PDB/function-nested-block.test
  lit/SymbolFile/PDB/lit.local.cfg
  lit/SymbolFile/PDB/pointers.test
  lit/SymbolFile/PDB/type-quals.test
  lit/SymbolFile/PDB/typedefs.test
  lit/SymbolFile/PDB/udt-layout.test
  lit/SymbolFile/PDB/variables-locations.test
  lit/SymbolFile/PDB/variables.test
  lit/Unit/lit.cfg
  lit/Unit/lit.cfg.py
  lit/Unit/lit.site.cfg.in
  lit/Unit/lit.site.cfg.py.in
  lit/lit.cfg
  lit/lit.cfg.py
  lit/lit.site.cfg.in
  lit/lit.site.cfg.py.in
  lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
  lit/tools/lldb-mi/breakpoint/break-insert.test
  lit/tools/lldb-mi/data/data-info-line.test
  lit/tools/lldb-mi/exec/exec-continue.test
  lit/tools/lldb-mi/exec/exec-finish.test
  lit/tools/lldb-mi/exec/exec-interrupt.test
  lit/tools/lldb-mi/exec/exec-next-instruction.test
  lit/tools/lldb-mi/exec/exec-next.test
  lit/tools/lldb-mi/exec/exec-step-instruction.test
  lit/tools/lldb-mi/exec/exec-step.test
  lit/tools/lldb-mi/symbol/symbol-list-lines.test

Index: lit/tools/lldb-mi/breakpoint/break-insert.test
===
--- lit/tools/lldb-mi/breakpoint/break-insert.test
+++ lit/tools/lldb-mi/breakpoint/break-insert.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o a.exe %p/inputs/break-insert.c -g
Index: lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
===
--- lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
+++ lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/break-insert-pending.c -g
Index: lit/tools/lldb-mi/data/data-info-line.test
===
--- lit/tools/lldb-mi/data/data-info-line.test
+++ lit/tools/lldb-mi/data/data-info-line.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/data-info-line.c -g
Index: lit/tools/lldb-mi/symbol/symbol-list-lines.test
===
--- lit/tools/lldb-mi/symbol/symbol-list-lines.test
+++ lit/tools/lldb-mi/symbol/symbol-list-lines.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c %p/inputs/symbol-list-lines.c %p/inputs/list-lines-helper.c -g
Index: lit/tools/lldb-mi/exec/exec-finish.test
===
--- lit/tools/lldb-mi/exec/exec-finish.test
+++ lit/tools/lldb-mi/exec/exec-finish.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c -g
Index: lit/tools/lldb-mi/exec/exec-next.test
===
--- lit/tools/lldb-mi/exec/exec-next.test
+++ lit/tools/lldb-mi/exec/exec-next.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c -g
Index: lit/tools/lldb-mi/exec/exec-next-instruction.test
===
--- lit/tools/lldb-mi/exec/exec-next-instruction.test
+++ lit/tools/lldb-mi/exec/exec-next-instruction.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c -g
Index: lit/tools/lldb-mi/exec/exec-interrupt.test
===
--- lit/tools/lldb-mi/exec/exec-interrupt.test
+++ lit/tools/lldb-mi/exec/exec-interrupt.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c -g
Index: lit/tools/lldb-mi/exec/exec-step.test
===
--- lit/tools/lldb-mi/exec/exec-step.test
+++ lit/tools/lldb-mi/exec/exec-step.test
@@ -1,4 +1,4 @@
-# XFAIL: 

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D54009#1284827, @stella.stamenova wrote:

> Looks good. The formatting in lit.cfg.py is a bit messy (indentations, 
> especially), but as long as the tests pass, this is an improvement :). Thanks!


Is there something like clang-format for Python?  Happy to fix it if so.  (I 
don't do a lot of Python so it's hard for me to eyeball it and figure out 
what's wrong, so I have to say it looks fine to me :))


https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova accepted this revision.
stella.stamenova added a comment.
This revision is now accepted and ready to land.

Looks good. The formatting in lit.cfg.py is a bit messy (indentations, 
especially), but as long as the tests pass, this is an improvement :). Thanks!


https://reviews.llvm.org/D54009



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

[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added reviewers: stella.stamenova, labath, beanz, davide.
Herald added subscribers: jfb, delcypher, mgorny, ki.stfu.

A year or so ago, I re-wrote most of the lit infrastructure in LLVM so that it 
wasn't so boilerplate-y.  I added lots of common helper type stuff, simplifed 
usage patterns, and made the code more elegant and maintainable.

We migrated to this in LLVM, clang, and lld's lit files, but not in LLDBs.  
This started to bite me recently, as the 4 most recent times I tried to run the 
lit test suite in LLDB on a fresh checkout the first thing that would happen is 
that python would just start crashing with unhelpful backtraces and I would 
have to spend time investigating.

You can reproduce this today by doing a fresh cmake generation, doing `ninja 
lldb` and then `python bin/llvm-lit.py -sv ~/lldb/lit/SymbolFile` at which 
point you'll get a segfault that tells you nothing about what your problem is.

I started trying to fix the issues with bandaids, but it became clear that the 
proper solution was to just bring in the work I did in the rest of the 
projects.  The side benefit of this is that the lit configuration files become 
much cleaner and more understandable as a result.


https://reviews.llvm.org/D54009

Files:
  lldb/lit/Breakpoint/case-insensitive.test
  lldb/lit/Breakpoint/lit.local.cfg
  lldb/lit/CMakeLists.txt
  lldb/lit/Expr/TestIRMemoryMapWindows.test
  lldb/lit/Expr/lit.local.cfg
  lldb/lit/Quit/lit.local.cfg
  lldb/lit/Settings/lit.local.cfg
  lldb/lit/SymbolFile/NativePDB/lit.local.cfg
  lldb/lit/SymbolFile/PDB/ast-restore.test
  lldb/lit/SymbolFile/PDB/calling-conventions.test
  lldb/lit/SymbolFile/PDB/class-layout.test
  lldb/lit/SymbolFile/PDB/compilands.test
  lldb/lit/SymbolFile/PDB/enums-layout.test
  lldb/lit/SymbolFile/PDB/func-symbols.test
  lldb/lit/SymbolFile/PDB/function-level-linking.test
  lldb/lit/SymbolFile/PDB/function-nested-block.test
  lldb/lit/SymbolFile/PDB/lit.local.cfg
  lldb/lit/SymbolFile/PDB/pointers.test
  lldb/lit/SymbolFile/PDB/type-quals.test
  lldb/lit/SymbolFile/PDB/typedefs.test
  lldb/lit/SymbolFile/PDB/udt-layout.test
  lldb/lit/SymbolFile/PDB/variables-locations.test
  lldb/lit/SymbolFile/PDB/variables.test
  lldb/lit/Unit/lit.cfg
  lldb/lit/Unit/lit.cfg.py
  lldb/lit/Unit/lit.site.cfg.in
  lldb/lit/Unit/lit.site.cfg.py.in
  lldb/lit/lit.cfg
  lldb/lit/lit.cfg.py
  lldb/lit/lit.site.cfg.in
  lldb/lit/lit.site.cfg.py.in
  lldb/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
  lldb/lit/tools/lldb-mi/breakpoint/break-insert.test
  lldb/lit/tools/lldb-mi/data/data-info-line.test
  lldb/lit/tools/lldb-mi/exec/exec-continue.test
  lldb/lit/tools/lldb-mi/exec/exec-finish.test
  lldb/lit/tools/lldb-mi/exec/exec-interrupt.test
  lldb/lit/tools/lldb-mi/exec/exec-next-instruction.test
  lldb/lit/tools/lldb-mi/exec/exec-next.test
  lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test
  lldb/lit/tools/lldb-mi/exec/exec-step.test
  lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -55,6 +55,8 @@
 features.add('system-windows')
 elif platform.system() == "Linux":
 features.add('system-linux')
+elif platform.system() in ['FreeBSD']:
+config.available_features.add('system-freebsd')
 
 # Native compilation: host arch == default triple arch
 # Both of these values should probably be in every site config (e.g. as
Index: lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test
===
--- lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test
+++ lldb/lit/tools/lldb-mi/symbol/symbol-list-lines.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c %p/inputs/symbol-list-lines.c %p/inputs/list-lines-helper.c -g
Index: lldb/lit/tools/lldb-mi/exec/exec-step.test
===
--- lldb/lit/tools/lldb-mi/exec/exec-step.test
+++ lldb/lit/tools/lldb-mi/exec/exec-step.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c -g
Index: lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test
===
--- lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test
+++ lldb/lit/tools/lldb-mi/exec/exec-step-instruction.test
@@ -1,4 +1,4 @@
-# XFAIL: windows
+# XFAIL: system-windows
 # -> llvm.org/pr24452
 #
 # RUN: %cc -o %t %p/inputs/main.c -g
Index: lldb/lit/tools/lldb-mi/exec/exec-next.test
===
--- lldb/lit/tools/lldb-mi/exec/exec-next.test
+++