Skip to content

ci: experiment with new mingw clang64#808

Open
SchrodingerZhu wants to merge 12 commits intomicrosoft:mainfrom
SchrodingerZhu:ci/mingw-clang64
Open

ci: experiment with new mingw clang64#808
SchrodingerZhu wants to merge 12 commits intomicrosoft:mainfrom
SchrodingerZhu:ci/mingw-clang64

Conversation

@SchrodingerZhu
Copy link
Collaborator

@SchrodingerZhu SchrodingerZhu commented Jan 29, 2026

Add a clang64 based mingw CI.

This should follow #806

@SchrodingerZhu
Copy link
Collaborator Author

SchrodingerZhu commented Jan 29, 2026

It seems that mingw-clang64 works OOB now.

However, there is a cmake issue that I do not understand:

 C:\Windows\system32\cmd.exe /C "cd . && C:\PROGRA~1\LLVM\bin\CLANG_~1.EXE -nostartfiles -nostdlib -O0 -g -Xclang -gcodeview -D_DEBUG -D_DLL -D_MT -Xclang --dependent-lib=msvcrtd -Xlinker /subsystem:console  -Wl,--no-undefined /DEBUG -rdynamic -fuse-ld=lld-link CMakeFiles/perf-contention-fast.dir/src/test/perf/contention/contention.cc.obj -o perf-contention-fast.exe -Xlinker /MANIFEST:EMBED -Xlinker /implib:perf-contention-fast.lib -Xlinker /pdb:perf-contention-fast.pdb -Xlinker /version:0.0   -lmincore.lib  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -loldnames  && cd ."
clang++: error: no such file or directory: '/DEBUG'

By using if (NOT MINGW) , this should already be eliminated. For some reasons, the change does not apply here.

cc @mjp41

@SchrodingerZhu SchrodingerZhu marked this pull request as ready for review January 29, 2026 15:11
This was referenced Jan 29, 2026
@ryancinsight
Copy link
Contributor

It seems that mingw-clang64 works OOB now.

However, there is a cmake issue that I do not understand:

 C:\Windows\system32\cmd.exe /C "cd . && C:\PROGRA~1\LLVM\bin\CLANG_~1.EXE -nostartfiles -nostdlib -O0 -g -Xclang -gcodeview -D_DEBUG -D_DLL -D_MT -Xclang --dependent-lib=msvcrtd -Xlinker /subsystem:console  -Wl,--no-undefined /DEBUG -rdynamic -fuse-ld=lld-link CMakeFiles/perf-contention-fast.dir/src/test/perf/contention/contention.cc.obj -o perf-contention-fast.exe -Xlinker /MANIFEST:EMBED -Xlinker /implib:perf-contention-fast.lib -Xlinker /pdb:perf-contention-fast.pdb -Xlinker /version:0.0   -lmincore.lib  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -loldnames  && cd ."
clang++: error: no such file or directory: '/DEBUG'

By using if (NOT MINGW) , this should already be eliminated. For some reasons, the change does not apply here.

cc @mjp41

could be this in cmakelists: target_link_options(${name} PRIVATE
$<$BOOL:${SNMALLOC_LINKER_SUPPORT_NO_ALLOW_SHLIB_UNDEF}:-Wl,--no-undefined>
$&lt;$&lt;PLATFORM_ID:Windows>:$<${ci_or_debug}:/DEBUG>>)
while I think you want: $&lt;$&lt;PLATFORM_ID:Windows>:$<${ci_or_debug}:LINKER:/DEBUG>>

@SchrodingerZhu
Copy link
Collaborator Author

I think it is working now.

@SchrodingerZhu SchrodingerZhu mentioned this pull request Jan 30, 2026
@lhmouse
Copy link

lhmouse commented Feb 3, 2026

If you mean to use C:\Program Files\LLVM\bin\clang++.exe then it's not MinGW clang; it's Clang-CL, because it links against MSVC CRT by default.

If you want to use clang that has a MinGW target by default, you should use MSYS2 CLANG64 shell.

@SchrodingerZhu
Copy link
Collaborator Author

SchrodingerZhu commented Feb 3, 2026

@lhmouse ah, nice catch. I just found that I deleted the shell command mistakenly.

@SchrodingerZhu SchrodingerZhu marked this pull request as draft February 3, 2026 04:12
@SchrodingerZhu
Copy link
Collaborator Author

I currently have a full time table. I may need more time to work on this.

@SchrodingerZhu
Copy link
Collaborator Author

@lhmouse also feel free to patch on top of this if you are interested and available.

Really a nice catch ... It turns out I was running clang-cl outside MSYS...... No wonder the if (MINGW) check was not working :).

@lhmouse
Copy link

lhmouse commented Feb 3, 2026

If you really mean to call the msys2 clang, you should set CMAKE_<LANG>_COMPILER to absolute paths.

@SchrodingerZhu
Copy link
Collaborator Author

SchrodingerZhu commented Feb 3, 2026

If you really mean to call the msys2 clang, you should set CMAKE_<LANG>_COMPILER to absolute paths.

shell: msys2 {0} is good enough according to my previous experience. The new problem is actually that we DO HAVE compatibility issue in current setup.

@SchrodingerZhu SchrodingerZhu marked this pull request as ready for review February 3, 2026 15:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds experimental MinGW Clang64 CI support following the refactored CI structure from PR #806. The changes enable building and testing snmalloc on Windows using MSYS2's Clang64 environment, which is a different toolchain from the existing MSVC-based Windows builds.

Changes:

  • Added a new MinGW Clang64 GitHub Actions workflow with Debug and RelWithDebInfo build profiles
  • Updated CMake to add -fms-extensions flag for Clang on MinGW to enable Microsoft pragma support
  • Changed linker flag syntax from /DEBUG to LINKER:/DEBUG for better cross-linker compatibility
  • Excluded MS-specific pragmas (pragma warning and pragma init_seg) from MinGW builds
  • Disabled the protect_fork test on MinGW since fork functionality doesn't work on this platform

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
.github/workflows/mingw.yml New workflow for MinGW Clang64 CI with matrix builds for Debug and RelWithDebInfo profiles
CMakeLists.txt Added -fms-extensions for MinGW Clang and fixed linker flag syntax for cross-platform compatibility
src/snmalloc/pal/pal_windows.h Excluded MSVC-specific pragmas from MinGW builds to avoid compiler errors
src/test/func/protect_fork/protect_fork.cc Skip test on MinGW where fork() is not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cmake
--build "$(cygpath -u "$GITHUB_WORKSPACE")/build"
--parallel

Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

There is trailing whitespace at the end of this line. Removing it would improve consistency.

Suggested change

Copilot uses AI. Check for mistakes.
run: |
cd "$(cygpath -u "$GITHUB_WORKSPACE")/build"
ctest

Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

There is trailing whitespace at the end of this line. Removing it would improve consistency.

Suggested change

Copilot uses AI. Check for mistakes.
-B"$(cygpath -u "$GITHUB_WORKSPACE")/build"
-DCMAKE_BUILD_TYPE=${{ matrix.profile }}
-DCMAKE_CXX_COMPILER=clang++
-DCMAKE_C_COMPILER=clang
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The configure step is missing the -DSNMALLOC_CI_BUILD=ON flag that is consistently used in other CI workflows. This flag enables additional debug information and backtrace support in CI builds, which would be beneficial for debugging failures in this workflow as well.

Suggested change
-DCMAKE_C_COMPILER=clang
-DCMAKE_C_COMPILER=clang
-DSNMALLOC_CI_BUILD=ON

Copilot uses AI. Check for mistakes.
shell: msys2 {0}
run: |
cd "$(cygpath -u "$GITHUB_WORKSPACE")/build"
ctest
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The ctest invocation is missing common flags used in other workflows. It should include --output-on-failure to provide better debugging information when tests fail, -j 4 for parallel execution, and --timeout 400 to prevent hanging tests. For consistency with other workflows (see .github/workflows/reusable-cmake-build.yml:115), consider using: ctest --output-on-failure -j 4 -C ${{ matrix.profile }} --timeout 400

Suggested change
ctest
ctest --output-on-failure -j 4 -C ${{ matrix.profile }} --timeout 400

Copilot uses AI. Check for mistakes.
profile: [RelWithDebInfo, Debug]

name: MinGW Clang64 Build (${{ matrix.profile }})
runs-on: windows-2025
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The job is missing a timeout-minutes configuration. Other workflows in this repository consistently set timeout-minutes (e.g., 30 minutes for similar build jobs in reusable-cmake-build.yml). This is important to prevent jobs from hanging indefinitely and consuming runner resources.

Suggested change
runs-on: windows-2025
runs-on: windows-2025
timeout-minutes: 30

Copilot uses AI. Check for mistakes.
Comment on lines +390 to 391

target_link_options(${name} PRIVATE
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

There is trailing whitespace at the end of this line. While this doesn't affect functionality, removing it would improve code consistency.

Suggested change
target_link_options(${name} PRIVATE
target_link_options(${name} PRIVATE

Copilot uses AI. Check for mistakes.
Comment on lines +23 to +26

- name: Setup MSYS2
id: msys2
uses: msys2/setup-msys2@v2
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

There is an extra blank line here. For consistency with other workflow files in the repository, there should be a single blank line between steps.

Suggested change
- name: Setup MSYS2
id: msys2
uses: msys2/setup-msys2@v2
- name: Setup MSYS2
id: msys2
uses: msys2/setup-msys2@v2
uses: msys2/setup-msys2@v2

Copilot uses AI. Check for mistakes.

name: MinGW Clang64 Build (${{ matrix.profile }})
runs-on: windows-2025

Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

There is trailing whitespace at the end of this line. Removing it would improve consistency.

Copilot uses AI. Check for mistakes.
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.

3 participants