[PATCH] D61660: [cmake] Make google benchmark project call llvm_add_library

[Don Hinton via Phabricator via llvm-commits]

hintonda added a comment.

In D61660#1495366 <https://reviews.llvm.org/D61660#1495366>, @lebedev.ri wrote:

> In D61660#1495354 <https://reviews.llvm.org/D61660#1495354>, @hintonda wrote:
>
> > In D61660#1495331 <https://reviews.llvm.org/D61660#1495331>, @lebedev.ri wrote:
> >
> > > .. this doesn't look like what i proposed, and has some unrelated diff?
> >
> >
> > It uses your wrapper suggestion, but keeps everything local to benchmark.  Generally, if projects are build within other projects, e.g., see `clang/CMakeLists.txt`, they don't redeclare `project()`.  Doing so wipes out some state, most notably policies.  By checking, you preserve whatever the parent project set.
>
>
> I'm sorry, i don't understand.
>  Why?
>  The diff i proposed i will happily merge upstream into google benchmark.
>  This - no.


Sure, but so will this one, and it doesn't require any changes to LLVM.

Alternatively, I could do one of the following:

- leave the `project(benchmark)` command as is and move the conditional before it.
- fix benchmark to build libraries correctly on OSX -- essentially duplicating logic in `llvm_setup_rpath()`.

But I really don't think fixing broken sub-projects should require changes to LLVM.  It's the subproject that's broken, not LLVM.

> 
> 
>> The only other change is just to fix the path to be relative to the cmake file instead of the project root, which will be different when contained in another project.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61660