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

[Mitch Phillips via Phabricator via llvm-commits]

hctim marked 4 inline comments as done.
hctim 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" ]
   }
----------------
thakis wrote:
> 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?
```
deps: Private linked dependencies.
  A list of target labels.

  Specifies private dependencies of a target. Private dependencies are
  propagated up the dependency tree and linked to dependant targets, but
  do not grant the ability to include headers from the dependency.
  ***Public configs are not forwarded.***
```

(emphasis mine)

> 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?

Yes. Probably a bad example on my part, but it would be nice to stop the dependency chain at shared objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80591