[PATCH] D61660: [cmake] Make google benchmark project handle libraries properly when built in-tree

[Don Hinton via Phabricator via llvm-commits]

hintonda marked 2 inline comments as done.
hintonda added a comment.

Thanks, and no worries...



================
Comment at: llvm/utils/benchmark/CMakeLists.txt:11
+  endif()
 
+  project (benchmark)
----------------
lebedev.ri wrote:
> I.e. i'm wondering if this `endif()` should be here
It matches line 8: `if(POLICY CMP0048)`, so it currently is needed.

However, the entire `if(POLICY CMP0048)` may no longer be needed if all of this remains couched in the larger `if(...)`.  I'll let @beanz comment on that, since I haven't really investigated it. 

That's more a benchmark issue, so as long as this works within LLVM, I'm happy.


================
Comment at: llvm/utils/benchmark/CMakeLists.txt:25-26
+
+# This wrapper is needed when including this project as a subproject,
+# and the parent project has defined benchmark_add_library, e.g., LLVM.
+if(NOT COMMAND benchmark_add_library)
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > Perfect, thank you!
> > If you wish to submit this upstream, i **will** //gladly// merge this part.
> I think the comment reads weird, inside-out.
> Maybe
> ```
> # Define this wrapper unless it is already defined,
> # e.g. because benchmark is included as a subproject,
> # and the parent project has provided it's own wrapper.
> ```
You're right, I'll fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61660