[mlir][sparse] Refining the IR/CMakeLists.txt (followup to D136477)

[vmurali]

Not sure if target_sources can be inside the for loop (i.e. does target_sources append to MLIRSparseTensorEnums, or does it rewrite it?). Did you check if it works if you have multiple headers?

[wrengr]

In D136506#3876277, @vmurali wrote:

Not sure if target_sources can be inside the for loop (i.e. does target_sources append to MLIRSparseTensorEnums, or does it rewrite it?). Did you check if it works if you have multiple headers?

Per https://cmake.org/cmake/help/latest/command/target_sources.html: "Repeated calls for the same <target> append items in the order called."

It works as written for building things within llvm-project, though I don't have an external project set up for verifying that it works there too.

[joshchngs]

In D136506#3876581, @wrengr wrote:

It works as written for building things within llvm-project, though I don't have an external project set up for verifying that it works there too.

I can confirm it doesn't work for my external project. I think because the INSTALL_INTERFACE generator needs to account for the include tree being separate from the lib source.
Added you as a review on the fix, @wrengr - https://reviews.llvm.org/D136649

mlir/lib/Dialect/SparseTensor/IR/CMakeLists.txt

 # This library is shared by both MLIRSparseTensorDialect and
 # MLIRSparseTensorRuntime, so it must not depend on any of the MLIR/LLVM
 # internals or else mlir_c_runner_utils will inherit that dependency.
 #
 # Because this is a header-only (`INTERFACE`) library, we cannot use
 # the `add_mlir_library` function.  So we do our best to replicate the
 # relevant portions below.  If doing so becomes too complicated, then
 # we should adjust the `add_mlir_library` function to also work for
 # `INTERFACE` libraries.
-set(MLIRSparseTensorEnums_srcs
-    ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/SparseTensor/IR/Enums.h)
+set(MLIRSparseTensorEnums_hdrs
+    mlir/Dialect/SparseTensor/IR/Enums.h)
 # This conditional is copypasta from `add_mlir_library`.
 if(MSVC_IDE OR XCODE)
-  set_source_files_properties(${MLIRSparseTensorEnums_srcs}
+  foreach(hdr ${MLIRSparseTensorEnums_hdrs})
+    set_source_files_properties(${MLIR_MAIN_INCLUDE_DIR}/${hdr}
       PROPERTIES HEADER_FILE_ONLY ON)
+  endforeach()
 endif()
 # Older versions of cmake (< 3.19) require INTERFACE libraries to separate
 # the `add_library` and `target_sources` calls.
 add_library(MLIRSparseTensorEnums INTERFACE)
+# If we call target_sources naively, then the library won't be
+# installed properly (i.e., so that it can be used by projects outside
+# the llvm-project repo).  To correct this, we must use BUILD_INTERFACE /
+# INSTALL_INTERFACE generator expressions to avoid paths being interpreted
+# as absolute; for more details, see <https://stackoverflow.com/a/62465051>.
+# Unfortunately BUILD_INTERFACE and INSTALL_INTERFACE require their file
+# paths to be relative to different things, hence why we use a foreach loop
+# to iterate over the sources (in case we ever need to have more than one).
+foreach(hdr ${MLIRSparseTensorEnums_hdrs})
   target_sources(MLIRSparseTensorEnums INTERFACE
-  $<BUILD_INTERFACE:${MLIRSparseTensorEnums_srcs}>
-  $<INSTALL_INTERFACE:mlir/Dialect/SparseTensor/IR/Enums.h>)
+    $<BUILD_INTERFACE:${MLIR_MAIN_INCLUDE_DIR}/${hdr}>
+    $<INSTALL_INTERFACE:${hdr}>)
+endforeach()
 # The `add_mlir_library_install` is required for other libraries to
 # depend on this one, but the conditional itself and the phony target
 # are copypasta from `add_mlir_library`.  Afaict (wrengr), the version
 # in `add_mlir_library` is to prevent `add_mlir_library_install` from
 # raising additional errors whenever the underlying call to `add_llvm_library`
 # fails.  However, since we are using `add_library` directly, I'm not
 # sure if the conditional is helpful/required here or not.
 if(TARGET MLIRSparseTensorEnums)