Skip to content

Conversation

kou
Copy link

@kou kou commented Jun 7, 2025

Issue #, and/or reason for changes (REQUIRED):

#1207

Description of changes:

We should use _WIN32 not _MSC_VER to support both of Visual C++ and MinGW.

This was missed in #801 and #822 .

MSYS2 also includes this change:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-aws-c-common/001-fix-build-on-mingw-aarch64.patch

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This was missed in awslabs#801
and awslabs#822 .

MSYS2 also includes this change:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-aws-c-common/001-fix-build-on-mingw-aarch64.patch

Error message without this:

https://github.com/ursacomputing/crossbow/actions/runs/15502494926/job/43652596580#step:7:5884

     cd /D/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-build && /D/a/crossbow/crossbow/sccache/sccache.exe /C/rtools40/mingw32/bin/cc.exe -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HAVE_SSE4_2 -DARROW_WITH_TIMING_TESTS -DAWS_AFFINITY_METHOD=AWS_AFFINITY_METHOD_NONE -DCJSON_HIDE_SYMBOLS -DINTEL_NO_ITTNOTIFY_API -DPSAPI_VERSION=1 -DWINDOWS_KERNEL_LIB=kernel32 -D_CRT_SECURE_NO_WARNINGS @CMakeFiles/aws-c-common.dir/includes_C.rsp -O3 -DNDEBUG -O2 -ftree-vectorize  -std=gnu99 -fvisibility=hidden -Wall -Wstrict-prototypes -pedantic -Wno-long-long -fPIC -D_FILE_OFFSET_BITS=64 -MD -MT _deps/aws-c-common-build/CMakeFiles/aws-c-common.dir/source/allocator.c.obj -MF CMakeFiles/aws-c-common.dir/source/allocator.c.obj.d -o CMakeFiles/aws-c-common.dir/source/allocator.c.obj -c /D/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/source/allocator.c
    In file included from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/byte_order.h:71,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/byte_buf.h:9,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/thread.h:8,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/logging.h:11,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/source/allocator.c:8:
    D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/byte_order.inl: In function 'aws_hton64':
    D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/byte_order.inl:47:23: warning: implicit declaration of function 'htonl'; did you mean 'wtoll'? [-Wimplicit-function-declaration]
         return ((uint64_t)htonl(low)) << 32 | htonl(high);
                           ^~~~~
                           wtoll
    In file included from C:/rtools40/mingw32/i686-w64-mingw32/include/windows.h:92,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/source/allocator.c:15:
    C:/rtools40/mingw32/i686-w64-mingw32/include/winsock.h: At top level:
    C:/rtools40/mingw32/i686-w64-mingw32/include/winsock.h:286:37: error: conflicting types for 'htonl'
       WINSOCK_API_LINKAGE u_long WSAAPI htonl(u_long hostlong);
                                         ^~~~~
    In file included from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/byte_order.h:71,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/byte_buf.h:9,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/thread.h:8,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/logging.h:11,
                     from D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/source/allocator.c:8:
    D:/a/crossbow/crossbow/src/build-i686-cpp/_deps/aws-c-common-src/include/aws/common/byte_order.inl:47:23: note: previous implicit declaration of 'htonl' was here
         return ((uint64_t)htonl(low)) << 32 | htonl(high);
                           ^~~~~
@kou
Copy link
Author

kou commented Aug 8, 2025

I close this in favor of #1210 .

@kou kou closed this Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant