[SR-5778] Swift CMake assumes Clang repository is named "clang"

[modocache]

Previous ID	SR-5778
Radar	None
Original Reporter	@modocache
Type	Bug

Environment

• macOS 10.12.6

• cmake version 3.7.20170204-gd5f0c

• Apple LLVM version 9.0.0 (clang-900.0.35)

• ~/local/Source/apple/standalone/swift revision f52b19238

• ~/local/Source/apple/standalone/swift-llvm revision 95bd0ac985

• ~/local/Source/apple/standalone/swift-clang revision d674193d759

• ~/local/Source/apple/standalone/libcxx revision e8c8bc9433

Additional Detail from JIRA

Votes	0
Component/s	Project Infrastructure
Labels	Bug, CMake
Assignee	None
Priority	Medium

md5: a7ca214f8df5b314b1556a93e2f57b72

Issue Description:

I encountered a limitation in Swift's CMake while attempting to build using the following directory structure:

~/local/Source/apple/standalone/
    libcxx            # Cloned from http://llvm.org/git/libcxx.git
    swift-cmark       # Cloned from https://github.com/apple/swift-cmark
    swift-llvm        # Cloned from https://github.com/apple/swift-llvm
    swift-clang       # Cloned from https://github.com/apple/swift-clang
    swift             # Cloned from https://github.com/apple/swift
    swift-cmark-build # cmark build directory
    swift-llvm-build  # LLVM/Clang build directory
    swift-build       # Swift build directory

Using the above, I can build cmark and LLVM like so:

cmake \
  -H$HOME/local/Source/apple/standalone/swift-cmark \
  -B$HOME/local/Source/apple/standalone/swift-cmark-build \
  -G Ninja
ninja -C ~/local/Source/apple/standalone/swift-cmark-build

cmake \
  -H$HOME/local/Source/apple/standalone/swift-llvm \
  -B$HOME/local/Source/apple/standalone/swift-llvm-build \
  -G Ninja \
  -DLLVM_ENABLE_PROJECTS="swift-clang;libcxx" \
  -DLLVM_EXTERNAL_PROJECTS=swift-clang \
  -DLLVM_EXTERNAL_SWIFT_CLANG_SOURCE_DIR=$HOME/local/Source/apple/standalone/swift-clang
ninja -C ~/local/Source/apple/standalone/swift-llvm-build

I would assume that I would then be able to build Swift like so:

cmake \
  -H$HOME/local/Source/apple/standalone/swift
  -B$HOME/local/Source/apple/standalone/swift-build \
  -G Ninja \
  -DCMAKE_BUILD_TYPE="Release" \
  -DSWIFT_PATH_TO_CMARK_SOURCE=$HOME/local/Source/apple/standalone/swift-cmark \
  -DSWIFT_PATH_TO_CMARK_BUILD=$HOME/local/Source/apple/standalone/swift-cmark-build
  -DSWIFT_PATH_TO_LLVM_SOURCE=$HOME/local/Source/apple/standalone/swift-llvm \
  -DSWIFT_PATH_TO_LLVM_BUILD=$HOME/local/Source/apple/standalone/swift-llvm-build \
  -DSWIFT_PATH_TO_CLANG_SOURCE=$HOME/local/Source/apple/standalone/swift-clang \
  -DSWIFT_PATH_TO_CLANG_BUILD=$HOME/local/Source/apple/standalone/swift-llvm-build
ninja -C ~/local/Source/apple/standalone/swift-build

However this results in the following error:

CMake Error at cmake/modules/SwiftSharedCMakeConfig.cmake:151 (message):
  Please set SWIFT_PATH_TO_CLANG_BUILD to a directory containing a Clang
  build.
Call Stack (most recent call first):
  cmake/modules/SwiftSharedCMakeConfig.cmake:202 (swift_common_standalone_build_config_clang)
  CMakeLists.txt:457 (swift_common_standalone_build_config)

The problem, as far as I understand, is due to the fact that these lines of SwiftSharedCMakeConfig.cmake assume a build path of "${PATH_TO_CLANG_BUILD}/tools/clang", whereas LLVM/Clang's CMake does not require the Clang repository be named "clang". Specifically, in the example structure I have above, the Clang repository is named swift-clang, and its build products exist at "${PATH_TO_CLANG_BUILD}/tools/swift-clang.

The following modification to that file allows me to build and run Swift successfully:

diff --git a/cmake/modules/SwiftSharedCMakeConfig.cmake b/cmake/modules/SwiftSharedCMakeConfig.cmake
index a832224ece..99f4ccc0fc 100644
--- a/cmake/modules/SwiftSharedCMakeConfig.cmake
+++ b/cmake/modules/SwiftSharedCMakeConfig.cmake
@@ -147,10 +147,10 @@ macro(swift_common_standalone_build_config_clang product is_cross_compiling)
   endif()
   get_filename_component(CLANG_MAIN_SRC_DIR "${PATH_TO_CLANG_SOURCE}" ABSOLUTE)

-  if(NOT EXISTS "${PATH_TO_CLANG_BUILD}/tools/clang/include/clang/Basic/Version.inc")
+  if(NOT EXISTS "${PATH_TO_CLANG_BUILD}/tools/swift-clang/include/clang/Basic/Version.inc")
     message(FATAL_ERROR "Please set ${product}_PATH_TO_CLANG_BUILD to a directory containing a Clang build.")
   endif()
-  set(CLANG_BUILD_INCLUDE_DIR "${PATH_TO_CLANG_BUILD}/tools/clang/include")
+  set(CLANG_BUILD_INCLUDE_DIR "${PATH_TO_CLANG_BUILD}/tools/swift-clang/include")

   if (NOT ${is_cross_compiling})
     set(${product}_NATIVE_CLANG_TOOLS_PATH "${LLVM_TOOLS_BINARY_DIR}")

While I understand that the Swift project encourages contributors to use swift/uitls/build-script to build the project, I think the above could be improved. Perhaps we could change the meaning of SWIFT_PATH_TO_CLANG_BUILD so that users instead are expected to pass in the path to tools/<name_of_clang_dir>.

What do you all think? Alternatively, if the mistake is actually with how I'm invoking CMake, please let me know!

[modocache]

+cc erg (JIRA User), not sure if you're still working on CMake stuff, but if you are, you might have an opinion here?

[belkadan]

Doug's moved on from the project. I'll rope in our new CMake person (who hasn't set up an account here yet). I think your proposed change makes sense, especially if we try to detect the old situation and add the "tools/clang/" if necessary.

[modocache]

Friendly ping, Rostepher (JIRA User)!

I think LLVM deals with a similar problem by transforming the values in the LLVM_ENABLE_PROJECTS CMake list variable, replacing any - with _ and transforming the value to uppercase – so in this case, a project named "swift-clang" would be transformed into SWIFT_CLANG – and then it checks for either llvm/../${original_name_of_project} or for ${LLVM_EXTERNAL_${UPPERCASE_PROJECT_NAME}_SOURCE_DIR}. So, with LLVM, I would be able to specify the following without any problems:

-DLLVM_ENABLE_PROJECTS="swift-clang"
-DLLVM_EXTERNAL_SWIFT_CLANG_SOURCE_DIR="/path/to/swift-clang"

Perhaps we could do something similar here, instead of hardcoding tools/clang? Let me know if you have any thoughts on the direction to take here.