[libcxx-commits] [PATCH] D73516: [libcxx] Link against android_support when needed

[Shoaib Meenai via Phabricator via libcxx-commits]

smeenai marked an inline comment as done.
smeenai added inline comments.


================
Comment at: libcxx/CMakeLists.txt:784
+  if (ANDROID AND ANDROID_PLATFORM_LEVEL LESS 21)
+    target_link_libraries(${target} PUBLIC android_support)
+  endif()
----------------
danalbert wrote:
> This should probably be `PRIVATE` rather than `PUBLIC`. The consumer doesn't need it if their `minSdkVersion` is 21+. The real constraint might be describable with a generator expression, but I'm not sure. (if we don't generate config files for this it doesn't matter anyway)
You're right ... I was assuming that `ANDROID_PLATFORM_LEVEL` would be consistent across the build of libc++ and the build of clients, but that's not necessarily true. With that being said, we don't generate config files for this as far as I'm aware, so the only place this `PUBLIC` will affect is other parts of LLVM's build that depend on libc++, and I think it's fair to assume the entirety of LLVM is being built with the same `ANDROID_PLATFORM_LEVEL`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73516