Skip to content

Support compiling on MSYS2 #814

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 19, 2024
Merged

Support compiling on MSYS2 #814

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5460cef
Support compiling on MinGW
qmfrederik Feb 15, 2024
e5b7b11
PR feedback
qmfrederik Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,18 @@ if("${CMAKE_C_SIMULATE_ID}" STREQUAL "MSVC")
endif()

if(CMAKE_SYSTEM_NAME STREQUAL Windows)
include(DispatchWindowsSupport)
dispatch_windows_arch_spelling(${CMAKE_SYSTEM_PROCESSOR} DISPATCH_MSVC_ARCH)
dispatch_windows_include_for_arch(${DISPATCH_MSVC_ARCH} DISPATCH_INCLUDES)
include_directories(BEFORE SYSTEM ${DISPATCH_INCLUDES})
dispatch_windows_lib_for_arch(${CMAKE_SYSTEM_PROCESSOR} DISPATCH_LIBDIR)
link_directories(${DISPATCH_LIBDIR})
if(NOT MINGW)
include(DispatchWindowsSupport)
dispatch_windows_arch_spelling(${CMAKE_SYSTEM_PROCESSOR} DISPATCH_MSVC_ARCH)
dispatch_windows_include_for_arch(${DISPATCH_MSVC_ARCH} DISPATCH_INCLUDES)
include_directories(BEFORE SYSTEM ${DISPATCH_INCLUDES})
dispatch_windows_lib_for_arch(${CMAKE_SYSTEM_PROCESSOR} DISPATCH_LIBDIR)
link_directories(${DISPATCH_LIBDIR})
endif()

include(CheckCSourceCompiles)
include(CheckSymbolExists)

check_c_source_compiles([=[
#include <Windows.h>
int main(int argc, char *argv[]) {
Expand Down Expand Up @@ -54,6 +58,22 @@ int main(int argc, char *argv[]) {
if(DISPATCH_HAVE_EXTENDED_SLPI_22000)
add_compile_definitions(DISPATCH_HAVE_EXTENDED_SLPI_22000)
endif()

check_c_source_compiles([=[
#include <Windows.h>
#include <winternl.h>
int main(int argc, char *argv[]) {
FILE_PIPE_LOCAL_INFORMATION fpli;
}
]=] HAVE_FILE_PIPE_LOCAL_INFORMATION)
if(HAVE_FILE_PIPE_LOCAL_INFORMATION)
add_compile_definitions(HAVE_FILE_PIPE_LOCAL_INFORMATION)
endif()

check_symbol_exists(mkstemp "stdlib.h" HAVE_MKSTEMP)
if(HAVE_MKSTEMP)
add_compile_definitions(HAVE_MKSTEMP)
endif()
endif()

set(CMAKE_C_STANDARD 11)
Expand Down
4 changes: 3 additions & 1 deletion cmake/modules/DispatchCompilerWarnings.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@

if("${CMAKE_C_SIMULATE_ID}" STREQUAL "MSVC")
if("${CMAKE_C_SIMULATE_ID}" STREQUAL "MSVC" OR MINGW)
# TODO: someone needs to provide the msvc equivalent warning flags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this project compiles on Windows using clang-cl. In that case, all of the compiler options defined below can be passed to the clang-cl compiler. That surfaces a bunch of warnings, which could be fixed in a follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

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

IIRC, -fms-extensions are non-conforming extensions and we should not be enabling them. If MinGW requires that, then it should be done under a MINGW specific clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You need -fms-extensions because __popcnt64 is used in hw_config.h. That's a Microsoft-specific extension.

The source code, as it stands, requires this Microsoft-specific extension when targeting the Windows platform. I assume you don't need to make it explicit on Windows because you're using clang-cl (which enables -fms-extensions by default), or the MSVC compiler.

I think the proper condition is "when targeting Windows but not using a MSVC-compatible compiler`, let me try to refine that if clause.

# -fms-extensions will enable __popcnt64
add_compile_options($<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:-fms-extensions>)
else()
add_compile_options($<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:-Werror>)
add_compile_options($<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:-Wall>)
Expand Down
3 changes: 3 additions & 0 deletions os/generic_win_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
// Unices provide `howmany` via sys/param.h
#define howmany(x, y) (((x) + ((y) - 1)) / (y))

#ifndef _MODE_T_
#define _MODE_T_
Copy link
Member

Choose a reason for hiding this comment

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

Is there no generic way to avoid this? I'd rather not encode the macro that MinGW uses for this into the header. Why not hoist this into a check in CMake and use HAVE_MODE_T instead?

typedef int mode_t;
#endif
typedef void pthread_attr_t;

#ifndef API_AVAILABLE
Expand Down
5 changes: 5 additions & 0 deletions src/event/workqueue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@

#if defined(_WIN32)
#include <wct.h>

#if !defined(WCT_MAX_NODE_COUNT)
#define WCT_MAX_NODE_COUNT 16
#endif

#endif

/*
Expand Down
2 changes: 2 additions & 0 deletions src/shims/generic_win_stubs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ bool _dispatch_handle_is_socket(HANDLE hFile);
void _dispatch_QueryInterruptTimePrecise(PULONGLONG lpInterruptTimePrecise);
void _dispatch_QueryUnbiasedInterruptTimePrecise(PULONGLONG lpUnbiasedInterruptTimePrecise);

#ifndef HAVE_FILE_PIPE_LOCAL_INFORMATION
enum {
FilePipeLocalInformation = 24,
};
Expand All @@ -65,6 +66,7 @@ typedef struct _FILE_PIPE_LOCAL_INFORMATION {
ULONG NamedPipeState;
ULONG NamedPipeEnd;
} FILE_PIPE_LOCAL_INFORMATION, *PFILE_PIPE_LOCAL_INFORMATION;
#endif

NTSTATUS _dispatch_NtQueryInformationFile(HANDLE FileHandle,
PIO_STATUS_BLOCK IoStatusBlock, PVOID FileInformation, ULONG Length,
Expand Down
2 changes: 2 additions & 0 deletions tests/generic_win_port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ randomize_name(char *out)
}
}

#ifndef HAVE_MKSTEMP
dispatch_fd_t
mkstemp(char *tmpl)
{
Expand All @@ -257,6 +258,7 @@ mkstemp(char *tmpl)
errno = EEXIST;
return -1;
}
#endif

void
print_winapi_error(const char *function_name, DWORD error)
Expand Down
4 changes: 4 additions & 0 deletions tests/generic_win_port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
#include <Windows.h>

typedef int kern_return_t;
#ifndef _PID_T_
Copy link
Member

Choose a reason for hiding this comment

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

Similar

typedef int pid_t;
#endif

#if defined(_WIN64)
typedef long long ssize_t;
Expand Down Expand Up @@ -68,8 +70,10 @@ mach_timebase_info(mach_timebase_info_t tbi)
return 0;
}

#ifndef HAVE_MKSTEMP
dispatch_fd_t
mkstemp(char *tmpl);
#endif

void
print_winapi_error(const char *function_name, DWORD error);
Expand Down