Skip to content

Add mingw-w64 support#82

Open
programmerjake wants to merge 4 commits intostoth68000:masterfrom
programmerjake:add-mingw-w64-support
Open

Add mingw-w64 support#82
programmerjake wants to merge 4 commits intostoth68000:masterfrom
programmerjake:add-mingw-w64-support

Conversation

@programmerjake
Copy link
Copy Markdown

@programmerjake programmerjake commented Jun 7, 2025

I got it to cross compile from Debian 12 using Autotools.

I ended up deciding to just disable UDP support on Windows, since I don't need it and it's a large amount of porting work (I'm not familiar with how Windows does multicast or finding interfaces).

Since mingw-w64 doesn't have a <regex.h>, I replaced all the regexps with C code that should have equivalent/improved functionality.

I also added .clang-format with a style that matched what existed before relatively closely (I couldn't match the line break in between else and if) and formatted all the code.

Because the code was reformatted, you'll definitely want to review it commit-by-commit rather than just looking at the combined diff.

Please don't squash or rebase that commit unless you also update .git-blame-ignore-revs.

I added CI that runs on GitHub Actions, it tests building on ubuntu 22.04 and 24.04 with either meson or autotools as well as on windows in msys2 with autotools. It works on my fork on Linux and Windows, but you seem to not have CI enabled on your repository, so it doesn't show up in this PR.

@programmerjake programmerjake force-pushed the add-mingw-w64-support branch 2 times, most recently from 9eda310 to 420dab5 Compare June 11, 2025 04:33
@programmerjake programmerjake marked this pull request as ready for review June 11, 2025 04:46
@dheitmueller
Copy link
Copy Markdown
Contributor

@programmerjake Thanks for your submission. This patch requires some careful review, as there's a quite a bit that has changed. The large patch reformatting the code will almost certainly not be accepted. The project follows the "linux" style for code formatting and indentation, and it's not quite clear what standard your patch attempts to enforce.

That said, the CI improvements and Windows support will likely be a welcome addition. I'll have to look at the regex/sockets stuff, as it's possible that it might be simpler to just exclude the one test tool which uses those facilities (a test harness for 2038 parsing).

@programmerjake
Copy link
Copy Markdown
Author

@programmerjake Thanks for your submission. This patch requires some careful review, as there's a quite a bit that has changed. The large patch reformatting the code will almost certainly not be accepted. The project follows the "linux" style for code formatting and indentation, and it's not quite clear what standard your patch attempts to enforce.

I tried to match the existing style. I can change that Linux's style if you like, that file is GPL 2.0 but since it isn't compiled into the library it should be fine.

@programmerjake programmerjake force-pushed the add-mingw-w64-support branch from 420dab5 to 3f87ef0 Compare June 16, 2025 21:18
@programmerjake programmerjake force-pushed the add-mingw-w64-support branch from 3f87ef0 to 56cf6d9 Compare June 16, 2025 21:23
@programmerjake
Copy link
Copy Markdown
Author

I removed the reformatting commits, but imo enforcing formatting using some style is a good idea, since the codebase currently has a lot of code that mixes spaces and tabs for indentation making it quite confusing to read and causes a bunch of misleading indentation warnings. One line I fixed had a tab in the middle of a sequence of spaces in the indentation.

Comment thread tools/Makefile.am
endif

if DEBUG
CFLAGS += -g
Copy link
Copy Markdown
Author

@programmerjake programmerjake Jun 16, 2025

Choose a reason for hiding this comment

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

I discovered that if you have variable assignments indented then they don't work correctly, so I fixed this one too since it likely also doesn't work. (edit: I put the comment one line too high, I'm dedenting the line, not removing it)

@dheitmueller
Copy link
Copy Markdown
Contributor

@programmerjake Thanks for the additional work. No disagreement on the fixing formatting issues in general, although the original patch appeared to have more changes than simply using tabs instead of spaces.

I'll try to find some time to look at this patch series over the weekend.

@dheitmueller
Copy link
Copy Markdown
Contributor

Sorry, I've been backlogged. I haven't forgotten about this pull request.

@programmerjake
Copy link
Copy Markdown
Author

Sorry, I've been backlogged. I haven't forgotten about this pull request.

are you caught up yet?

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.

2 participants