CMake usage updates to INSTALL-CMAKE.md #16329
Conversation
* Links to jump to relevant sections more quickly (build migration, CMake options) * Notes on best practices, e.g. build out-of-source, single vs. multi config * New section on usage from CMake, in particular features/protocols support * Update some of the build migration sections (multi-config notes)
|
Spellcheck did not like |
docs/INSTALL-CMAKE.md
Outdated
| @@ -49,6 +58,9 @@ If you want to build in the source tree, it is enough to do this: | |||
|
|
|||
| $ cmake . | |||
|
|
|||
| Note, however, that it is recommended to build apart from the source tree, to | |||
There was a problem hiding this comment.
I think "apart" sounds a bit odd.
| Note, however, that it is recommended to build apart from the source tree, to | |
| It is recommended to build curl outside of the source tree, to |
There was a problem hiding this comment.
Yes, I agree. I think I chose this wording because earlier in the section, an out-of -source build is referred to as building "apart from the source tree", so I decided to keep this same language.
I definitely think your suggestion has better wording.
There was a problem hiding this comment.
Actually, I will move this content upwards to the bullets where the in-source and separate build directory methods of running the CMake configure are being showcased. It's more natural to put this info there.
| produced by the `curl-config` script is determined at CMake configure time. If | ||
| you want to set a custom install prefix for CURL, use the | ||
| [`CMAKE_INSTALL_PREFIX`](https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX.html) | ||
| when configuring the CMake build. |
There was a problem hiding this comment.
Unless curl behaves differently than most other projects out there, I'd rather omit general, non-curl specific, advice about CMAKE_INSTALL_PREFIX and --prefix, or at least make it less strong by dropping emphasis.
If curl needs special handling compared to other CMake builds, we should probably aim to fix the issue rather than document it.
There was a problem hiding this comment.
We can drop the emphasis. However, the CMAKE_INSTALL_PREFIX and curl-config relationship is definitely specific to the curl project. I thought I made a note about this in #15854, but I clearly did not, and I only remembered this relationship while working on the documentation changes.
I'm not sure about other people, but when I do local build-from-source stuff on Windows I generally prefer to install into %HOME%/abc-major.minor.patch or something similar using --prefix during the cmake --install step. However, curl-config --prefix will instead report the value of CMAKE_INSTALL_PREFIX because it has already been configured as such. I suppose that supporting the use of --prefix will require an install(SCRIPT ...) rule to reconfigure curl-config on-the-fly with an updated install prefix as a pre-install action.
There was a problem hiding this comment.
It looks fairly common that non-CMake files like *.pc and *-config are non-relocatable and need the prefix configured early. On a quick check, none of the curl dependencies do install(SCRIPT ..) and require this.
(On the flipside, LibreSSL breaks curl-for-win if using CMAKE_INSTALL_PREFIX because it embeds the custom local path (prefix) into the binary, which is undesired.)
I'm still not convinced this advice is necessary here.
There was a problem hiding this comment.
I suppose that in the wild there may not be much support for --prefix (I think it was added in 3.15) because it is more of a recent option. Can't say how many others (besides me) like to use this, but I did some messing around to see if curl-config could be reconfigured before installation and that was already some work (and I haven't gotten it working). Since there's also the .pc file to worry about, supporting --prefix is probably not worth the effort; essentially both have to be reconfigured for each cmake --install invocation.
But if CMAKE_INSTALL_PREFIX is not going to work correctly we definitely should document that. Installing into a non-default root is pretty common, especially on Windows (to avoid Program Files, etc.) :(
There was a problem hiding this comment.
CMAKE_INSTALL_PREFIX should be fine with curl and most other project too. LibreSSL is an outlier and not a concern regarding curl docs.
Since this recommendation is true for most projects, I still think this is something we can leave out from curl docs, though I don't feel strongly about it. If it helps using CMake with curl, we can have it here.
There was a problem hiding this comment.
Yes, CMAKE_INSTALL_PREFIX works as expected with curl. But I think I will leave the cmake --install --prefix note in there since it is not 100% expected behavior. I'll also go and look over my changes in case of weird wording.
Co-authored-by: Daniel Stenberg <daniel@haxx.se>
|
Ooh, some CI test fails. The msys2 one just seems to be a GitHub Actions timeout (reached CI job runtime limit), but the other two have failing test cases (1196 failed for maketgz, 1631 + 1632 failed for FreeBSD). I should keep the branch up to date anyways and it will be good to just re-run CI in case of flakiness. |
…n section
modified: .github/scripts/spellcheck.words
Remove NMake (no longer using in a context that will be flagged)
modified: docs/INSTALL-CMAKE.md
Only one sentence for single vs. multi-config generator since we have link
|
@phetdam Left a nit note about indentation. Apart from it, this LGTM. edit: the timeouts (or hangs really) sometimes happen on the Windows runners. You may ignore them. |
Great! I'll fix the indentation and good to know that the CI failures aren't from some mysterious cause. |
|
@phetdam: Let me know when you're ready for merge. |
Just need to make a couple tweaks and I'll be good :) |
|
@vszakats Alright, I put a couple final tweaks and I think I'm now ready for merge. |
|
Thank you @phetdam, this is merged now! |
This PR updates the CMake build/install docs in
docs/INSTALL-CMAKE.md, in particular focusing on the use of libcurl from CMake usingfind_packageas well as the newly added features/protocols support via usingCOMPONENTSorOPTIONAL_COMPONENTSwithfind_package. See #15854 for initial discussion and the corresponding PR #15858 that was merged.Some additional best-practices notes are added, for example:
--configwithcmake --buildfor multi-config CMake generators, notCMAKE_BUILD_TYPEWe also add a CURL CMake-specific tip on using
CMAKE_INSTALL_PREFIXduring configure time to set the install prefix, not using--prefixwhen runningcmake --installsocurl-configoutput is consistent.