[PATCH] D80591: Patch up issues with GN builds (pthread / libz)

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: llvm/utils/gn/build/libs/pthread/BUILD.gn:10
+  if (llvm_enable_threads && current_os != "win") {
+    all_dependent_configs = [ ":pthread_config" ]
   }
----------------
hctim wrote:
> thakis wrote:
> > `all_dependent_configs` is an anti pattern and we shouldn't use it. Why is this part of the change needed?
> If a transitive dependency needs pthreads, we need to supply it at link time.
> 
> `all_dependent_configs` seems like the right knob here - as otherwise anybody that transitively depends on pthreads must be included ONLY via. `public_deps`.
> 
> e.g. there's a current chain from
> `hwasan_shared -> sanitizer_common/sources -> pthread`
> 
> if we were to opt not to use `all_dependent_configs`, then:
>  1. `sanitizer_common/sources` needs to use `public_deps = [ "path/to/pthread" ]` (reasonable at this point)
>  2. `hwasan_shared` needs to use `public_deps = [ "sanitizer_common/sources" ]` or `public_deps = [ "path/to/pthread" ]` (yuck!), and same with anybody who depends on `hwasan_shared`.
ldflags should propagate until the first linkable (ie executable or shared library) but no further. If sanitizer_common/sources is a static lib or a source set, the ldflag it gets from its config should make it to the link of hwasan_shared as far as I understand. Is that not what's happening?

And if you have something that depends on hwasan_shared, you wouldn't want _that_ to link to pthreads unless it actively asks for it, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80591