Skip to content

Win32 support#118

Draft
cyanidle wants to merge 13 commits intomichaelforney:masterfrom
cyanidle:win32
Draft

Win32 support#118
cyanidle wants to merge 13 commits intomichaelforney:masterfrom
cyanidle:win32

Conversation

@cyanidle
Copy link
Copy Markdown

@cyanidle cyanidle commented May 14, 2025

Hello. I am another poor soul trying to add windows support.

I got basic stuff working: os-dependent jobs stuff is separated into os.h. I also fixed lots of memory leaks found with -fsanitize-address. Posix side with new abstractions seems to work OK.

Things that need fixing before windows support can be declared:

  • deps = msvc
  • $variable substitutions are escaped with quotes, which works fine with /usr/bin/sh -c, but fails on Windows (CreateProcess does no CLI processing), when MSVC tries to compile something (cl.exe ... -c '<file') -> causes warning D9027 : source file ''samurai\os-win32.c'' ignored
  • Parsing .ninja_deps from ninja causes segfault in depsinit() (node->gen is null for some reason)
  • jobs/osjobs array seems to never shrink. On huge number of jobs in build this will cause lots of iterations over these two arrays. Perf hit is probably not very big, but this is very O(n^2)
  • Makefile should be modified to support .exe suffix and use mt.exe to add manifest file -> important for UTF-8 and long paths. NMake does not support Gnu Make's ifeq(), it has its own syntax for conditionals. May be easier to have two Makefiles, for make and nmake
  • setvbuf() is disabled in win32, bc it causes assert with bufsize zero
  • canonpath() and other path related APIS

CMakefile and related gitignores are for development only, it helps with IDE support.

There seem to be issues with formatting caused by said IDEs and TABS. Idk how to fix that :D

@tritao
Copy link
Copy Markdown

tritao commented May 15, 2025

I'm just a passer by, just want to say nice work, hope this can be completed and Win32 support added 👍

There seem to be issues with formatting caused by said IDEs and TABS. Idk how to fix that :D

Left a few comments related to this before reading this, but seems like you're already aware.

Copy link
Copy Markdown

@TomasBorquez TomasBorquez left a comment

Choose a reason for hiding this comment

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

left comments on some warning you were getting, also i think you can fix the indentation with clang-format project settings:

clang-format -i -style=file $(find ./ -name "*.c" -o -name "*.h")

@TomasBorquez
Copy link
Copy Markdown

First test - simplest

Simple ninja file:

cc = gcc
target = main.exe

rule link
  command = $cc $flags -o $out $in

rule compile
  command = $cc $flags -c $in -o $out

build main.o: compile main.c
build $target: link main.o

default $target

Ran it on the same folder:

Lewboski@DESKTOP-CK46RK4 MINGW64  /c/Users/Lewboski/Desktop/Programming/learn/mate/tests/01-basic-build/build
$ ./samu.exe 
[1/2] gcc  -c main.c -o main.o
C:\Users\Lewboski\Desktop\Programming\learn\mate\tests\01-basic-build\build\samu.exe: CreateProcess: No such file or directory
C:\Users\Lewboski\Desktop\Programming\learn\mate\tests\01-basic-build\build\samu.exe: job failed to start

Second test - vars

I tested it with some variables on a single source:

cc = gcc
flags = -Wall -g
cwd = C$:\Users\Lewboski\Desktop\Programming\learn\mate\tests\01-basic-build
builddir = C$:\Users\Lewboski\Desktop\Programming\learn\mate\tests\01-basic-build\build
target = $builddir/main.exe

rule link
  command = $cc $flags -o $out $in

rule compile
  command = $cc $flags -c $in -o $out

build $builddir/main.o: compile $cwd/src/main.c
build $target: link $builddir/main.o

default $target

Ran it with -f, and got:

Lewboski@DESKTOP-CK46RK4 MINGW64  /c/Users/Lewboski/Desktop/Programming/learn/mate/tests/01-basic-build
$ ./build/samu.exe -f ./build/build.ninja
C:\Users\Lewboski\Desktop\Programming\learn\mate\tests\01-basic-build\build\samu.exe: mkdirs C:\: No error
C:\Users\Lewboski\Desktop\Programming\learn\mate\tests\01-basic-build\build\samu.exe: job failed to start
C:\Users\Lewboski\Desktop\Programming\learn\mate\tests\01-basic-build\build\samu.exe: subcommand failed

you can use these as reference meanwhile testing

Copy link
Copy Markdown
Owner

@michaelforney michaelforney left a comment

Choose a reason for hiding this comment

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

This looks like a pretty good initial attempt! I'll need to take a closer look at the jobs abstraction.

It seems like there are a lot of places where pointer to bool conversion doesn't work. Is this because MSVC doesn't have bool? Or is something else going on?

Could you revert the formatting changes? As mentioned in .clang-format, the changes it suggests aren't authoritative. I have formatted everything how I want it, and I think the rules in .clang-format are as close as I can get.

There should be a way to configure your IDE to use tabs. Maybe somewhere in the settings?

build/
CMakeUserPresets.json
CMakePresets.json
*.user No newline at end of file
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

These are probably better to put in your .git/info/exclude instead.

Copy link
Copy Markdown
Author

@cyanidle cyanidle Apr 8, 2026

Choose a reason for hiding this comment

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

Agree, will revert later probably

static size_t nstarted, nfinished, ntotal;
static bool consoleused;
static struct timespec starttime;
static struct ostimespec starttime;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think we should just make it ostime() and have it return a double.

@@ -0,0 +1,29 @@
cmake_minimum_required(VERSION 3.16)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Do you think it's possible to get the Makefile to work with nmake?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

NMake is not very compatible with Make, easier to maintain both, considering project simplicity

build.c Outdated
int64_t old;

restat = edgevar(e, "restat", true);
struct string * restat = edgevar(e, "restat", true);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you show the error here? Does MSVC not allow conversion of pointer to bool?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

MSVC allows, I did this and similar patches to fix memory leaks - edgevar() allocates new string (e.g: see line 380).

@cyanidle
Copy link
Copy Markdown
Author

cyanidle commented Apr 8, 2026

@michaelforney Hello! you are finally active. Though I myself moved away from this PR) I kinda lost interest and there is a lot of work to be done.
But maybe not all hope is lost, if you think this job abstraction thing is worth it - I may continue work on Win compatability. Though not at a very quick pace.

@cyanidle cyanidle marked this pull request as draft April 8, 2026 19:09
@cyanidle
Copy link
Copy Markdown
Author

cyanidle commented Apr 8, 2026

I read couple of your new commits, I see you are doing some work on abstracting os-specific things. Reviving this PR may not be very worth it (though looking at the implementation of some things probably is)

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.

4 participants