[libcxx-commits] [PATCH] D60372: [gn] Support for building libc++abi

[Nico Weber via Phabricator via libcxx-commits]

thakis accepted this revision.
thakis added a comment.

Sorry, didn't realize I hadn't accepted this yet. lgtm with the change you had suggested below.



================
Comment at: llvm/utils/gn/secondary/libcxxabi/src/BUILD.gn:17
+  # Do not export any symbols from the static library.
+  libcxxabi_hermetic_static_library = false
+}
----------------
phosek wrote:
> thakis wrote:
> > phosek wrote:
> > > thakis wrote:
> > > > Wouldn't you always want this? What's an example where one would want to statically link c++abi but export its symbols? They only thing I can think of is when linking it into libc++.dylib, which which case we probably want a toggle for that instead?
> > > > 
> > > > Why is the default false? Why is this settable?
> > > I've tried to match the CMake build where this is disabled by default. I agree with your reasoning but I'm not sure whether we want to deviate from the CMake build?
> > I think it's fine to deviate from CMake where it makes sense.
> > 
> > - there are many examples of things that are options in cmake but not in gn (usually, because noone needed the option in GN yet, but not always)
> > 
> > - there are a few examples where variable defaults are different because cmake's defaults are arguably not optimal for the average case (examples: gn gives you release+asserts by default while cmake gives you debug by default, gn only builds host target by default while cmake builds all of them by default, ...)
> > 
> > So if that's the only reason this is the way it is, I think it's fine to change it if you agree that that makes sense.
> It depends on the expected use case. If you want to build static libc++abi so you can link it into the shared libc++, then you want this option to be disabled. If you want to build static libc++abi so you can distribute it as an archive, you probably want it enabled. It seems like CMake build is optimized for the former, but in practice I'd argue that the latter is more common so I'm fine making that the default.
Sounds good.


Repository:
  rCXXA libc++abi

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

https://reviews.llvm.org/D60372