Skip to content

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Jan 12, 2025

Before this patch the CURL_CHECK_WIN32_LARGEFILE feature check was
running an AC_COMPILE snippet that always succeeded. (except for
Windows CE, which isn't supported in other parts of ./configure yet.)

The only Windows toolchain autotools supports is mingw. Of them, curl
only supports mingw-w64. All mingw-w64 versions support large files.
This allows to drop the check and assume it supported on Windows. To not
lose Windows CE support, rework that too, without using AC_COMPILE.

Drop the feature check altogether for non-Windows targets.

Ref: #15968 (comment)
Follow-up to 7eb4ddb #15968


w/o whitespace: https://github.com/curl/curl/pull/15971/files?w=1

Before this patch the feature check was running an `AC_COMPILE` check
that always succeeded.

The only Windows toolchain autotools supports is mingw. Of these, curl
only supports mingw-w64. All mingw-w64 versions support large files.
This allows to drop the check and assume it supported.

Drop the feature check altogether for non-native Windows targets.

Ref: curl#15968 (comment)
esac
case "$curl_win32_has_largefile" in
yes)
if test x"$enable_largefile" = 'xno'; then
Copy link
Member

Choose a reason for hiding this comment

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

is this check necessary, like what would possibly set enable_largefile false at this point and should we allow it for windows? i don't see where it's defined but does it have something to do with --disable-largefile?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's a ./configure option:

  --disable-largefile     omit support for large files

I think it's built-in, and also applies to other platforms, but I didn't dig into it beyond a Windows test build.

vszakats added a commit to vszakats/curl that referenced this pull request Jan 12, 2025
Before this patch the `CURL_CHECK_WIN32_LARGEFILE` feature check was
running an `AC_COMPILE` snippet that always succeeded. (except for
Windows CE, which isn't supported in other parts of `./configure` yet.)

The only Windows toolchain autotools supports is mingw. Of them, curl
only supports mingw-w64. All mingw-w64 versions support large files.
This allows to drop the check and assume it supported on Windows. To not
lose Windows CE support, rework that too, without using `AC_COMPILE`.

Drop the feature check altogether for non-Windows targets.

Ref: curl#15968 (comment)
Follow-up to 7eb4ddb curl#15968

Closes curl#15971
@vszakats
Copy link
Member Author

After recent few tidy ups, there may be a chance to merge USE_WIN32_LARGE_FILES
with SIZEOF_OFF_T (>4) into a CURL_LARGE_FILE macro that applies to all platforms.

@vszakats vszakats closed this in a118452 Jan 13, 2025
@vszakats vszakats deleted the am-large-file branch January 13, 2025 01:46
vszakats added a commit that referenced this pull request Jan 17, 2025
(Did not cause issues in this particular case.)

Follow-up to a118452 #15971
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
Before this patch the `CURL_CHECK_WIN32_LARGEFILE` feature check was
running an `AC_COMPILE` snippet that always succeeded. (except for
Windows CE, which isn't supported in other parts of `./configure` yet.)

The only Windows toolchain autotools supports is mingw. Of them, curl
only supports mingw-w64. All mingw-w64 versions support large files.
This allows to drop the check and assume it supported on Windows. To not
lose Windows CE support, rework that too, without using `AC_COMPILE`.

Drop the feature check altogether for non-Windows targets.

Ref: curl#15968 (comment)
Follow-up to 7eb4ddb curl#15968

Closes curl#15971
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
(Did not cause issues in this particular case.)

Follow-up to a118452 curl#15971
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.

2 participants