[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added subscribers: beanz, smeenai.
smeenai added a comment.

In D58418#1577349 <https://reviews.llvm.org/D58418#1577349>, @jkorous wrote:

> Thanks for the revert.
>
> There's actually one more problem - seems like ninja doesn't like the generated build.ninja file on Linux.
>
>   ninja: error: build.ninja:52390: bad $-escape (literal $ must be written as $$)
>
>
> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/13617/steps/build-stage1-compiler/logs/stdio


We ran into this as well, and I was curious about what was going on, so I dug into it.

When you add a private dependency to static library target, as you're doing in this change with `${DIRECTORY_WATCHER_LINK_LIBS}`, CMake adds the dependency to the target's INTERFACE_LINK_LIBRARIES property using the `$<LINK_ONLY:...>` generator expression. The code for clang-shlib iterates through all the clang libraries and uses generator expressions to gather their INTERFACE_LINK_LIBRARIES, but if those generator expressions themselves evaluate to generator expressions (`$<LINK_ONLY:...>` in this case), the second level of generator expressions won't get evaluated and will just end up in the ninja file directly, hence the complaint about the dollar. The clang-shlib code in question is https://github.com/llvm/llvm-project/blob/3837f4273fcc40cc519035479aefe78e5cbd3055/clang/tools/clang-shlib/CMakeLists.txt#L10.

Here's a simple repro (where `empty.c` is literally an empty file). Running `cmake -G Ninja` on this and then running `ninja` should demonstrate the issue.

  cmake_minimum_required(VERSION 3.4.3)
  project(dollartest C)
  
  add_library(a STATIC empty.c)
  add_library(b_obj OBJECT empty.c)
  add_library(b STATIC empty.c)
  target_link_libraries(b PRIVATE a)
  
  add_library(c SHARED empty.c)
  target_link_libraries(c PRIVATE
    b_object
    $<TARGET_PROPERTY:b,INTERFACE_LINK_LIBRARIES>
    $<TARGET_PROPERTY:b,LINK_LIBRARIES>
    )

@beanz, thoughts on how best to handle this?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58418/new/

https://reviews.llvm.org/D58418