Skip to content

configure: fix WinIDN builds targeting old Windows#14680

Closed
vszakats wants to merge 6 commits intocurl:masterfrom
vszakats:idn-wnd-fixes
Closed

configure: fix WinIDN builds targeting old Windows#14680
vszakats wants to merge 6 commits intocurl:masterfrom
vszakats:idn-wnd-fixes

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Aug 25, 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 configure: set classic mingw minimum OS version to XP #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

@vszakats vszakats added Windows Windows-specific name lookup DNS and related tech labels Aug 25, 2024
@vszakats vszakats marked this pull request as draft August 25, 2024 17:33
@github-actions github-actions bot added the CI Continuous Integration label Aug 25, 2024
@vszakats vszakats changed the title idn: fix WinIDN builds targeting old Windows configure: fix WinIDN builds targeting old Windows Aug 25, 2024
@vszakats vszakats added the build label Aug 25, 2024
@vszakats vszakats marked this pull request as ready for review August 25, 2024 17:56
All mingw-w64 versions declare these IDN function, but some mingw-envs
set the default _WIN32_WINNT value conservatively, e.g. to 0x0502. In
such case idn.c was assuming an old SDK env, assumed these declarations
missing and declared them. This caused duplicate declaration warnings.
Fix it by always skipping local declarations for mingw builds. (Classic
mingw may fail on that, but curl no longer supports classic mingw)

I'm not sure why this wasn't happening with cmake builds in the same
build env.

Seen on Ubuntu mingw-w64 cross-builds.

```
../../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);
      |                              ^~~~~~~~~~
../../lib/idn.c:159:23: error: redundant redeclaration of ‘IdnToUnicode’ [-Werror=redundant-decls]
  159 | WINBASEAPI int WINAPI IdnToUnicode(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:1077:30: note: previous declaration of ‘IdnToUnicode’ was here
 1077 |   WINNORMALIZEAPI int WINAPI IdnToUnicode (DWORD dwFlags, LPCWSTR lpASCIICharStr, int cchASCIIChar, LPWSTR lpUnicodeCharStr, int cchUnicodeChar);
      |                              ^~~~~~~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/10548869067/job/29223257811?pr=14680#step:7:90
This reverts commit 6c8068dfdecbef0390eac1c77e06b8db4e0fca5c.

Not good, this time with CMake, missing the original declarations:

They are guarded with in winnls.h:
```
  #if WINVER >= 0x0600
```

```
/home/runner/work/curl/curl/lib/idn.c: In function ‘win32_idn_to_ascii’:
/home/runner/work/curl/curl/lib/idn.c:174:17: error: implicit declaration of function ‘IdnToAscii’; did you mean ‘ToAscii’? [-Wimplicit-function-declaration]
  174 |     int chars = IdnToAscii(0, in_w, (int)(wcslen(in_w) + 1), punycode,
      |                 ^~~~~~~~~~
      |                 ToAscii
/home/runner/work/curl/curl/lib/idn.c:174:17: error: nested extern declaration of ‘IdnToAscii’ [-Werror=nested-externs]
/home/runner/work/curl/curl/lib/idn.c: In function ‘win32_ascii_to_idn’:
/home/runner/work/curl/curl/lib/idn.c:204:17: error: implicit declaration of function ‘IdnToUnicode’; did you mean ‘ToUnicode’? [-Wimplicit-function-declaration]
  204 |     int chars = IdnToUnicode(0, in_w, (int)(wcslen(in_w) + 1), idn,
      |                 ^~~~~~~~~~~~
      |                 ToUnicode
/home/runner/work/curl/curl/lib/idn.c:204:17: error: nested extern declaration of ‘IdnToUnicode’ [-Werror=nested-externs]
```
Ref: https://github.com/curl/curl/actions/runs/10545946413/job/29216819270?pr=13947#step:11:76
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 vszakats closed this in 2625360 Aug 27, 2024
@vszakats vszakats deleted the idn-wnd-fixes branch August 27, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build CI Continuous Integration name lookup DNS and related tech Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

2 participants