[Mlir-commits] [mlir] [mlir] Fix MLIR dylib CMake config when LLVM_BUILD_LLVM_DYLIB is OFF (PR #155488)

[Bryan Tan]

================
@@ -172,8 +172,11 @@ set(MLIR_INSTALL_AGGREGATE_OBJECTS 1 CACHE BOOL
 
 set(MLIR_BUILD_MLIR_C_DYLIB 0 CACHE BOOL "Builds libMLIR-C shared library.")
 
-set(MLIR_LINK_MLIR_DYLIB ${LLVM_LINK_LLVM_DYLIB} CACHE BOOL
-    "Link tools against libMLIR.so")
+set(MLIR_BUILD_MLIR_DYLIB 0 CACHE BOOL "Builds the libMLIR shared library")
+set(MLIR_LINK_MLIR_DYLIB ${LLVM_LINK_LLVM_DYLIB} CACHE BOOL "Link tools against libMLIR.so")
+if (MLIR_LINK_MLIR_DYLIB)
----------------
Technius wrote:

I politely disagree with the suggestion. I think the [CMake documentation](https://cmake.org/cmake/help/book/mastering-cmake/chapter/CMake%20Cache.html) is quite clear on the behavior of cache variables. Forcing the value of `MLIR_BUILD_MLIR_DYLIB` to `ON` in the cache would be quite confusing to a user that initially sets `MLIR_LINK_MLIR_DYLIB` to `ON` and later turns it off.

Instead, do you think it would be better to mirror the style of how `LLVM_BUILD_LLVM_DYLIB` is defined? 
https://github.com/llvm/llvm-project/blob/0b42e117c829c6e127ef4b1bd82807ba01853128/llvm/CMakeLists.txt#L938-L952

The name of `LLVM_BUILD_LLVM_DYLIB_default` makes it clear that it is a "default value", which could avoid the confusion that you mention.

https://github.com/llvm/llvm-project/pull/155488