[PATCH] D82994: [RFC] Instrumenting Clang/LLVM with Perfetto

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/cmake/modules/AddPerfetto.cmake:9
+  ExternalProject_Add(perfetto_git
+      GIT_REPOSITORY https://github.com/google/perfetto.git
+      GIT_TAG releases/v4.x
----------------
nickdesaulniers wrote:
> lebedev.ri wrote:
> > nickdesaulniers wrote:
> > > lebedev.ri wrote:
> > > > I have concerns about this.
> > > > It really should use system-provided version via `find_package()`
> > > > At worst, the sources should be bundled into the tree like it's already done in some rare cases.
> > > Not that I'm very good with CMake, but this seems to suggest that `ExternalProject_Add` may not compose well with `find_package`: https://stackoverflow.com/questions/6351609/cmake-linking-to-library-downloaded-from-externalproject-add
> > What i am saying is that any code that fetches anything from internet during cmake/build time is just plain broken.
> > perfetto should be provided by system package and we should link to it just like we link to zlib/etc.
> That's how GTest is fetched.  See: llvm/utils/benchmark/cmake/HandleGTest.cmake.
> That's how GTest is fetched.

No, it's not. That cmake file is never ever executed by LLVM's cmake.
LLVM's gtest is bundled in `llvm/utils/unittest/googletest`,
much like googlebenchmark is bundled in `llvm/utils/benchmark`,
etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82994