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

windows: drop redundant USE_WIN32_SMALL_FILES macro #15968

Closed
wants to merge 12 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jan 11, 2025

In effect it meant _WIN32 && !USE_WIN32_LARGE_FILES.
Replace it with these macros.

Also:

  • configure: delete tautological check for small file support.
  • configure: delete stray _MSC_VER reference. autotools does not
    support MSVC.
  • drop tautological checks for WinCE in config-win32*.h when setting
    USE_WIN32_LARGE_FILES.
  • merge related PP logic.
  • prefer #ifdef, fix whitespace.

Suggested-by: Marcel Raad
Report: #15952 (comment)


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

@vszakats vszakats added the Windows Windows-specific label Jan 11, 2025
@vszakats vszakats changed the title windows: drop USE_WIN32_SMALL_FILES macro windows: drop redundant USE_WIN32_SMALL_FILES macro Jan 11, 2025
That said, any __MINGW32__ logic is unused in config-win32.h since the
deletion of the `Makefile.mk` build method. It'd probably make sense to
delete it.

The users of this file now is winbuild and Visual Studio Project file
builds.
@@ -1385,7 +1385,7 @@ AC_DEFUN([CURL_CHECK_WIN32_LARGEFILE], [
AC_COMPILE_IFELSE([
AC_LANG_PROGRAM([[
]],[[
#if !defined(_WIN32_WCE) && (defined(__MINGW32__) || defined(_MSC_VER))
#if !defined(_WIN32_WCE) && defined(__MINGW32__)
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't we want to default to the large file API instead of the small? Defaulting to large breaks in an obvious way for the user if there is no support for large and there is some build mistake.

Copy link
Member Author

@vszakats vszakats Jan 11, 2025

Choose a reason for hiding this comment

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

Makes sense. I'd be more comfortable touching it in a separate PR.

I think an external check isn't needed here, nor running this code on
non-Windows. WinCE also was never supported with ./configure, so
this could be reduced to permanently enabling large files for native
Windows (all supported mingw builds support it, and configure only
supports Windows through mingw).

(If CeGCC support lands in ./configure, we need to disable it there
with a $host_os check.)

@vszakats vszakats closed this in 7eb4ddb Jan 11, 2025
@vszakats vszakats deleted the w-small-large-files branch January 11, 2025 23:35
vszakats added a commit to vszakats/curl that referenced this pull request Jan 12, 2025
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)
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 added a commit that referenced this pull request Jan 13, 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

Closes #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