This is an archive of the discontinued LLVM Phabricator instance.

[Support] [Debuginfod] Use libcurl imported library.
ClosedPublic

Authored by noajshu on Dec 6 2021, 2:39 PM.

Details

Summary

A report on llvm-dev indicated that curl.h could be found during build configuration but not during compilation. This diff uses the libcurl imported library created by findcurl, so that includes and libs are automatically managed correctly by cmake.

Diff Detail

Event Timeline

noajshu created this revision.Dec 6 2021, 2:39 PM
noajshu requested review of this revision.Dec 6 2021, 2:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2021, 2:39 PM
phosek added a subscriber: phosek.Dec 6 2021, 3:00 PM
phosek added inline comments.
llvm/lib/Support/CMakeLists.txt
79

You need to use the imported library here instead which configures both the include directory and the library. See zlib case above where we do the same.

296–311

You also need to replicate the ZLIB block above for CURL here.

333–338

This shouldn't be needed.

noajshu updated this revision to Diff 392196.Dec 6 2021, 3:04 PM
noajshu marked 2 inline comments as done.

Treat Curl similar to Zlib.

noajshu updated this revision to Diff 392198.Dec 6 2021, 3:08 PM

Add block for importing curl with llvm-config.

phosek accepted this revision.Dec 6 2021, 3:19 PM

LGTM

This revision is now accepted and ready to land.Dec 6 2021, 3:19 PM
noajshu retitled this revision from [Support] [Debuginfod] Include curl include dirs when curl is enabled. (WIP) to [Support] [Debuginfod] Use libcurl imported library. (WIP).Dec 6 2021, 5:31 PM
noajshu edited the summary of this revision. (Show Details)
noajshu updated this revision to Diff 392249.EditedDec 6 2021, 6:07 PM
noajshu marked an inline comment as done.

Rebase against main after landing D115131.
Now I am seeing this:

CMake Warning (dev) in CMakeLists.txt:
  Policy CMP0111 is not set: An imported target missing its location property
  fails during generation.  Run "cmake --help-policy CMP0111" for policy
  details.  Use the cmake_policy command to set the policy and suppress this
  warning.

  IMPORTED_LOCATION not set for imported target "CURL::libcurl" configuration
  "Debug".
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) in CMakeLists.txt:
  Policy CMP0111 is not set: An imported target missing its location property
  fails during generation.  Run "cmake --help-policy CMP0111" for policy
  details.  Use the cmake_policy command to set the policy and suppress this
  warning.

  IMPORTED_LOCATION not set for imported target "CURL::libcurl" configuration
  "Debug".
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) in CMakeLists.txt:
  Policy CMP0111 is not set: An imported target missing its location property
  fails during generation.  Run "cmake --help-policy CMP0111" for policy
  details.  Use the cmake_policy command to set the policy and suppress this
  warning.

  IMPORTED_LOCATION not set for imported target "CURL::libcurl" configuration
  "Debug".
This warning is for project developers.  Use -Wno-dev to suppress it.

And then when I try to build, I get:

ninja: Entering directory `build'
ninja: error: 'CURL::libcurl-NOTFOUND', needed by 'unittests/Debuginfod/DebuginfodTests', missing and no known rule to make it
noajshu retitled this revision from [Support] [Debuginfod] Use libcurl imported library. (WIP) to [Support] [Debuginfod] Use libcurl imported library..Dec 6 2021, 6:08 PM
phosek added a comment.Dec 6 2021, 6:49 PM
ninja: Entering directory `build'
ninja: error: 'CURL::libcurl-NOTFOUND', needed by 'unittests/Debuginfod/DebuginfodTests', missing and no known rule to make it

This means that cURL wasn't found so LLVM_ENABLE_CURL should evaluate to false, but in your case it appears to be evaluating to true. I'd check https://github.com/llvm/llvm-project/blob/b206ee69061120d559d8315bede6c63758b06154/llvm/cmake/config-ix.cmake#L180.

noajshu updated this revision to Diff 392980.Dec 8 2021, 4:51 PM

Remove extra whitespace line.

This revision was landed with ongoing or failed builds.Dec 8 2021, 5:49 PM
This revision was automatically updated to reflect the committed changes.