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: enable more detection on Windows #9687

Closed
wants to merge 2 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Oct 10, 2022

Enable HAVE_UNISTD_H, HAVE_STRTOK_R and HAVE_STRCASECMP detection on Windows, instead of having predefined values.

With these features detected correctly, CMake Windows builds get closer to the autotools and config-win32.h ones.

This also fixes detecting HAVE_FTRUNCATE correctly, which required unistd.h.

Fixing ftruncate() in turn causes a build warning/error with legacy MinGW/MSYS1 due to an offset type size mismatch. This env misses to detect HAVE_FILE_OFFSET_BITS, which may be a reason. This patch force-disables HAVE_FTRUNCATE for this platform.

Reviewed-by: Daniel Stenberg

Closes #9687

@vszakats vszakats added build cmake Windows Windows-specific labels Oct 10, 2022
bagder
bagder approved these changes Oct 10, 2022
@vszakats vszakats removed the build label Oct 10, 2022
@vszakats
Copy link
Member Author

vszakats commented Oct 10, 2022

The good news is that this also fixed 1 to correctly detect HAVE_FTRUNCATE with modern MinGW.

The bad news is that this in turn breaks legacy MinGW/MSYS1 builds because of the enabled ftruncate() 2:

[ 86%] Building C object src/CMakeFiles/curl.dir/tool_operhlp.c.obj
C:/projects/curl/src/tool_operate.c: In function 'post_per_transfer':
C:/projects/curl/src/tool_operate.c:576:48: error: conversion from 'curl_off_t' {aka 'long long int'} to 'off_t' {aka 'long int'} may change value [-Werror=conversion]
  576 |         if(ftruncate(fileno(outs->stream), outs->init)) {
      |                                            ~~~~^~~~~~
[ 86%] Building C object src/CMakeFiles/curl.dir/tool_panykey.c.obj
cc1.exe: all warnings being treated as errors
make[2]: *** [src/CMakeFiles/curl.dir/build.make:456: src/CMakeFiles/curl.dir/tool_operate.c.obj] Error 1
[ 86%] Building C object src/CMakeFiles/curl.dir/tool_paramhlp.c.obj
[ 86%] Building C object src/CMakeFiles/curl.dir/tool_parsecfg.c.obj

Is there someone with access to this old env with an idea how to fix it?

Footnotes

  1. Detection of HAVE_UNISTD_H is required to include unistd.h, which is required to successfully detect ftruncate().

  2. https://ci.appveyor.com/project/curlorg/curl/builds/45020994/job/2udxojuyoxuuoi3n

vszakats added 2 commits Oct 10, 2022
Enable `HAVE_UNISTD_H`, `HAVE_STRTOK_R` and `HAVE_STRCASECMP` detection
on Windows, instead of having predefined values.

With these features detected correctly, CMake Windows builds get closer
to the autotools and `config-win32.h` ones.

This also fixes detecting `HAVE_FTRUNCATE` correctly, which required
`unistd.h`.

Fixing `ftruncate()` in turn causes a build warning/error with legacy
MinGW/MSYS1 due to an offset type size mismatch. This env misses to
detect `HAVE_FILE_OFFSET_BITS`, which may be a reason. This patch
force-disables `HAVE_FTRUNCATE` for this platform.
@vszakats
Copy link
Member Author

vszakats commented Oct 10, 2022

Updated to keep ftruncate() disabled for old MinGW.

@vszakats vszakats closed this in 474a947 Oct 11, 2022
@vszakats vszakats deleted the cmakewin1 branch Oct 11, 2022
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 11, 2022
Upcoming curl release received fixes to correctly detect these build
flags, so drop manual settings for them:

- `HAVE_UNISTD_H` and with it: `HAVE_FTRUNCATE`
- `HAVE_STRTOK_R`
- `HAVE_STRCASECMP`
- `HAVE_INET_NTOP`

Ref: curl/curl#9687
Ref: curl/curl#9689
obonaventure pushed a commit to mptcp-apps/curl that referenced this pull request Oct 12, 2022
Enable `HAVE_UNISTD_H`, `HAVE_STRTOK_R` and `HAVE_STRCASECMP` detection
on Windows, instead of having predefined values.

With these features detected correctly, CMake Windows builds get closer
to the autotools and `config-win32.h` ones.

This also fixes detecting `HAVE_FTRUNCATE` correctly, which required
`unistd.h`.

Fixing `ftruncate()` in turn causes a build warning/error with legacy
MinGW/MSYS1 due to an offset type size mismatch. This env misses to
detect `HAVE_FILE_OFFSET_BITS`, which may be a reason. This patch
force-disables `HAVE_FTRUNCATE` for this platform.

Reviewed-by: Daniel Stenberg

Closes curl#9687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Windows Windows-specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants