[PATCH] Do not create a clang-install target for MSVC solutions

[aaron.ballman]

Summary

When generating an MSVC solution from CMake, it creates a clang-install project. We do not have any *-install targets as part of MSVC, so this seems out of place (not to mention, a bit strange). This patch disables creation of the clang-install project when making an MSVC solution.

[bcraig]

Does this affect the presence or functionality of the "INSTALL" project?

[aaron.ballman]

In D14502#285305, @bcraig wrote:

Does this affect the presence or functionality of the "INSTALL" project?

There is no INSTALL project in MSVC from what I can see (before or after this patch), so I don't think that this affects that functionality.

[aaron.ballman]

In D14502#285371, @aaron.ballman wrote:

In D14502#285305, @bcraig wrote:

Does this affect the presence or functionality of the "INSTALL" project?

There is no INSTALL project in MSVC from what I can see (before or after this patch), so I don't think that this affects that functionality.

I spoke too soon, I found it by squinting harder. It's in the CMakePredefinedTargets folder. What do INSTALL and PACKAGE actually *do*? For me, PACKAGE fails to build (CPack error : Cannot find NSIS compiler makensis: likely it is not installed, or not in your PATH), and INSTALL fails with the expected outcome (CMake Error at cmake_install.cmake: 31 (file): file INSTALL cannot make directory "C:/Program Files (x86)/LLVM/include/llvm": No such file or directory

I do not think we should have an INSTALL target for MSVC at all (unless it is building an installer, for instance). Installation requires user input, and likely also requires elevation. Having a project that does the wrong thing (for instance, all of my applications live on D:, not C:) doesn't seem fruitful as a project that is generated by default (but it could be useful to some people to be enabled via a CMake flag).

[bcraig]

In a text file, I have a big long cmake invocation. Part of that invocation is the following:

-DCMAKE_INSTALL_PREFIX=c:/install/Tools

The VC "INSTALL" project copies files there, in a very unix-y hierarchy (i.e. c:\install\Tools\bin, c:\install\Tools\include, c:\install\Tools\etc, and so on).

[beanz]

tools/driver/CMakeLists.txt

58

Can you change this to if(NOT CMAKE_CONFIGURATION_TYPES)?

With that it won't create the install-clang target for any IDE, which is probably the way I should have done this in the first place.

Other than that LGTM.

Thanks,
-Chris

[aaron.ballman]

tools/driver/CMakeLists.txt

58

Thanks, I'll make that modification. Out of curiosity, do you think INSTALL and PACKAGE should be opt-in for MSVC? Not present at all? Always present (as they are currently)?

[aaron.ballman]

Committed in r252601.

tools/driver/CMakeLists.txt

 install(TARGETS clang
   RUNTIME DESTINATION bin
   COMPONENT clang)
-add_custom_target(install-clang
-  DEPENDS clang
-  COMMAND "${CMAKE_COMMAND}"
-          -DCMAKE_INSTALL_COMPONENT=clang
-          -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
 
+if(NOT MSVC)
+  add_custom_target(install-clang
+    DEPENDS clang
+    COMMAND "${CMAKE_COMMAND}"
+            -DCMAKE_INSTALL_COMPONENT=clang
+            -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+endif()
+
 if(NOT CLANG_LINKS_TO_CREATE)
   set(CLANG_LINKS_TO_CREATE clang++ clang-cl)