Skip to content

build: enable AppleIDN / WinIDN by default#13947

Closed
vszakats wants to merge 16 commits intocurl:masterfrom
vszakats:winidn-default
Closed

build: enable AppleIDN / WinIDN by default#13947
vszakats wants to merge 16 commits intocurl:masterfrom
vszakats:winidn-default

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Jun 14, 2024

Enable and use WinIDN and AppleIDN by default on these platforms,
if they can be used.

Compatibility warning: This change can make default builds incompatible
with Windows XP by default. WinIDN requires Normaliz.dll. Windows XP
isn't shipping with it and the necessary extra package is no longer
offered by Microsoft. Thus, for Windows XP compatibility, we recommend
building with -DUSE_WIN32_IDN=OFF.

Discussion: #13565
Closes #13947


Cleaner diff without whitespace changes: https://github.com/curl/curl/pull/13947/files?w=1

There are a few ways to resolve compatibility with existing builds
and also to handle interactions with the USE_LIBIDN2 option.
This is an initial attempt.

TODO:

  • rethink defaulting logic to honor an explicitly selected libidn2?
  • is WinIDN useful without Unicode? [YES] Enable it only in Unicode builds by default? (Also needs reshuffling options in Azure and GHA/Windows)
    Downside: it makes the defaulting logic difficult to predict.
  • rebase on configure: fix WinIDN builds targeting old Windows #14680. (delete explicit WinIDN options added to GHA/Windows/cross.)
  • rebase on GHA/macos: drop options no longer necessary #14693.
  • add jobs that disable AppleIDN/WinIDN for both autotools and cmake.
  • configure: enable WinIDN by default for HAVE_WIN32_WINNT >= 0x0600, as cmake.
  • review and fixup AppleIDN/WinIDN and libidn2 interactions in autotools and cmake.
  • tackle AppleIDN.
  • cmake: detect WinXP target using HAVE_WIN32_WINNT and adjust default USE_WIN32_IDN setting accordingly?
  • review and update CI jobs where/if necessary.
  • fix autotools logic to only make WinIDN the default when targeting native Windows.
  • figure out how to make IDN tests work on Windows. [SKIP]
  • split off VS2008 log dump on failure in AppVeyor into separate PR. → appveyor: dump build logs on failure in VS2008 jobs #13957

@vszakats vszakats added build Windows Windows-specific name lookup DNS and related tech labels Jun 14, 2024
@vszakats vszakats marked this pull request as draft June 14, 2024 02:31
@vszakats
Copy link
Member Author

Tests are unprepared to handle IDN with Windows, both UNICODE and non-UNICODE modes (but differently).

@github-actions github-actions bot added the CI Continuous Integration label Jun 14, 2024
@vszakats
Copy link
Member Author

vszakats commented Jun 14, 2024

Unexpected fallout with VS2008. Vista target, Normaliz.lib specified, yet the IDN functions are not found:

6>unity_0.obj : error LNK2019: unresolved external symbol __imp__IdnToAscii@20 referenced in function _win32_idn_to_ascii
6>unity_0.obj : error LNK2019: unresolved external symbol __imp__IdnToUnicode@20 referenced in function _win32_ascii_to_idn
6>C:\projects\curl\_bld\lib\libcurl.dll : fatal error LNK1120: 2 unresolved externals

Anybody with a solution for the above issue?

I still couldn't figure out how to enable verbose logging for the VS2008 jobs. [RESOLVED]

@vszakats
Copy link
Member Author

The VS2008 issue was reported here earlier: #1863

This trace message says that normaliz is added to the list of libs:

-- ws2_32;bcrypt;wldap32;normaliz

Ref: https://ci.appveyor.com/project/curlorg/curl/builds/50021801/job/xpd1d5cor294gp3m#L42

It feels like something very obvious (or something very cryptic). Seeing the actual linker command would tell more of what's happening.

@vszakats vszakats force-pushed the winidn-default branch 2 times, most recently from a7e55a2 to 7463635 Compare June 15, 2024 19:08
@vszakats
Copy link
Member Author

vszakats commented Jun 17, 2024

Backing off of this. It looks nearly hopeless to make IDN tests pass on Windows (without going crazies that is.) It needs passing Unicode text via the command-line for starters. Also the unexplained VC2008 fallout [FIXED]. Then autotools.

I might cherry pick the part moving WINVER detection up to the top. [DONE → 919394e #14450]

Feel free to pick up and continue.

@vszakats vszakats closed this Jun 17, 2024
@vszakats vszakats deleted the winidn-default branch June 17, 2024 22:34
@vszakats vszakats restored the winidn-default branch August 7, 2024 13:25
@vszakats vszakats reopened this Aug 7, 2024
@vszakats vszakats changed the title [WIP] build: enable WinIDN by default [WIP] build: enable AppleIDN/WinIDN by default Aug 7, 2024
@vszakats vszakats force-pushed the winidn-default branch 3 times, most recently from 7cd2aab to 3c53042 Compare August 9, 2024 16:53
@vszakats vszakats marked this pull request as ready for review August 9, 2024 17:07
@vszakats vszakats changed the title [WIP] build: enable AppleIDN/WinIDN by default build: enable AppleIDN/WinIDN by default Aug 9, 2024
@vszakats vszakats changed the title build: enable AppleIDN/WinIDN by default build: enable AppleIDN / WinIDN by default Aug 9, 2024
@vszakats vszakats added the appleOS specific to an Apple operating system label Aug 9, 2024
@vszakats vszakats added the feature-window A merge of this requires an open feature window label Aug 10, 2024
@vszakats vszakats force-pushed the winidn-default branch 2 times, most recently from 3b1c273 to efe750d Compare August 12, 2024 13:02
@vszakats vszakats added feature-window A merge of this requires an open feature window and removed feature-window A merge of this requires an open feature window labels Aug 12, 2024
@vszakats vszakats force-pushed the winidn-default branch 4 times, most recently from 65d0620 to 4ff43bc Compare October 3, 2024 14:37
@vszakats vszakats force-pushed the winidn-default branch 3 times, most recently from 35bbf41 to c558a30 Compare October 17, 2024 14:39
@vszakats vszakats force-pushed the winidn-default branch 2 times, most recently from 3b9e056 to ee6474d Compare November 26, 2024 16:39
cmake: do not enable winidn in msvc 2008 and older

with normaliz.lib linked, it fails to find the necessary symbols:
```
C:\projects\curl\_bld\lib\Release\libcurl_imp.exp
>unity_0.obj : error LNK2019: unresolved external symbol __imp__IdnToAscii@20 referenced in function _win32_idn_to_ascii
>unity_0.obj : error LNK2019: unresolved external symbol __imp__IdnToUnicode@20 referenced in function _win32_ascii_to_idn
>C:\projects\curl\_bld\lib\libcurl.dll : fatal error LNK1120: 2 unresolved externals
```
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/50374075

"VS2008 default SDK doesn't have IdnToAscii or IdnToUnicode in normaliz.lib"

Ref: curl#1863
Ref: https://curl.se/mail/lib-2014-12/0084.html
@vszakats vszakats closed this Dec 1, 2024
@vszakats vszakats deleted the winidn-default branch December 1, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

appleOS specific to an Apple operating system build CI Continuous Integration feature-window A merge of this requires an open feature window name lookup DNS and related tech Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

2 participants