ci: add Windows builds using MinGW toolchain #1123

alaviss · 2024-01-20T23:04:14Z

Summary

Implement Windows 64-bit testing and builds using MinGW toolchain, as
well as providing Windows compatibility patches

Details

Toolchain and dependencies

DLLs are built within CI from Microsoft's vcpkg repository as it
appears to be the most reliable source for pinned library versions out
there.
vcpkg uses custom triplets hosted in this repository to ensure
reproducible builds
Toolchain is sourced from
https://github.com/niXman/mingw-builds-binaries, which provides bare
toolchains without extras like gdb.
The target is Windows Universal C Runtime, the new C99 compatible
standard library bundled with Windows 10.

CI changes

Download/Upload compiler actions has been rewritten in Python to make
sure that they are compatible across OSes.
Added a new setup-mingw action to setup a MinGW environment.
Added Windows to the matrix using windows-2022 image.

Git tree changes

Added configuration to force LF line endings on all platforms. This
makes sure that Windows users always has the same source on disk as
everyone else.

Compiler changes

Added missing resources required to compile koch from source
archive.
On Windows, nim and nimsuggest now have a stack size of 8MiB.
This avoids stack overflow when compiling complex programs, and make
sure that the stack size is similar to Linux and macOS.
On Windows, PE timestamps are removed for release builds using GCC.
This makes sure that release builds are reproducible.

Standard library changes

posix_utils examples are disabled on Windows as they could not be
compiled there.
pcre and sqlite now prefers canonical DLL names on Windows.
This makes it easier to use built DLLs from repositories like Conan or
vcpkg.

Tests changes

nimsuggest 's tester now normalizes all path endings to Unix-style
/ .
Removed tfdleak_multiple as the "flaky" status no longer applies
due to changes to tfdleak .
tvmprofiler got an increased time range to account for Windows CI
being relatively slower than the rest.
Tests that requires POSIX imports are disabled.

Closes #276

saem · 2024-01-20T23:25:37Z

Minor update to the PR summary to indicate that it's for 64-bit. Reviewing the rest of the PR now.

saem

LGTM

this is now a behemoth that requires slicing

## Summary Importing posix or modules that are meant to work on POSIX systems will make tests unable to compile on non-POSIX platforms (ie. Windows) due to missing required C headers. ---  ## Notes for Reviewers * Cherry picked from #1123 * I'm not sure if importing `posix` is required for `tconcepts_issues` (ie. due to unwanted interactions), but I haven't seen complaints from CI so far, nor was there any references to symbols defined in posix. <!-- Pull Request(PR) Help Before Merge Ensure: * title reads like a short changelog line entry * code includes tests and is documented * leave the source better than before, but split out big reformats See contributor (guide)[https://nim-works.github.io/nimskull/contributing.html] for details, especially if you're new to this project. Tips that make PRs easier: * for big/impactful changes, start with chat/discussions to refine ideas * refine the pull request message over time; don't have to nail it in one go

This commit implements CI/CD for Windows using MinGW toolchain. Implementation details: * Toolchain is sourced from https://github.com/niXman/mingw-builds-binaries, which are bare toolchain without extras like gdb. * The target is Windows Universal C Runtime, the new C99 compatible standard library bundled with Windows 10. * Download/Upload compiler actions has been rewritten in Python to make sure that they are compatible across OSes.

This file is required to build on Windows x64

The path in the test message uses Unix path separators, which is incorrect on Windows. There's no easy way to handle this with the current testament, so skip this one on Windows.

While POSIX shells will unwrap this, on Windows the quotes are passed verbatim.

With the usage of CompareObjectHandles, Windows leaks are now accurately determined and this test is obsolete.

We don't bundles DLLs at the moment, so make sure the name is close to what users might have in their environment.

We don't bundle DLLs at the moment, so keep the names close to what a typical Windows dev environment might carry.

Windows have \ as its preferred separator, which broke tests due to how they are compared. To handle this, normalize the separator to /, which is both supported by Windows and *nix, simplifying test writing

Windows CI seems to barely cut it to 10000, so bump the value higher. Also add a debug print so we know if the value requires adjustment.

The compiler requires a fair bit of stack space to function properly. Windows default 1MB is rather restrictive, so use 8MB to match the rest of the supported OS.

This ensure that release builds are reproducible.

This makes sure that Git will checkout files exactly the way they are on Windows, preventing errors from the automatic CRLF conversion.

This is a POSIX test

This test requires a POSIX import

The runnable examples only compiles on POSIX

saem

IMO, this change is about as lean as it gets. If we broke things out further, then we'd be making changes without Windows CI in place. Alternatively, we'd be blocked on Windows CI because it needs some of these changes.

saem · 2024-01-30T00:01:34Z

PR description looks good.

saem · 2024-01-30T00:02:03Z

Not going to merge until we get a chance to test the windows binaries that are built from this CI flow.

zerbina

Looks good to me. Thank you for adding Windows CI support!

Since this closes #276, could you add a "Closes" line for it to the PR description?

As far as I know of, @zerbina uses MinGW on Windows, so we are targeting the most tested toolchain.

Yep, I use MinGW on Windows.

tests/arc/timportedobj.nim

This makes sure that nimgrep.exe works out of the box

This makes sure that DLLs obtained from vcpkg are reproducible.

vcpkg-action switches directory before running vcpkg, so the relative path could not work in this case. This commit switches to absolute path to solve this problem.

alaviss · 2024-01-31T00:19:12Z

/merge

nimgrep is working on windows with the bundled pcre, so

github-actions · 2024-01-31T00:19:40Z

Merge requested by: @alaviss

Contents after the first section break of the PR description has been removed and preserved below:

Notes for Reviewers

Toolchain and target ABI selection

Why target MinGW instead of MSVC?

Most developers appear to carry a MinGW toolchain due to Nim using it as the default. Bringing in DLLs based on MSVC ABI is a recipe for disaster in a MinGW environment.

As far as I know of, @zerbina uses MinGW on Windows, so we are targeting the most tested toolchain.

Why don't we source toolchains and dependencies from MSYS2?

MSYS2 is a rolling release, as such their compiler and libraries may fluctuates greatly between runs, creating uncertainty in the pipeline.

Will there be an MSVC target?

Maybe. When this PR was first created there were too many issues. Now that those has been cleaned up an MSVC CI is definitely possible.

What architectures are supported?

Only x86-64 is built and tested at the moment.

alaviss requested a review from a team as a code owner January 20, 2024 23:04

alaviss force-pushed the mingw branch 3 times, most recently from 959e475 to 03fbbf7 Compare January 20, 2024 23:13

saem added test Add or improve tests ci Continuous Integration labels Jan 20, 2024

alaviss force-pushed the mingw branch from 03fbbf7 to 34eab8c Compare January 20, 2024 23:20

saem previously approved these changes Jan 20, 2024

View reviewed changes

alaviss force-pushed the mingw branch 3 times, most recently from dea909f to 708190d Compare January 21, 2024 19:24

saem added this to the Infrastructure and contribution handling milestone Jan 22, 2024

This was referenced Jan 22, 2024

niminst: use 7-Zip to create zip archives #1128

Merged

shellrunner: add Windows support #1129

Merged

testament: don't close Process before reading exit code #1130

Merged

tests: avoid importing posix on non-POSIX systems #1131

Merged

alaviss added 11 commits January 29, 2024 16:42

compiler: add koch_icon.o to source archive

ceb9ad9

This file is required to build on Windows x64

tstructured_parse_fail: disable on Windows

bceb1df

The path in the test message uses Unix path separators, which is incorrect on Windows. There's no easy way to handle this with the current testament, so skip this one on Windows.

t01_ident_pragmas: don't quote value

b9c74d5

While POSIX shells will unwrap this, on Windows the quotes are passed verbatim.

tfdleak_multiple: no longer necessary

78f7bcb

With the usage of CompareObjectHandles, Windows leaks are now accurately determined and this test is obsolete.

pcre: use canonical DLL name on Windows

9be3350

We don't bundles DLLs at the moment, so make sure the name is close to what users might have in their environment.

sqlite3: use canonical DLL name on Windows

a4cf8f1

We don't bundle DLLs at the moment, so keep the names close to what a typical Windows dev environment might carry.

nimsuggest/tester: normalize path separator

cd1dfd5

Windows have \ as its preferred separator, which broke tests due to how they are compared. To handle this, normalize the separator to /, which is both supported by Windows and *nix, simplifying test writing

tosproc: add Windows support

b2f1759

tvmprofiler: triple the upper range

0417783

Windows CI seems to barely cut it to 10000, so bump the value higher. Also add a debug print so we know if the value requires adjustment.

compiler: use 8MB stack on Windows to prevent overflow

a7769b1

The compiler requires a fair bit of stack space to function properly. Windows default 1MB is rather restrictive, so use 8MB to match the rest of the supported OS.

alaviss added 7 commits January 29, 2024 16:42

nim.cfg: don't insert build timestamps to PE in release mode

50f4b9e

This ensure that release builds are reproducible.

koch: build vcc in release mode

2e4bafa

koch: disable PE timestamps for nim_dbg

c0820f2

gitattributes: enforce LF line endings

07ba0f7

This makes sure that Git will checkout files exactly the way they are on Windows, preventing errors from the automatic CRLF conversion.

timportedobj: disable on windows

0c2fc8d

This is a POSIX test

tconcepts_issues: disable on Windows

43bb6fa

This test requires a POSIX import

posix_utils: only define runnable examples on posix

4237390

The runnable examples only compiles on POSIX

alaviss force-pushed the mingw branch from ede6030 to 4237390 Compare January 29, 2024 23:19

saem approved these changes Jan 29, 2024

View reviewed changes

zerbina approved these changes Jan 30, 2024

View reviewed changes

tests/arc/timportedobj.nim Show resolved Hide resolved

ci: install PCRE DLLs to Windows package

bc2d046

This makes sure that nimgrep.exe works out of the box

alaviss force-pushed the mingw branch from 2531cc9 to bc2d046 Compare January 30, 2024 21:57

alaviss added 2 commits January 30, 2024 17:14

ci: add custom vcpkg triplet to remove PE timestamps

cd769c3

This makes sure that DLLs obtained from vcpkg are reproducible.

ci: use absolute path for triplets

864dd43

vcpkg-action switches directory before running vcpkg, so the relative path could not work in this case. This commit switches to absolute path to solve this problem.

chore-runner bot added this pull request to the merge queue Jan 31, 2024

Merged via the queue into nim-works:devel with commit 2ba0893 Jan 31, 2024
25 checks passed

alaviss deleted the mingw branch January 31, 2024 00:56

alaviss mentioned this pull request Jan 31, 2024

Add Windows binaries to GitHub Actions #276

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: add Windows builds using MinGW toolchain #1123

ci: add Windows builds using MinGW toolchain #1123

alaviss commented Jan 20, 2024 •

edited by github-actions bot

Loading

saem commented Jan 20, 2024

saem left a comment

saem left a comment

saem commented Jan 30, 2024

saem commented Jan 30, 2024

zerbina left a comment

alaviss commented Jan 31, 2024

github-actions bot commented Jan 31, 2024

Notes for Reviewers

Toolchain and target ABI selection

Why target MinGW instead of MSVC?

Why don't we source toolchains and dependencies from MSYS2?

Will there be an MSVC target?

What architectures are supported?

ci: add Windows builds using MinGW toolchain #1123

ci: add Windows builds using MinGW toolchain #1123

Conversation

alaviss commented Jan 20, 2024 • edited by github-actions bot Loading

Summary

Details

Toolchain and dependencies

CI changes

Git tree changes

Compiler changes

Standard library changes

Tests changes

saem commented Jan 20, 2024

saem left a comment

Choose a reason for hiding this comment

saem left a comment

Choose a reason for hiding this comment

saem commented Jan 30, 2024

saem commented Jan 30, 2024

zerbina left a comment

Choose a reason for hiding this comment

alaviss commented Jan 31, 2024

github-actions bot commented Jan 31, 2024

Notes for Reviewers

Toolchain and target ABI selection

Why target MinGW instead of MSVC?

Why don't we source toolchains and dependencies from MSYS2?

Will there be an MSVC target?

What architectures are supported?

alaviss commented Jan 20, 2024 •

edited by github-actions bot

Loading