[PATCH] D54345: Add initial scaffolding for the GN build.

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:51
+    } else {
+      command = "ar rcsDT {{arflags}} -o {{output}} {{inputs}}"
+    }
----------------
phosek wrote:
> thakis wrote:
> > phosek wrote:
> > > Can we make `ar` variable as well? I'd like to change this, e.g. for Clang toolchains to use `llvm-ar`.
> > Sure, but let's do that when we need it?
> Why not use `llvm-ar` if the `clang_base_path` is set? I don't think there's any reason not to?
My use case for clang_base_path is using goma with chromium's clang package, which doesn't include llvm-ar.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:169
+    libfile = "$dllfile.lib"
+    command = "link /nologo /dll {{ldflags}} /out:$dllfile /implib:$libfile {{libs}} /pdb:$dllfile.pdb {{inputs}}"
+    description = "LINK $dllfile"
----------------
phosek wrote:
> Can we use `lld-link` if `clang_base_path` is set?
Sure, why not :-)


================
Comment at: llvm/utils/gn/secondary/llvm/lib/Demangle/BUILD.gn:2
+static_library("Demangle") {
+  output_name = "LLVMDemangle"
+
----------------
phosek wrote:
> Setting `output_name` explicitly is generally considered an anti-pattern and should be used only when absolutely necessary. I think we should just  `"LLVMDemangle"` as the target name.
I had replied to this at https://reviews.llvm.org/D54345?id=173428#inline-480457


https://reviews.llvm.org/D54345