Skip to content

cmake: more small tidy-ups and fixes#14450

Closed
vszakats wants to merge 14 commits intocurl:masterfrom
vszakats:cm-strequal
Closed

cmake: more small tidy-ups and fixes#14450
vszakats wants to merge 14 commits intocurl:masterfrom
vszakats:cm-strequal

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Aug 7, 2024 •
edited
Loading

  • tidy up two MATCHES expression by avoiding macros expansion and
    adding quotes. Then convert then to STREQUAL to match other places
    in the code doing the same checks.

  • fix setting _ALL_SOURCE for AIX to match what autotools does.

  • delete stray _ALL_SOURCE reference from lib/config_riscos.h

  • simplify/fix two STREQUAL "" checks.
    The one in the openssl_check_symbol_exists() macro succeeded
    regardless of the value. The other could return TRUE when
    CMAKE_OSX_SYSROOT was undefined.

  • delete code for CMake versions (<3.7) we no longer support.

  • prefer LIST(APPEND ...) to extend CURL_LIBS.

  • use CURL_LIBS to add the network lib for Haiku.
    Before this patch it was done via raw C flags. I could not test this.

  • move _WIN32_WINNT-related code next to each other.
    It also moves detection to the top, allowing more code to use
    the result.

  • merge two WIN32 blocks.

  • rename internal variables to underscore + lowercase.

  • unwrap a line, indent, whitespace.

Closes #14450

vszakats added 2 commits August 8, 2024 00:53
@vszakats
formatting
5cde3db
@vszakats
cmake: tidy-up STREQUAL empty checks
d600bc8 
- delete it from the macro. It seems to always check out TRUE.

- replace with simple boolean check which doesn't check out TRUE
  if the variable is undefined, unlike STREQUAL.

Closes #xxxxx
@vszakats vszakats added the cmake label Aug 7, 2024
@github-actions github-actions bot added the tests label Aug 7, 2024
@vszakats vszakats changed the title cmake: fixes cmake: assortment of fixes and tidy-ups Aug 8, 2024
@vszakats vszakats changed the title cmake: assortment of fixes and tidy-ups cmake: assortment of tidy-ups Aug 8, 2024
@vszakats vszakats changed the title cmake: assortment of tidy-ups cmake: more small tidy-ups Aug 8, 2024
@vszakats vszakats changed the title cmake: more small tidy-ups cmake: more small tidy-ups and fixes Aug 8, 2024
@vszakats vszakats closed this in 919394e Aug 8, 2024
@vszakats vszakats deleted the cm-strequal branch August 8, 2024 11:49
@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024 •
edited
Loading

This odd AIX construct is also generated by autoreconf into lib/curl_config.h.in:

/* Define to 1 if OS is AIX. */
#ifndef _ALL_SOURCE
#  undef _ALL_SOURCE
#endif

What may be the purpose of this code?

Besides, autotools (like CMake after this PR) does define _ALL_SOURCE for AIX, which should work regardless of this code. What am I missing?

@dfandrich
Copy link
Contributor

dfandrich commented Aug 8, 2024 via email

@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024

Thanks Dan.

Do you know what purpose the #ifndef / #undef combo may have?
After staring at it for a while I cannot figure it out. It seems, just defining _ALL_SOURCE should be enough for AIX, or is it not?

@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024 •
edited
Loading

Ah OK, I think I get it, autotools converts the odd construct to this, when defined:

/* Define to 1 if OS is AIX. */
#ifndef _ALL_SOURCE
#  define _ALL_SOURCE 1
#endif

I still cannot test it, but the CMake method of passing -D_ALL_SOURCE via the command-line would likely work without such trick. (Unless, AIX also defines it to a different value without checking if it's already set).

Probably making it -D_ALL_SOURCE=1 would be useful to sync it more with autotools.

vszakats added a commit to vszakats/curl that referenced this pull request Aug 8, 2024
@vszakats
cmake: sync AIX _ALL_SOURCE trick with autotools
1f5a29a 
Follow-up to 919394e curl#14450
Closes #xxxxx
@vszakats vszakats mentioned this pull request Aug 8, 2024
Closed
@dfandrich
Copy link
Contributor

dfandrich commented Aug 8, 2024

All the AIX references to _ALL_SOURCE are either #ifdef or defined() so it doesn't need to be set to a value like 1. AIX seems to set _ALL_SOURCE itself if none of _XOPEN_SOURCE, _POSIX_SOURCE or ANSI_C_SOURCE is already set.

You should get yourself a Compile Farm account to help with this kind of testing. They have two different AIX versions available as well as other obscure machines.

@vszakats
Copy link
Member Author

vszakats commented Aug 8, 2024

Thanks, they answer the questions and I've closed #14461 as not necessary.

Also for the AIX tip!

@vszakats vszakats mentioned this pull request Aug 8, 2024
Closed
vszakats added a commit that referenced this pull request Aug 9, 2024
@vszakats
cmake: more small tidy-ups
a298df7 
- prefix local variables with underscore and convert to lowercase.
- list variables accepted by `libcurl.pc` and `curl-config` templates.
- quote more string literals.

Follow-up to 919394e #14450
Closes #14462
@vszakats vszakats mentioned this pull request Aug 16, 2024
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cmake tests tidy-up

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vszakats @dfandrich