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

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Thanks for the patch! Look great, except for the all_dependent_libs bit.



================
Comment at: llvm/utils/gn/build/libs/pthread/BUILD.gn:10
+  if (llvm_enable_threads && current_os != "win") {
+    all_dependent_configs = [ ":pthread_config" ]
   }
----------------
`all_dependent_configs` is an anti pattern and we shouldn't use it. Why is this part of the change needed?


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:91
       } else {
-        command = "$ld {{ldflags}} -o $outfile {{libs}} -Wl,--start-group {{inputs}} -Wl,--end-group"
+        command = "$ld {{ldflags}} -o $outfile -Wl,--start-group {{inputs}} {{libs}} -Wl,--end-group"
       }
----------------
pcc wrote:
> Nit: you could keep `{{libs}}` and `{{inputs}}` in the same order as on Mac.
D81035 just puts {{libs}} after the inputs group. Is that sufficient?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80591