thakis created this revision.
thakis added reviewers: mbonadei, sdefresne.
Herald added subscribers: jdoerfert, kadircet, arphaman, jkorous, MaskRay, 
ioeric, ilya-biryukov, mgorny.
Herald added a project: LLVM.
thakis edited the summary of this revision.

This is a bit of a larger change since this is the first (and as far as I can 
tell only) place where the LLVM build produces macOS framework bundles.

GN has some built-in support for this, so use that. `gn help create_bundle` has 
a terse description (but it's a bit outdated: `deps` must be `public_deps` and 
the conditionals in the example in the help aren't quite right on non-iOS).

We need a new 'copy_bundle_data' tool, and since we copy the clangd.xpc bundle 
as bundle_data into ClangdXPC.framework it needs to be able to handle 
directories in addition to files.

GN also insists we have a compile_xcassets tool even though it's not used. I 
just made that run `false`.

Despite GN's support for bundles, we still need to manually create the expected 
symlink structure in the .framework bundle. Since this code never runs on 
Windows, it's safe to create the symlinks before the symlink targets exist, so 
we can just make the bundle depend on the steps that create the symlinks. For 
this to work, change the symlink script to create the symlink's containing 
directory if it doesn't yet exist.

I locally verified that CMake and GN build create the same bundle structure. (I 
noticed that both builds set LC_ID_DYLIB to the pre-copy libClangdXPCLib.dylib 
name, but that seems to not cause any issues and it happens in the CMake build 
too.)

(Also add an error message to clangd-xpc-test-client for when loading the dylib 
fails – this was useful while locally debugging this.)


https://reviews.llvm.org/D60130

Files:
  clang-tools-extra/clangd/xpc/framework/CMakeLists.txt
  clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
  llvm/utils/gn/build/symlink_or_copy.py
  llvm/utils/gn/build/toolchain/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
===================================================================
--- llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
@@ -1,3 +1,4 @@
+import("//clang-tools-extra/clangd/xpc/enable.gni")
 import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
 import("//llvm/triples.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
@@ -35,9 +36,14 @@
     "LLVM_LIT_TOOLS_DIR=",  # Intentionally empty, matches cmake build.
     "LLVM_TOOLS_DIR=" + rebase_path("$root_out_dir/bin"),
     "PYTHON_EXECUTABLE=$python_path",
-    "CLANGD_BUILD_XPC_SUPPORT=0",  # FIXME
   ]
 
+  if (clangd_build_xpc) {
+    extra_values += [ "CLANGD_BUILD_XPC_SUPPORT=1" ]
+  } else {
+    extra_values += [ "CLANGD_BUILD_XPC_SUPPORT=0" ]
+  }
+
   if (clang_enable_static_analyzer) {
     extra_values += [ "CLANG_ENABLE_STATIC_ANALYZER=1" ]
   } else {
@@ -83,6 +89,10 @@
     "//llvm/utils/llvm-lit",
     "//llvm/utils/not",
   ]
+  if (clangd_build_xpc) {
+    deps +=
+        [ "//clang-tools-extra/clangd/xpc/test-client:clangd-xpc-test-client" ]
+  }
 
   # FIXME: dep on dexp once it exist
   testonly = true
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
===================================================================
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
@@ -0,0 +1,20 @@
+executable("clangd-xpc-test-client") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+    "//clang-tools-extra/clangd",
+    "//clang-tools-extra/clangd/xpc:conversions",
+    "//clang-tools-extra/clangd/xpc/framework:ClangdXPC.framework",
+    "//clang/lib/Basic",
+    "//clang/lib/Format",
+    "//clang/lib/Frontend",
+    "//clang/lib/Sema",
+    "//clang/lib/Tooling",
+    "//clang/lib/Tooling/Core",
+    "//llvm/lib/Support",
+  ]
+
+  include_dirs = [ "../.." ]
+  sources = [
+    "ClangdXPCTestClient.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
===================================================================
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
@@ -0,0 +1,154 @@
+import("//llvm/utils/gn/build/symlink_or_copy.gni")
+import("//llvm/utils/gn/build/write_cmake_config.gni")
+
+# create_clangd_xpc_framework() in cmake creates both the ClangdXPC.framework
+# bundle and the clangd.xpc bundle within it in a single action.
+# Since GN has some native support for macOS bundles, it's more natural
+# to have one create_bundle() each for both ClangdXPC.framework and clangd.xpc.
+# See `llvm/utils/gn/gn.py help create_bundle` and ../cmake/modules.
+
+######################################################################
+# clangd.xpc bundle
+
+write_cmake_config("XPCServiceInfo.plist") {
+  input = "../cmake/XPCServiceInfo.plist.in"
+  output = "$target_gen_dir/XPCServiceInfo.plist"
+  service_name = "clangd"
+  values = [
+    "CLANGD_XPC_SERVICE_NAME=$service_name",
+    "CLANGD_XPC_SERVICE_BUNDLE_NAME=org.llvm.$service_name",
+  ]
+}
+
+bundle_data("clangxpc_bundle_xpc_service_info_plist") {
+  public_deps = [
+    ":XPCServiceInfo.plist",
+  ]
+  sources = [
+    "$target_gen_dir/XPCServiceInfo.plist",
+  ]
+  outputs = [
+    "{{bundle_contents_dir}}/Info.plist",
+  ]
+}
+
+bundle_data("clangxpc_bundle_xpc_service_executable") {
+  public_deps = [
+    "//clang-tools-extra/clangd/tool:clangd",
+  ]
+  sources = [
+    "$root_out_dir/bin/clangd",
+  ]
+  outputs = [
+    "{{bundle_executable_dir}}/clangd",
+  ]
+}
+
+create_bundle("clangd.xpc") {
+  # .app directory structure.
+  # Since this target only exists to be copied into ClangdXPC.framework,
+  # put it in $target_gen_dir, not $root_out_dir.
+  bundle_root_dir = "$target_gen_dir/$target_name"
+  bundle_contents_dir = "$bundle_root_dir/Contents"
+  bundle_executable_dir = "$bundle_contents_dir/MacOS"
+
+  deps = [
+    ":clangxpc_bundle_xpc_service_executable",
+    ":clangxpc_bundle_xpc_service_info_plist",
+  ]
+}
+
+######################################################################
+# ClangdXPC.framework
+
+write_cmake_config("Info.plist") {
+  input = "../cmake/Info.plist.in"
+  output = "$target_gen_dir/Info.plist"
+  values = [ "CLANGD_XPC_FRAMEWORK_NAME=ClangdXPC" ]
+}
+
+bundle_data("clangdxpc_bundle_info_plist") {
+  public_deps = [
+    ":Info.plist",
+  ]
+  sources = [
+    "$target_gen_dir/Info.plist",
+  ]
+  outputs = [
+    "{{bundle_resources_dir}}/Info.plist",
+  ]
+}
+
+shared_library("ClangdXPCLib") {
+  deps = [
+    "//clang-tools-extra/clangd/tool:clangd",
+  ]
+  sources = [
+    "ClangdXPC.cpp",
+  ]
+}
+
+bundle_data("clangdxpc_bundle_executable") {
+  public_deps = [
+    ":ClangdXPCLib",
+  ]
+  sources = [
+    "$root_out_dir/lib/libClangdXPCLib.dylib",
+  ]
+  outputs = [
+    "{{bundle_executable_dir}}/ClangdXPC",
+  ]
+}
+
+bundle_data("clangdxpc_bundle_xpc") {
+  public_deps = [
+    ":clangd.xpc",
+  ]
+  sources = [
+    "$target_gen_dir/clangd.xpc",
+  ]
+  outputs = [
+    "{{bundle_contents_dir}}/XPCServices/clangd.xpc",
+  ]
+}
+
+# .framework bundle symlinks:
+# - ./ClangdXPC -> Versions/Current/ClangdXPC
+# - ./Resources -> Versions/Current/Resources
+# - ./XPCServices -> Versions/Current/XPCServices
+# - ./Versions/Current -> Versions/A
+# Since bundles are a mac thing, we know that symlink_or_copy() will symlink
+# and not copy, and hence creating the symlink before the target exists is safe.
+symlinks = [
+  "ClangdXPC",
+  "Resources",
+  "XPCServices",
+]
+foreach(target, symlinks) {
+  symlink_or_copy("clangdxpc_symlink_$target") {
+    source = "Versions/Current/$target"
+    output = "$root_out_dir/lib/ClangdXPC.framework/$target"
+  }
+}
+symlink_or_copy("clangdxpc_symlink_Versions_Current") {
+  source = "A"
+  output = "$root_out_dir/lib/ClangdXPC.framework/Versions/Current"
+}
+
+create_bundle("ClangdXPC.framework") {
+  # .framework directory structure.
+  bundle_root_dir = "$root_out_dir/lib/$target_name"
+  bundle_contents_dir = "$bundle_root_dir/Versions/A"
+  bundle_executable_dir = "$bundle_contents_dir"
+  bundle_resources_dir = "$bundle_contents_dir/Resources"
+
+  deps = [
+    ":clangdxpc_bundle_executable",
+    ":clangdxpc_bundle_info_plist",
+    ":clangdxpc_bundle_xpc",
+    ":clangdxpc_symlink_Versions_Current",
+  ]
+  foreach(target, symlinks) {
+    deps += [ ":clangdxpc_symlink_$target" ]
+  }
+}
Index: llvm/utils/gn/build/toolchain/BUILD.gn
===================================================================
--- llvm/utils/gn/build/toolchain/BUILD.gn
+++ llvm/utils/gn/build/toolchain/BUILD.gn
@@ -113,11 +113,27 @@
       default_output_dir = "{{root_out_dir}}/bin"
     }
 
+    copy_command = "ln -f {{source}} {{output}} 2>/dev/null || (rm -rf {{output}} && cp -af {{source}} {{output}})"
     tool("copy") {
-      command = "ln -f {{source}} {{output}} 2>/dev/null || (rm -rf {{output}} && cp -af {{source}} {{output}})"
+      command = copy_command
       description = "COPY {{source}} {{output}}"
     }
 
+    if (current_os == "mac") {
+      tool("copy_bundle_data") {
+        # http://serverfault.com/q/209888/43689
+        _copydir = "mkdir -p {{output}} && cd {{source}} && " +
+                   "pax -rwl . \"\$OLDPWD\"/{{output}}"
+        command = "rm -rf {{output}} && if [[ -d {{source}} ]]; then " +
+                  _copydir + "; else " + copy_command + "; fi"
+        description = "COPY_BUNDLE_DATA {{source}} {{output}}"
+      }
+      tool("compile_xcassets") {
+        command = "false"
+        description = "The LLVM build doesn't use any xcasset files"
+      }
+    }
+
     tool("stamp") {
       command = "touch {{output}}"
       description = "STAMP {{output}}"
Index: llvm/utils/gn/build/symlink_or_copy.py
===================================================================
--- llvm/utils/gn/build/symlink_or_copy.py
+++ llvm/utils/gn/build/symlink_or_copy.py
@@ -22,6 +22,11 @@
     # FIXME: This should not check the host platform but the target platform
     # (which needs to be passed in as an arg), for cross builds.
     if sys.platform != 'win32':
+        try:
+            os.makedirs(os.path.dirname(args.output))
+        except OSError as e:
+            if e.errno != errno.EEXIST:
+                raise
         try:
             os.symlink(args.source, args.output)
         except OSError as e:
Index: clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
===================================================================
--- clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
+++ clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
@@ -49,8 +49,10 @@
   // Open the ClangdXPC dylib in the framework.
   std::string LibPath = getLibraryPath();
   void *dlHandle = dlopen(LibPath.c_str(), RTLD_LOCAL | RTLD_FIRST);
-  if (!dlHandle)
+  if (!dlHandle) {
+    llvm::errs() << "Failed to load framework from \'" << LibPath << "\'\n";
     return 1;
+  }
 
   // Lookup the XPC service bundle name, and launch it.
   clangd_xpc_get_bundle_identifier_t clangd_xpc_get_bundle_identifier =
Index: clang-tools-extra/clangd/xpc/framework/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/xpc/framework/CMakeLists.txt
+++ clang-tools-extra/clangd/xpc/framework/CMakeLists.txt
@@ -1,6 +1,7 @@
 
 set(SOURCES
-    ClangdXPC.cpp)
+  ClangdXPC.cpp
+)
 add_clang_library(ClangdXPCLib SHARED
   ${SOURCES}
   DEPENDS
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to