Skip to content
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

Ignore broken curl-config.cmake #40

Merged
merged 2 commits into from
Apr 8, 2020
Merged

Ignore broken curl-config.cmake #40

merged 2 commits into from
Apr 8, 2020

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Apr 8, 2020

This flag was introduced to maintain existing behavior in CMake 3.17 and newer, where FindCURL.cmake was changed to use curl-config.cmake if it is found.

It appears that the curl-config.cmake that comes with the current version of curl that this package includes does not work properly. Until we target a version of curl that has a working curl-config.cmake on Windows, we should continue with the CMake behavior prior to 3.17 by specifying CURL_NO_CURL_CMAKE.

Here is the change in CMake 3.17: Kitware/CMake@c11e7c5

Alternative to #39
Closes #38

This flag was introduced to maintain existing behavior in CMake 3.17 and
newer, where FindCURL.cmake was changed to use curl-config.cmake if it
is found.

It appears that the curl-config.cmake that comes with the current
version of curl that this package includes does not work properly. Until
we target a version of curl that has a working curl-config.cmake on
Windows, we should continue with the CMake behavior prior to 3.17 by
specifying CURL_NO_CURL_CMAKE.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@Karsten1987
Copy link

If that passes CI, I think I prefer this over the alternative.

@sloretz
Copy link
Contributor

sloretz commented Apr 8, 2020

CI (build all, test libcurl_vendor resource_retriever)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987
Copy link

@sloretz I would recommend also testing RViz. I think the default plugins make use of the resource retriever.

@sloretz
Copy link
Contributor

sloretz commented Apr 8, 2020

I would recommend also testing RViz. I think the default plugins make use of the resource retriever.

CI (test libcurl_vendor resource_retriever, rviz_default_plugins)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@cottsay
Copy link
Member Author

cottsay commented Apr 8, 2020

@sloretz - the builds LGTM. Just the one CMake warning on Windows - looks related to spdlog though.

@sloretz sloretz merged commit dd142bb into ros:ros2 Apr 8, 2020
@cottsay cottsay deleted the no_curl_config branch April 8, 2020 19:10
sloretz pushed a commit that referenced this pull request Apr 9, 2020
* Ignore broken curl-config.cmake

This flag was introduced to maintain existing behavior in CMake 3.17 and
newer, where FindCURL.cmake was changed to use curl-config.cmake if it
is found.

It appears that the curl-config.cmake that comes with the current
version of curl that this package includes does not work properly. Until
we target a version of curl that has a working curl-config.cmake on
Windows, we should continue with the CMake behavior prior to 3.17 by
specifying CURL_NO_CURL_CMAKE.

Signed-off-by: Scott K Logan <logans@cottsay.net>

* Only ignore curl-config.cmake for vendor-built curl

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
sloretz added a commit that referenced this pull request Apr 9, 2020
* Ignore broken curl-config.cmake

This flag was introduced to maintain existing behavior in CMake 3.17 and
newer, where FindCURL.cmake was changed to use curl-config.cmake if it
is found.

It appears that the curl-config.cmake that comes with the current
version of curl that this package includes does not work properly. Until
we target a version of curl that has a working curl-config.cmake on
Windows, we should continue with the CMake behavior prior to 3.17 by
specifying CURL_NO_CURL_CMAKE.

Signed-off-by: Scott K Logan <logans@cottsay.net>

* Only ignore curl-config.cmake for vendor-built curl

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Co-authored-by: Scott K Logan <logans@cottsay.net>
nuclearsandwich pushed a commit that referenced this pull request Jun 30, 2020
* Ignore broken curl-config.cmake

This flag was introduced to maintain existing behavior in CMake 3.17 and
newer, where FindCURL.cmake was changed to use curl-config.cmake if it
is found.

It appears that the curl-config.cmake that comes with the current
version of curl that this package includes does not work properly. Until
we target a version of curl that has a working curl-config.cmake on
Windows, we should continue with the CMake behavior prior to 3.17 by
specifying CURL_NO_CURL_CMAKE.

Signed-off-by: Scott K Logan <logans@cottsay.net>

* Only ignore curl-config.cmake for vendor-built curl
nuclearsandwich added a commit that referenced this pull request Jul 1, 2020
* Ignore broken curl-config.cmake

This flag was introduced to maintain existing behavior in CMake 3.17 and
newer, where FindCURL.cmake was changed to use curl-config.cmake if it
is found.

It appears that the curl-config.cmake that comes with the current
version of curl that this package includes does not work properly. Until
we target a version of curl that has a working curl-config.cmake on
Windows, we should continue with the CMake behavior prior to 3.17 by
specifying CURL_NO_CURL_CMAKE.

Signed-off-by: Scott K Logan <logans@cottsay.net>

* Only ignore curl-config.cmake for vendor-built curl

Co-authored-by: Scott K Logan <logans@cottsay.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants