Shouldn't the title be the other way around?

In D36908#847787, @george.karpenkov wrote:

Ok, the first thing that I've noticed is that the compiler-rt version does not build with gcc:

@mgorny this is actually due to change proposed in https://reviews.llvm.org/D36887.
I am not sure what is the correct strategy here: previously, the flag was behind the guard, which was never set for gcc.
I don't think it is correct to rely on LLVM_* variable from a compiler-rt repository.
I can also test whether the flag is available first before compiling.

Ok, the first thing that I've noticed is that the compiler-rt version does not build with gcc:

A side notice but wouldn't it be better to make libFuzzer a completely separate component (like openmp)? I've seen people take interest in it outside LLVM toolchain.

Ping.

Also, I think that if you're splitting them up, it'd also logical to move them into a subdirectory, x86-common maybe.

No problem at all. Thanks a lot.

Updated with @chapuni's patch. Please confirm it's good to go.

Updated to explicitly respect LLDB_DISABLE_CURSES and the LLVM HAVE_TERMINFO logic.

Forgive my poor eyesight, obviously this could go into existing LINK_LIBS ;-).

@chapuni, this fixes the issue for me as well. Will you commit it or should I update this revision with your patch?

Well, @chapuni disagrees on D36211. Which way should we go then?

Well, that was my original idea but @beanz wanted it the other way around ;-). I'm fine either way, as long as it works.

Well, I can confirm that it doesn't break running in place ;-).

https://bugs.llvm.org/show_bug.cgi?id=33987 now open for that purpose.

Yes, I've planned on doing that. https://bugs.llvm.org/show_bug.cgi?id=33987 now open for that purpose.

Updated to only play with NDEBUG definitions.

All right, I'll update it to tackle [-/][DU]NDEBUG and move on.

In D35898#824549, @whitequark wrote:

message(FATAL_ERROR "LLVM_OCAML_OUT_OF_TREE cannot be enabled while on Debug mode. Use -DCMAKE_BUILD_TYPE=Release instead.")

Why not just ignore CMAKE_C_COMPILER &c when building out of tree? What's the problem here exactly? This seems like an arbitrary restriction.

I suppose both should work but I only tested with plain LD_LIBRARY_PATH.

@jpdeplaix, well, I don't know how NDEBUG affects you but in Gentoo it's more complex. We wipe out the CMake defaults that play with NDEBUG and set it manually so that we don't need to depend on CMAKE_BUILD_TYPE to enable assertions&co.

v2: sets the compiler and with the flags so that we won't try to pass incorrect flags to another compiler. Since C compilers are reasonably compatible with each other, it shouldn't be a problem if OCaml was built with another.

This broke some buildbots due to C compiler mismatch between OCaml and LLVM. Will have to think it over.

Well, I wanted to suggest creating completely separate test inputs for max-failures to avoid implicitly relying on the number of failures in shtest-shell but this works as well ;-).

Well, I can't reproduce that one now but I've got another one. An easy way to reproduce:

Another ping. @beanz, any chance you could take a look at this?

This patch has introduced a test suite failure:

Technically LGTM but I don't have a Windows setup to test.

The code looks good, and seems to work. I'm sometimes getting a failure from shtest-timeout.py but I suspect that's an irrelevant race condition.

LGTM. I've confirmed that it fixes lit test suite, and no additional tests from the LLVM test suite fails (except the one that failed before applying the patch). I haven't tested other LLVM projects though.

LGTM. I've confirmed that it fixes lit test suite, and no additional tests from the LLVM test suite fails (except the one that failed before applying the patch). I haven't tested other LLVM projects though.

Oh, I see there's D35878 supposedly addressing this. Sorry for missing it.

Thanks for the fix. Now it fails on another problem:

Green on Linux as well. I suppose Windows will be broken but we can mark it appropriately when the buildbots are up and confirm.

Ping.

I'm sorry, I was wrong. It breaks four tests:

Please do it. I've got my plate full. I'm going just to finish complaining on the patches that broke stuff and hopefully get some good out of that.

This commit breaks the unittest adaptor:

This broke two lit tests:

In D35091#820739, @george.karpenkov wrote:

@mgorny I still can't run them:

1. No class called FileBasedTest: where is it?

TBH I haven't been able to run tests: how did you do that? I've even opened a bug about this: https://bugs.llvm.org/show_bug.cgi?id=33704

Yes, I had a similar idea but I think we need to get the tests to pass first. Do you want me to continue bisecting bugs or are you planning to do some yourself? I've found one caused by D35091 but I don't see an obvious solution there, so any help would be appreciated.

I'm sorry to crash the party but the added test fails on Linux:

This causes a test failure with non-standard CLANG_RESOURCE_DIR:

(note that I don't mind removing the llvm-config bit for it; but we need the cache variable to stay)

This is going to break Gentoo. We're relying on the ability to specify LLVM_MAIN_SRC_DIR to provide unpacked LLVM sources with gtest. Without that, I don't see how we can use tests.

Well, I've just checked gcc-config sources and it seems that it sources the file through bash, and gets the resulting ${CURRENT}. While I don't think we ought to do complete bash support here, I guess allowing the same degree of whitespace bash does makes sense.

Is there any specific reason you need this? Since the file is autogenerated, I don't think it matters very much to allow extra whiespace. But then, I do not think it hurts, so feel free to do it. The code LGTM; note that I haven't tested it, however.

D31864 covered all that was needed for Gentoo, and I lack the knowledge to push the full split properly forward.