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

[Dan Albert via Phabricator via libcxx-commits]

danalbert accepted this revision.
danalbert marked an inline comment as done.
danalbert added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/CMakeLists.txt:784
+  if (ANDROID AND ANDROID_PLATFORM_LEVEL LESS 21)
+    target_link_libraries(${target} PUBLIC android_support)
+  endif()
----------------
smeenai wrote:
> 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`.
Okay, works for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73516