moves most third-party packages into /third_party#325
Open
cjdb wants to merge 5 commits intojeremy-rifkin:mainfrom
Open
moves most third-party packages into /third_party#325cjdb wants to merge 5 commits intojeremy-rifkin:mainfrom
/third_party#325cjdb wants to merge 5 commits intojeremy-rifkin:mainfrom
Conversation
Proper use of `FetchContent` requires sequencing all calls to
`FetchContent_Declare` before any calls to `FetchContent_MakeAvailable`.
This allows dependencies with similar `FetchContent` calls to coalesce
their declarations, so that projects don't end up repeatedly downloading
and building identical packages.
We introduce a directory dedicated to third-party packages for two
reasons:
1. Moving all of the third-party packages into a single directory
improves readability, as it becomes more apparent that calls to
`FetchContent_Declare` and `FetchContent_MakeAvailable` are correctly
sequenced.
2. Providing each third-party package with its own directory allows us
to colocate the `FindPackage_Declare` with any relevant project-local
patches. There aren't any at present, so we document how this ought
to be done below.
libdwarf and libunwind are still handled in `/CMakeLists.txt`. Their
requirements seem to be substantially more complicated than all other
third-party packages, so we'll migrate them into `/third_party` at a
later time.
**How to patch a third-party package**
Suppose googletest needed to be updated, and requires a local patch so
cpptrace can continue to support C++11. To integrate this with
FetchContent, we:
1. Make the necessary changes in our local fork of `google/googletest`.
2. Run `git format-patch @{u} -o /path/to/cpptrace/third_party/googletest`
to generate the patch files directly into `/third_party/googletest`.
3. Edit `/third_party/googletest/CMakeLists.txt` like so:
```
FetchContent_Declare(
"${package_name}"
GIT_REPOSITORY "https://github.com/google/googletest.git"
- GIT_TAG release-v1.12.1 # last to support C++11
+ GIT_TAG v1.17.0
+ PATCH_COMMAND git am 0001-makes-foo-c++11-compatible.patch
+ 0002-makes-bar-c++11-compatible.patch
+ 0003-makes-baz-c++11-compatible.patch
OVERRIDE_FIND_PACKAGE
SYSTEM
)
```
This will apply the patches during the `FetchContent` process, and
because all the `FetchContent_Declare`s come first, it will ensure that
the patches aren't clobbered by any dependencies.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proper use of
FetchContentrequires sequencing all calls toFetchContent_Declarebefore any calls toFetchContent_MakeAvailable. This allows dependencies with similarFetchContentcalls to coalesce their declarations, so that projects don't end up repeatedly downloading and building identical packages.We introduce a directory dedicated to third-party packages for two reasons:
FetchContent_DeclareandFetchContent_MakeAvailableare correctly sequenced.FindPackage_Declarewith any relevant project-local patches. There aren't any at present, so we document how this ought to be done below.libdwarf and libunwind are still handled in
/CMakeLists.txt. Their requirements seem to be substantially more complicated than all other third-party packages, so we'll migrate them into/third_partyat a later time.How to patch a third-party package
Suppose googletest needed to be updated, and requires a local patch so cpptrace can continue to support C++11. To integrate this with FetchContent, we:
google/googletest.git format-patch @{u} -o /path/to/cpptrace/third_party/googletestto generate the patch files directly into/third_party/googletest./third_party/googletest/CMakeLists.txtlike so:FetchContent_Declare( "${package_name}" GIT_REPOSITORY "https://github.com/google/googletest.git" - GIT_TAG 58d77fa8070e8cec2dc1ed015d66b454c8d78850 # v1.12.1, last to support C++11 + GIT_TAG v1.17.0 + PATCH_COMMAND git am 0001-makes-foo-c++11-compatible.patch + 0002-makes-bar-c++11-compatible.patch + 0003-makes-baz-c++11-compatible.patch OVERRIDE_FIND_PACKAGE SYSTEM )This will apply the patches during the
FetchContentprocess, and because all theFetchContent_Declares come first, it will ensure that the patches aren't clobbered by any dependencies.