[PATCH] D89142: llvmbuildectomy

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 2 09:32:27 PST 2020


mehdi_amini added a comment.

In D89142#2368614 <https://reviews.llvm.org/D89142#2368614>, @Meinersbur wrote:

> [style] The indentation convention for CMake files is two spaces.
>
> In D89142#2364349 <https://reviews.llvm.org/D89142#2364349>, @serge-sans-paille wrote:
>
>> That bein g said, I agree it's only the first step toward removing the LLVMBuild.* files, and the redunduncy between these files and the regular CMakeFiles.txt. I just don't want to do all the changes at once, and this one is already a big change!
>
> It's still a detour. More work to get the intermediate state to a good state, its risk to break something, the confusion it creates to contributors. In case of Bazel, the intermediate state might also cause additional work (Bazel people may want to comment on that).

It won't make a different for our Bazel generator script I believe.

About the incrementality, I assume this is the main point:

> Do we really need a LLVMBuild.cmake for every component? I hoped it would be possible to derive that information from CMake's dependency graph (which would avoid them being out-of-sync). If it's not feasible to get that information from CMake, why not use add_llvm_library in the CMakeLists.txt to pass that information?

Is there an agreement that we want to remove these LLVMBuild.cmake files and move everything to the main CMakeLists.txt files?

It isn't clear why we can't just do it immediately, but I haven't thought it through really. If Serge perceives the current patch as a useful incremental step before addressing to the next steps, I don't mind doing it this way either.


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

https://reviews.llvm.org/D89142



More information about the llvm-commits mailing list