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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant