-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert cmake build to (mostly) use urls rather than git clones #852
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,18 +13,14 @@ | |
# limitations under the License. | ||
|
||
include(ExternalProject) | ||
include(ExternalProjectFlags) | ||
|
||
ExternalProject_GitSource( | ||
GOOGLETEST_GIT | ||
GIT_REPOSITORY "https://github.com/google/googletest.git" | ||
GIT_TAG "release-1.8.0" | ||
) | ||
|
||
ExternalProject_Add( | ||
googletest | ||
|
||
${GOOGLETEST_GIT} | ||
DOWNLOAD_DIR ${PROJECT_BINARY_DIR}/downloads | ||
DOWNLOAD_NAME googletest-1.8.0.tar.gz | ||
URL https://github.com/google/googletest/archive/release-1.8.0.tar.gz | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (As mentioned) It's possible to use http (rather than https) here, which may help if cmake is having trouble with ssl. But unless there's a compelling reason, I'd rather just use https. Note that the URL_HASH is (more-or-less) required even when using https. (If you omit it, cmake can't tell if the file was downloaded successfully, so it insists on re-downloading it.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https FTW. Let there be no unencrypted bits. |
||
URL_HASH SHA256=58a6f4277ca2bc8565222b3bbd58a177609e9c488e8a72649359ba51450db7d8 | ||
|
||
PREFIX ${PROJECT_BINARY_DIR}/external/googletest | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This puts the downloaded tarballs into ./build/downloads. Alternatively, we could put them into the top level directory (./downloads or similar). This would prevent them from being deleted when the user make clean's via a
rm -rf ./build
. OTOH, it's kinda surprising that the build would touch files outside of ./build.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM. Travis caching doesn't really care.