[PATCH] D135578: [CMake] Add option LLVM_FORCE_CREATE_SYMLINKS

Tue Oct 11 23:23:48 PDT 2022

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

In D135578#3848714 <https://reviews.llvm.org/D135578#3848714>, @thieta wrote:

> In D135578#3848706 <https://reviews.llvm.org/D135578#3848706>, @mstorsjo wrote:
>
>> This looks reasonable to me (although I haven't dug through the code to see if there are more cases that would need to be set in sync). Is it kosher to add new end-user options like this in a nested cmake module file?
>>
>> I'll leave it open for @phosek to comment on in any case.
>
> There are other places where symlinks are used and some similar logic is being applied - like here: 
> https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/CMakeLists.txt#L533
>
> I am not sure it's worth to fix this at the same time since that has to do with symlinking the files when building rather then the finished distribution.

This would be a separate option since we don't use `AddLLVM.cmake` module in that context so I think it's better to do it as a separate change.

> Regarding option() in this cmakefile - I had the same idea, but I saw several option() calls in this file already, so I figured it was a pattern already.

Those are only defined in functions, there are no top-level options in this module. I'd move this option to `llvm/CMakeLists.txt`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135578