configure.ac: error if --with-winidn and target OS too old#12684
Closed
jay wants to merge 1 commit intocurl:masterfrom
Closed
configure.ac: error if --with-winidn and target OS too old#12684jay wants to merge 1 commit intocurl:masterfrom
jay wants to merge 1 commit intocurl:masterfrom
Conversation
- If the minimum Windows target OS is pre-Vista (_WIN32_WINNT < 0x600) then error. - Stop comparing against WINVER when making the minimum OS determinations (ie check only _WIN32_WINNT instead of both since that's the one that matters). Prior to this change, if --with-winidn was used in such a case configure would change the minimum target OS to Vista (0x600) so the build would compile. That was done primarily as a workaround for original MinGW which we no longer support. Bug: curl#12606 (comment) Reported-by: Viktor Szakats Closes #xxxxx
76b8d0b to
e02b27e
Compare
vszakats
approved these changes
Jan 12, 2024
Member
|
Thinking about this, if we require Vista, we'd need some additional tweaks for consistency. In Anyway, if we require Vista for IDN, the diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3a6ae3d42..332c08186 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -841,13 +841,6 @@ else()
set(HAVE_LIBIDN2 OFF)
endif()
-if(WIN32)
- option(USE_WIN32_IDN "Use WinIDN for IDN support" OFF)
- if(USE_WIN32_IDN)
- list(APPEND CURL_LIBS "normaliz")
- endif()
-endif()
-
#libpsl
option(CURL_USE_LIBPSL "Use libPSL" ON)
mark_as_advanced(CURL_USE_LIBPSL)
@@ -1085,6 +1078,14 @@ if(WIN32)
unset(HAVE_INET_PTON CACHE)
endif()
endif()
+
+ option(USE_WIN32_IDN "Use WinIDN for IDN support" OFF)
+ if(USE_WIN32_IDN)
+ if(HAVE_WIN32_WINNT AND HAVE_WIN32_WINNT STRLESS "0x600")
+ message(FATAL_ERROR "WinIDN requires building for Windows Vista or newer.")
+ endif()
+ list(APPEND CURL_LIBS "normaliz")
+ endif()
endif()
check_include_file_concat("sys/filio.h" HAVE_SYS_FILIO_H)
diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 551243e27..794d42058 100644
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -623,6 +623,14 @@
/* ---------------------------------------------------------------- */
+#if !defined(_WIN32) && defined(USE_WIN32_IDN)
+#undef USE_WIN32_IDN
+#endif
+
+#if defined(USE_WIN32_IDN) && (_WIN32_WINNT < 0x0600)
+#error "WinIDN requires building for Windows Vista or newer."
+#endif
+
#if defined(HAVE_LIBIDN2) && defined(HAVE_IDN2_H) && !defined(USE_WIN32_IDN)
/* The lib and header are present */
#define USE_LIBIDN2
diff --git a/lib/idn.c b/lib/idn.c
index 81a177f8c..c4167db18 100644
--- a/lib/idn.c
+++ b/lib/idn.c
@@ -51,20 +51,6 @@
#include "memdebug.h"
#ifdef USE_WIN32_IDN
-/* using Windows kernel32 and normaliz libraries. */
-
-#if !defined(_WIN32_WINNT) || _WIN32_WINNT < 0x600
-WINBASEAPI int WINAPI IdnToAscii(DWORD dwFlags,
- const WCHAR *lpUnicodeCharStr,
- int cchUnicodeChar,
- WCHAR *lpASCIICharStr,
- int cchASCIIChar);
-WINBASEAPI int WINAPI IdnToUnicode(DWORD dwFlags,
- const WCHAR *lpASCIICharStr,
- int cchASCIIChar,
- WCHAR *lpUnicodeCharStr,
- int cchUnicodeChar);
-#endif
#define IDN_MAX_LENGTH 255Edits:
If want to keep supporting WinIDN for XP, maybe the autotools detection could probe building with |
Member
|
Stalled. |
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Aug 26, 2024
Ref: curl#12684 Ref: curl#12606 (comment) Ref: curl@37f1c21
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Aug 26, 2024
Originall added in 37f1c21 curl#7581 for classic mingw. curl no longer supports building with classic mingw. Ref: curl#12684 Ref: curl#12606 (comment)
vszakats
added a commit
that referenced
this pull request
Aug 27, 2024
1. GHA/windows: enable WinIDN in Linux cross-builds.
(to reveal the issue in CI.)
2. fix compiler warning when building with mingw-w64 supporting
WinIDN, while targeting pre-Vista Windows, with a `WINVER` set to
target Vista or newer. (Such was Ubuntu's mingw-w64 with the
classic-mingw-specific trick in point 3 of this PR.)
```
../../lib/idn.c:154:23: error: redundant redeclaration of ‘IdnToAscii’ [-Werror=redundant-decls]
154 | WINBASEAPI int WINAPI IdnToAscii(DWORD dwFlags,
| ^~~~~~~~~~
In file included from /usr/share/mingw-w64/include/windows.h:73,
from /usr/share/mingw-w64/include/winsock2.h:23,
from ../../lib/setup-win32.h:91,
from ../../lib/curl_setup.h:308,
from ../../lib/idn.c:29:
/usr/share/mingw-w64/include/winnls.h:1075:30: note: previous declaration of ‘IdnToAscii’ was here
1075 | WINNORMALIZEAPI int WINAPI IdnToAscii (DWORD dwFlags, LPCWSTR lpUnicodeCharStr, int cchUnicodeChar, LPWSTR lpASCIICharStr, int cchASCIIChar);
| ^~~~~~~~~~
[...same for IdnToUnicode...]
```
Ref: https://github.com/curl/curl/actions/runs/10542832783/job/29210098553#step:7:89
3. drop `WINVER` override for classic-mingw. curl no longer supports
building with classic-mingw.
Reverts 37f1c21 #7581
4. sync `if IdnToUnicode can be linked` detection snippet with the live
code in `lib/idn.c`. It fixes detection for the scenario in point 2.
5. delete unused `WINIDN_DIR` variable.
Bug: #12606 (comment)
Previous abandoned attempt: #12684
Reviewed-by: Jay Satiro
Closes #14680
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If the minimum Windows target OS is pre-Vista (_WIN32_WINNT < 0x600) then error.
Stop comparing against WINVER when making the minimum OS determinations (ie check only _WIN32_WINNT instead of both since that's the one that matters).
Prior to this change, if --with-winidn was used in such a case configure would change the minimum target OS to Vista (0x600) so the build would compile. That was done primarily as a workaround for original MinGW which we no longer support.
Bug: #12606 (comment)
Reported-by: Viktor Szakats
Closes #xxxxx