-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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: do not force Windows target versions #9046
Conversation
This PR deprecates I'm happy to discuss what to do with this option. Keep it as-is, leave it deprecated, delete it? |
093fae5
to
644b784
Compare
I'm not very familiar with this aspect of CMake, but several other projects I use provide a way to add compiler or preprocessor options without overriding the defaults via If that assumption is wrong or there's a different way to do that, I'm all for deleting the option. Then I'm probably the only one using it anyway 🙂 |
@MarcelRaad Thanks for your feedback. If this brings things closer to the CMake-way, we can certainly keep it. I haven't tried it, but CMake says to also support |
Projects tend to provide dedicated options for aspects that they deem "frequently used" or "overly complicated to setup otherwise". So the question would be if this setting falls into one of those categories, I guess. I for sure don't know ;) |
The goal of this patch is to avoid CMake forcing specific Windows versions and rely on toolchain defaults or manual selection instead. This gives back control to the user. This also brings CMake closer to how autotools and `Makefile.m32` behaves in this regard. - CMake had a setting `ENABLE_INET_PTON` defaulting to `ON`, which did nothing else than fixing the Windows build target to Vista. This also happened when the toolchain did not have Vista support (e.g. original MinGW), breaking such builds. In other environments it did not make a user-facing difference, because libcurl has its own pton() implementation, so it works well with or without Vista's inet_pton(). This patch drops this setting. inet_pton() is now used whenever building for Vista or newer, either when requested manually or by default with modern toolchains (e.g. mingw-w64). Older envs will fall back to curl's pton(). Ref: curl#9027 (comment) Ref: curl#8997 (comment) - When the user did no select a Windows target version manually, stop explicitly targeting Windows XP, and instead use the toolchain default. This may pose an issue with old toolchains defaulting to pre-XP targets. In such case you must manually target Windows XP via: `-DCURL_TARGET_WINDOWS_VERSION=0x0501` or `-DCMAKE_C_FLAGS=-D_WIN32_WINNT=0x0501` Closes #xxxx
Thanks for the inputs. I deleted the deprecation part. It's nice to make passing this setting a little bit friendlier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would save this for the next feature window which is in several days barring any issues worthy of a patch release
Sync up OS target selection settings with autotools and m32. Ref: curl/curl#9046
The goal of this patch is to avoid CMake forcing specific Windows
versions and rely on toolchain defaults or manual selection instead.
This gives back control to the user. This also brings CMake closer to
how autotools and
Makefile.m32
behaves in this regard.CMake had a setting
ENABLE_INET_PTON
defaulting toON
, which didnothing else than fixing the Windows build target to Vista. This also
happened when the toolchain did not have Vista support (e.g. original
MinGW), breaking such builds.
In other environments it did not make a user-facing difference,
because libcurl has its own
pton()
implementation, so it works wellwith or without Vista's
inet_pton()
.This patch drops this setting.
inet_pton()
is now used wheneverbuilding for Vista or newer, either when requested manually or by
default with modern toolchains (e.g. mingw-w64). Older envs will fall
back to curl's
pton()
.Ref: windows: improve random source #9027 (comment)
Ref: lib/easy_lock.h: include synchapi.h on Windows #8997 (comment)
When the user did no select a Windows target version manually, stop
explicitly targeting Windows XP, and instead use the toolchain default.
This may pose an issue with old toolchains defaulting to pre-XP
targets. In such case you must manually target Windows XP via:
-DCURL_TARGET_WINDOWS_VERSION=0x0501
or
-DCMAKE_C_FLAGS=-D_WIN32_WINNT=0x0501
Closes #xxxx