Skip to content

Find CMake files for add_subdirectory(curl) use-case #498

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Find CMake files for add_subdirectory(curl) use-case #498

wants to merge 1 commit into from

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Oct 19, 2015 •
edited by vszakats
Loading

When including CURL using add_subdirectory the variables CMAKE_BINARY_DIR and CURL_BINARY_DIR hold different paths.

(It was PR #488 but I messed it up :/)

@jgsogo
Find CMake files for add_subdirectory(curl) use-case
3996dca 
When including CURL using add_subdirectory the variables
CMAKE_BINARY_DIR and CURL_BINARY_DIR hold different paths.
@jgsogo jgsogo mentioned this pull request Oct 19, 2015
Closed
@jgsogo
Copy link
Contributor Author

jgsogo commented Oct 19, 2015

@snikulov @bagder @bradking @whoshuu I'm closing PR#488 in favor of this one. I don't know but happened there when trying to rewrite commit message.

@bradking
Copy link
Contributor

Thanks. LGTM.

@jzakrzewski
Copy link
Contributor

Just out of curiosity - the change makes it more consistent, so that's ok but isn't it invalid to build this via add_subdirectory()? I mean it contains both: cmake_minimum_required an project. As far as I know it can mess up ones build badly.

@bradking
Copy link
Contributor

@jzakrzewski: Using add_subdirectory to include one project inside another is supported by CMake. It just takes some care that the inner project does not reference CMAKE_BINARY_DIR and such. Actually CMake's own source builds an internal libcurl using add_subdirectory to support the file(DOWNLOAD) and a few other commands.

@jgsogo jgsogo mentioned this pull request Oct 19, 2015
Closed
@snikulov
Copy link
Contributor

LGTM 👍

@jzakrzewski
Copy link
Contributor

@bradking Thanks for clarification. I have just never used it.

@jay jay closed this in 72646c2 Oct 19, 2015
@jay
Copy link
Member

jay commented Oct 19, 2015

Thanks, landed in 72646c2.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jgsogo @bradking @jzakrzewski @snikulov @jay