-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
build: improve OS string in CMake and config-win32.h
#9117
Conversation
Autotools builds fill the curl "OS string" with the platform triplet. This value appears in the curl version info: `curl 7.84.0 (os_string)` This patch adds the ability to do the same with CMake and non-autotools builds, allowing to configure consistent version strings (and thus reproducible binaries) across build systems. Add the `CURL_OS` macro and CMake configuration variable to allow passing a custom "OS string", e.g.: with CMake: `cmake [...] -DCURL_OS=os_string` with non-autotools builds, via C flag: `-DCURL_OS="os_string"` `os_string` is typically a triplet, e.g.: `x86_64-pc-linux-gnu` Before this patch CMake uncoditionally filled this value with `CMAKE_SYSTEM_NAME`, which is a crude platform value, e.g. for Windows it is always "Windows", without CPU or other information. Now it allows to override this via its `CURL_OS` configuration option. For non-autotools builds, it was automatically set to a value in `lib/config-*.h` headers. Now they allow to override these defaults when the `CURL_OS` macro is set. (For now this is implemented for `config-win32.h` only.) This patch supersedes the earlier, partial, CMake-only solution: 435f395, thus retiring the `CURL_OS_SUFFIX` CMake option. Closes #xxxx
Do you see anyone besides yourself manually setting the OS string? It seems to me a better solution would be improve cmake's detected OS string. |
Anybody who wants to create reproducible builds across multiple build system will face these, but yes, it might be only me doing that. Not sure how to improve this in CMake, but even if doable, I suspect any autodetection would be inherently different from |
I would also rather have the CMake build choose the right system triplet on its
own. This is entirely a automatible process that doesn't need to be have a
manual override. It wouldn't surprise me if there were a CMake macro available
to do this already, bu worst-case, you could just include config.guess in the
tarball and call that to get the same triplet as autoconf.
|
Actually autotools isn't even guessing the triplet in my (or any cross-build) case, I'm passing it to The same can be done with CMake, where the triplet is passed (for cross-builds at least) via set(OPERATING_SYSTEM "${CMAKE_SYSTEM_NAME}")
if(CMAKE_C_COMPILER_TARGET)
set(OS "\"${CMAKE_C_COMPILER_TARGET}\"")
else()
set(OS "\"${CMAKE_SYSTEM_NAME}\"")
endif() This would change the default behaviour for CMake builds where this value is passed (unless we put it behind an extra switch). |
CURL_OS
option to set the OS stringconfig-win32.h
I don't understand how this makes it reproducible in Windows pregenerated config. If the triplet is the same it's already the same string. Similar for cmake, if anything "Windows" is always "Windows"? Not that it should be that, but you can see what I mean. IMO CURL_OS_SUFFIX is a workaround for the actual problem which is lack of cmake detailed target recognition |
@jay: In case of cross-builds there no automatic target recognition (there cannot be), it's an input provided manually to the build systems by the builder. Currently this input is only used to form the OS string with autotools. Makefile.m32 uses some hard-coded value, and CMake just puts 'Windows' there. It means that even though I do build for the exact same target with the same triplet and with precisely the same build options, the three build systems generate objects that differ only in the object compiled from In general, CMake's solution to put 'Windows' or other crude OS string there seems wrong in general, especially when someone clearly passed a triplet to it via The (updated) proposed solution makes CMake use the above value, if present. It also lets the user override |
Ok for most recent cmake change. Are you saying you need to override OS in config-win32? What is wrong with the OS logic already there, is it not accurate enough? edit: Are you saying you want the same OS string for both build systems? |
@jay: That's correct, I'd like to see the same OS string for CMake, Makefile.m32 (and autotools) builds. |
Apply upstream patch [1] [2], which unconditionally sets the OS string to the cross-build target value with CMake. For Makefile.m32, use the new option to override the OS string to the triplet with an extra build option. The internal switch `CW_DEV_CROSSMAKE_REPR` no longer has an effect on this. And, curl-for-win could get rid of two local (disabled-by-default) patches. [1] curl/curl@ca73991 [2] curl/curl#9117
This patch makes CMake fill the "OS string" with the value
of
CMAKE_C_COMPILER_TARGET
, if passed. This typicallycontains a triplet, the same that is passed to
./configure
via--host=
.For non-CMake, non-autotools, Windows builds, this patch
adds the ability to override the default
OS
value inlib/config-win32.h
.With these its possible to get the same OS string across the
three build systems.
This patch supersedes the earlier, partial, CMake-only solution:
435f395, thus retiring the
CURL_OS_SUFFIX
CMake option.Closes #xxxx