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

curl_setup: disallow Windows IPv6 builds missing getaddrinfo #12221

Closed
wants to merge 1 commit into from

Conversation

jay
Copy link
Member

@jay jay commented Oct 28, 2023

  • On Windows if IPv6 is enabled but getaddrinfo is missing then #error the build.

curl can be built with IPv6 support (ENABLE_IPV6) but without the ability to resolve hosts to IPv6 addresses (HAVE_GETADDRINFO). On Windows this is highly unlikely and should be considered a bad build configuration.

Such a bad configuration has already given us a bug that was hard to diagnose. See #12134 and #12136 for discussion.

Ref: #12134
Ref: #12136

Closes #xxxx

- On Windows if IPv6 is enabled but getaddrinfo is missing then #error
  the build.

curl can be built with IPv6 support (ENABLE_IPV6) but without the
ability to resolve hosts to IPv6 addresses (HAVE_GETADDRINFO). On
Windows this is highly unlikely and should be considered a bad build
configuration.

Such a bad configuration has already given us a bug that was hard to
diagnose. See curl#12134 and curl#12136 for discussion.

Ref: curl#12134
Ref: curl#12136

Closes #xxxx
@jay jay added build Windows Windows-specific labels Oct 28, 2023
Copy link
Member

@MarcelRaad MarcelRaad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, both IPv6 and getaddrinfo were added in Windows XP. Makes sense to me!

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sensible lesson to draw from this past week

@vszakats
Copy link
Member

vszakats commented Oct 28, 2023

Related, we might also consider:

  1. requiring Windows XP as a minimum.
  2. dropping Windows support from Makefile.mk.
    curl-for-win was likely the sole user of this and I have been its
    only maintainer for many years. It's not CI tested and even though
    simple enough, it's an extra path to maintain. curl-for-win
    is switching to CMake now that it surpassed Makefile.mk in performance.
    That'd leave Makefile.mk for MS-DOS and Amiga.

vszakats added a commit to vszakats/curl that referenced this pull request Oct 28, 2023
After this patch we assume availability of `getaddrinfo` and
`freeaddrinfo`, first introduced in Windows XP.

TODO: assume these also in autotools.

Ref: curl#12221 (comment)
Closes #xxxxx
vszakats added a commit to vszakats/curl that referenced this pull request Oct 28, 2023
After this patch we assume availability of `getaddrinfo` and
`freeaddrinfo`, first introduced in Windows XP.

TODO: assume these also in autotools.

Ref: curl#12221 (comment)
Closes #xxxxx
@jay jay closed this in 904ae12 Oct 29, 2023
@jay jay deleted the win32_ipv6_sanity_check branch October 29, 2023 07:48
vszakats added a commit that referenced this pull request Oct 30, 2023
After this patch we assume availability of `getaddrinfo` and
`freeaddrinfo`, first introduced in Windows XP. Meaning curl
now requires building for Windows XP as a minimum.

TODO: assume these also in autotools.

Ref: #12221 (comment)
Closes #12225
vszakats added a commit that referenced this pull request Dec 16, 2023
And DLL-support with it. This leaves `Makefile.mk` for MS-DOS and Amiga.

We recommend CMake instead. With unity mode it's much faster, and about
the same without.

Ref: #12221 (comment)
Reviewed-by: Daniel Stenberg
Closes #12224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants