phosek added a comment.

In D124673#3512070 <https://reviews.llvm.org/D124673#3512070>, @MaskRay wrote:

> In D124673#3512037 <https://reviews.llvm.org/D124673#3512037>, @paulkirth 
> wrote:
>
>> Hi, Sorry for the late notification, but I think this change may not apply 
>> correctly to all configs.
>>
>> We're seeing a breakage in Fuchsia's Clang CI builders: 
>> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8814329895908917697/overview
>>
>>   FAILED: lib/LineEditor/CMakeFiles/LLVMLineEditor.dir/LineEditor.cpp.o 
>>   /b/s/w/ir/cache/goma/client/gomacc /b/s/w/ir/x/w/cipd/bin/clang++ 
>> --sysroot=/b/s/w/ir/x/w/cipd/linux -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
>> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
>> -I/b/s/w/ir/x/w/staging/llvm_build/lib/LineEditor 
>> -I/b/s/w/ir/x/w/llvm-llvm-project/llvm/lib/LineEditor 
>> -I/b/s/w/ir/cache/vpython/79db7c/lib/python3.8/site-packages/tensorflow/include
>>  -I/b/s/w/ir/x/w/staging/llvm_build/include 
>> -I/b/s/w/ir/x/w/llvm-llvm-project/llvm/include -isystem 
>> /b/s/w/ir/x/w/staging/zlib_install/include -stdlib=libc++ -fPIC 
>> -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time 
>> -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
>> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
>> -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
>> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
>> -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment 
>> -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color 
>> -ffunction-sections -fdata-sections 
>> -ffile-prefix-map=/b/s/w/ir/x/w/staging/llvm_build=../staging/llvm_build 
>> -ffile-prefix-map=/b/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes 
>> -O3 -DNDEBUG  -fno-exceptions -fno-unwind-tables 
>> -fno-asynchronous-unwind-tables -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
>> lib/LineEditor/CMakeFiles/LLVMLineEditor.dir/LineEditor.cpp.o -MF 
>> lib/LineEditor/CMakeFiles/LLVMLineEditor.dir/LineEditor.cpp.o.d -o 
>> lib/LineEditor/CMakeFiles/LLVMLineEditor.dir/LineEditor.cpp.o -c 
>> /b/s/w/ir/x/w/llvm-llvm-project/llvm/lib/LineEditor/LineEditor.cpp
>>   /b/s/w/ir/x/w/llvm-llvm-project/llvm/lib/LineEditor/LineEditor.cpp:18:10: 
>> fatal error: 'histedit.h' file not found
>>   #include <histedit.h>
>>            ^~~~~~~~~~~~
>>
>> since the error is a missing header for `histedit.h`, and this change alters 
>> how libedit is found in CMake, I assume this is the root cause.
>>
>> I'm guessing some part of the old CMake checks like the old 
>> `HAVE_HISTEDIT_H` may not have propagated correctly in all cases?
>>
>> If this will be hard to fix, would you mind reverting until a fix is ready?
>
> It's not clear to me that this patch caused the issue for you, so I don't 
> think it is right to ask for a revert now. You need to provide more evidence.
> lldb-x86_64-debian and lldb-cmake-standalone on 
> https://lldb.llvm.org/resources/bots.html work well with the change.

I looked a bit more into the error. In the CMake log I see the following:

  -- Found LibEdit: /usr/include (found version ".") 

This is incorrect because in our build we set 
`CMAKE_SYSROOT=/b/s/w/ir/x/w/cipd/linux` so CMake shouldn't be looking in paths 
like `/usr/include` because at build time, Clang will be invoked with 
`--sysroot=/b/s/w/ir/x/w/cipd/linux` and it won't consider include paths like 
`/usr/include`.

I don't this issue was introduced in this change, it's a pre-existing issue 
with the `FindLibEdit.cmake` module, but it was uncovered by this change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124673/new/

https://reviews.llvm.org/D124673

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

Reply via email to