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

cmake: support building static and shared libcurl in one go #11505

Closed
wants to merge 11 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jul 24, 2023

This patch adds the ability to build a static and shared libcurl library
in a single build session. It also adds an option to select which one to
use when building the curl executable.

New build options:

  • BUILD_STATIC_LIBS. Default: OFF.
    Enabled automatically if BUILD_SHARED_LIBS is OFF.
  • BUILD_STATIC_CURL. Default: OFF.
    Requires BUILD_STATIC_LIBS enabled.
    Enabled automatically if building static libcurl only.
  • STATIC_LIB_SUFFIX. Default: empty.
  • IMPORT_LIB_SUFFIX. Default: _imp if implib filename would collide
    with static lib name (typically with MSVC) in Windows builds.
    Otherwise empty.

Also:

  • Stop setting the CURL_STATICLIB macro via curl_config.h, and pass
    it directly to the compiler. This also allows to delete a condition
    from tests/server/CMakeLists.txt.

  • Complete a TODO by following the logic used in autotools (also for
    LIBCURL_NO_SHARED), and set -DCURL_STATICLIB in Cflags: of
    libcurl.pc for static-only curl builds.

  • Convert an existing CI test to build both shared and static libcurl.

Closes #11505

@vszakats vszakats added the cmake label Jul 24, 2023
@github-actions github-actions bot added cmdline tool tests CI Continuous Integration labels Jul 24, 2023
@vszakats vszakats added the feature-window A merge of this requires an open feature window label Jul 24, 2023
@vszakats vszakats added enhancement and removed cmdline tool tests CI Continuous Integration labels Jul 24, 2023
vszakats added a commit to curl/curl-for-win that referenced this pull request Jul 24, 2023
In preparation for single-pass shared/static CMake builds.

Ref: curl/curl#11505
@github-actions github-actions bot added cmdline tool tests CI Continuous Integration labels Jul 25, 2023
@vszakats vszakats removed cmdline tool tests CI Continuous Integration labels Jul 25, 2023
@github-actions github-actions bot added cmdline tool tests CI Continuous Integration labels Jul 25, 2023
Affecting those that might be empty.

```
CMake Error at lib/CMakeLists.txt:160 (set_target_properties):
  set_target_properties called with incorrect number of arguments.
```
@vszakats vszakats removed cmdline tool tests CI Continuous Integration labels Jul 27, 2023
@vszakats
Copy link
Member Author

vszakats commented Jul 27, 2023

I replicated existing behaviour for compatibility where the implib is always named libcurl_imp.lib when building for MSVC (or —after this patch— any other compiler prone to static/implib name collision).

Though as discovered earlier, this past choice appears to be arbitrary. Also the _imp suffix is seldom used on Windows (AFAIK).

A more widely used convention would be using libcurl_static.lib for the static lib. This can be further limited to builds building both shared and static lib at the same time. (I implemented it this way in libssh2, also present in this PR's commit history.)

It's an odd problem with no standard or perfect solution. Let me know if we should do any breaking changes here.

@github-actions github-actions bot added cmdline tool tests CI Continuous Integration labels Jul 27, 2023
@vszakats vszakats removed cmdline tool tests CI Continuous Integration feature-window A merge of this requires an open feature window labels Jul 27, 2023
@vszakats vszakats closed this in 1199308 Jul 29, 2023
@vszakats vszakats deleted the cmake-dual branch July 29, 2023 00:41
vszakats added a commit to curl/curl-for-win that referenced this pull request Jul 29, 2023
Making it clear that the measurement was done with separate shared and
static build passes, and without this patch:

curl/curl@1199308
curl/curl#11505
vszakats added a commit to curl/curl-for-win that referenced this pull request Jul 29, 2023
vszakats added a commit to vszakats/curl that referenced this pull request Aug 8, 2023
Follow-up to 1199308 curl#11505

Reported-by: balikalina on Github
Fixes curl@1199308#r123923098
Closes #xxxxx
vszakats added a commit that referenced this pull request Aug 9, 2023
Replace (wrong) literal with a variable to specify the curl
namespace.

Follow-up to 1199308 #11505

Reported-by: balikalina on Github
Fixes 1199308#r123923098
Closes #11629
vszakats added a commit to vszakats/curl that referenced this pull request Aug 9, 2023
Fixes:
```
[25/26] -- Found CURL: ***/build/parts/curl/install/lib64/cmake/CURL/CURLConfig.cmake (found version "8.3.0-DEV")
[25/26] CMake Error at ***/build/parts/curl/install/lib64/cmake/CURL/CURLConfig.cmake:62 (add_library):
[25/26]   add_library cannot create ALIAS target "CURL::libcurl" because target
[25/26]   "CURL::libcurl_static" is imported but not globally visible.
```

Follow-up to 1199308 curl#11505

Reported-by: balikalina on Github
Suggested-by: balikalina on Github
Ref: curl#11629 (comment)
Closes #xxxxx
vszakats added a commit to vszakats/curl that referenced this pull request Aug 11, 2023
Fixes:
```
[25/26] -- Found CURL: ***/build/parts/curl/install/lib64/cmake/CURL/CURLConfig.cmake (found version "8.3.0-DEV")
[25/26] CMake Error at ***/build/parts/curl/install/lib64/cmake/CURL/CURLConfig.cmake:62 (add_library):
[25/26]   add_library cannot create ALIAS target "CURL::libcurl" because target
[25/26]   "CURL::libcurl_static" is imported but not globally visible.
```

Follow-up to 1199308 curl#11505

Reported-by: balikalina on Github
Suggested-by: balikalina on Github
Ref: curl#11629 (comment)
Closes #xxxxx
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
This patch adds the ability to build a static and shared libcurl library
in a single build session. It also adds an option to select which one to
use when building the curl executable.

New build options:
- `BUILD_STATIC_LIBS`. Default: `OFF`.
  Enabled automatically if `BUILD_SHARED_LIBS` is `OFF`.
- `BUILD_STATIC_CURL`. Default: `OFF`.
  Requires `BUILD_STATIC_LIBS` enabled.
  Enabled automatically if building static libcurl only.
- `STATIC_LIB_SUFFIX`. Default: empty.
- `IMPORT_LIB_SUFFIX`. Default: `_imp` if implib filename would collide
  with static lib name (typically with MSVC) in Windows builds.
  Otherwise empty.

Also:

- Stop setting the `CURL_STATICLIB` macro via `curl_config.h`, and pass
  it directly to the compiler. This also allows to delete a condition
  from `tests/server/CMakeLists.txt`.

- Complete a TODO by following the logic used in autotools (also for
  `LIBCURL_NO_SHARED`), and set `-DCURL_STATICLIB` in `Cflags:` of
  `libcurl.pc` for _static-only_ curl builds.

- Convert an existing CI test to build both shared and static libcurl.

Closes curl#11505
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
Replace (wrong) literal with a variable to specify the curl
namespace.

Follow-up to 1199308 curl#11505

Reported-by: balikalina on Github
Fixes curl@1199308#r123923098
Closes curl#11629
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 25, 2023
Switch from `curl-gnumake.sh` to `curl-cmake.sh` with upcoming curl
release v8.5.0.

cmake builds are now _faster_ for Windows builds than raw gnumake
(aka `Makefile.mk`). They also use 'unity' mode, which makes builds
run fast with the side-effect of also creating potentially more
efficient binaries by allowing better compiler optimizations.

This also makes curl-for-win use the same build system for all target
platforms (`Makefile.mk` is not available for *nix platforms).

Initially on 2022-07-04 cmake was 25% slower than gnumake. By
2022-09-26 this reduced to 20%, by 2023-07-29 to 18% and after the
latest round of updates gnumake came out 7% slower than cmake.
This is for Windows, for a triple x64, arm64 and x86 build. In
absolute times this is 27m22s for cmake and 29m11s for gnumake.

(FWIW autotools builds are 52% slower than cmake unity builds now.)

Making cmake builds fast was a multi-year effort with these major steps:

1. add support for cmake builds in curl-for-win.
   420e73c
2. bring it in-line with gnumake and autotools builds and tidy-up
   as much as possible. Scattered to a many commits.
3. delete a whole bunch of unused feature detections.
   curl/curl@4d73854
   curl/curl#9044
   (and a lot more smaller commits)
4. speed up picky warning option initialization by avoiding brute-force
   all options. (first in libssh2, then in curl, then applied also
   ngtcp2, nghttp3, nghttp2)
   curl/curl@9c543de
   curl/curl#10973
5. implement single build run for shared and static libcurl + tool
   (first in libssh2)
   curl/curl@1199308
   curl/curl#11505
   53dcd49
6. implement single build pass for shared and static libcurl
   (for Windows initially)
   curl/curl@2ebc74c
   curl/curl#11546
7. extend above to non-Windows (e.g. macOS)
   curl/curl@fc9bfb1
   curl/curl#11627
   bafa77d (mac)
   1b27304 (linux)
8. implement unity builds: single compiler invocation for libcurl + tool
   curl/curl@3f8fc25
   curl/curl#11095
   curl/curl@f42a279
   curl/curl#11928
   67d7fd3
9. speed up 4x the cmake initialization phase (for Windows)
   curl/curl@2100d9f
   curl/curl#12044
10. speed up cmake initialization even more by pre-filling
   detection results for our well-known mingw-w64 environments.
   74dd967
   5a43c61

+1: speed up autotools initialization by mitigating a brute-force
   feature detection on Windows. This reduced total build times
   by 5 minutes at the time, for the 3 Windows targets in total.
   curl/curl@6adcff6
   curl/curl#9591

Also update build times for cmake-unity and gnumake, based on runs:
cmake-unity: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48357875
gnumake: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48358005
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

1 participant