Skip to content

Fix Windows build#117

Merged
ecyht2 merged 8 commits into
mmwillet:mainfrom
mrexodia:msvc-build
Oct 5, 2025
Merged

Fix Windows build#117
ecyht2 merged 8 commits into
mmwillet:mainfrom
mrexodia:msvc-build

Conversation

@mrexodia

@mrexodia mrexodia commented Sep 1, 2025

Copy link
Copy Markdown
Contributor
  • Automatically fetch SDL2 if it cannot be found
  • Use std::numbers::pi instead of non-standard M_PI to fix dummy build
  • Use path.string() where needed to fix compilation

@danielzgtg danielzgtg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I reproduced the .string() problem, thank you for fixing our oversight of tts-server!

I made two minor suggestions for improvement.

Comment thread src/models/dummy/model.cpp
Comment thread examples/cli/CMakeLists.txt Outdated
@danielzgtg

Copy link
Copy Markdown
Collaborator

@mmwillet Pinging as this will use your CI minutes.

I plan to merge after 24 hours, despite the Windows runner being projected to more than triple the minutes used, because we weren't using many CI minutes to begin with.

Comment thread src/models/dummy/model.cpp
@ecyht2

ecyht2 commented Sep 2, 2025

Copy link
Copy Markdown
Collaborator

I do have Windows VMs, I might be able to test it out. @mrexodia, are you using msvc or mingw?

Edit: Whoops probably should read the code first.

@mrexodia

mrexodia commented Sep 2, 2025

Copy link
Copy Markdown
Contributor Author

I am using MSVC. If you want to test you can use a zig toolchain: https://github.com/mrexodia/zig-cross

Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml

@ecyht2 ecyht2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you also update the README.md to show that we have Windows support?

@mrexodia

mrexodia commented Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

Can you also update the README.md to show that we have Windows support?

I do not know what to change in the feature matrix because I did not test everything. I think this should be done by one of the maintainers and not a random contributing some minor compilation fixes 😅

@mrexodia

mrexodia commented Sep 6, 2025

Copy link
Copy Markdown
Contributor Author

I also fixed the Linux build, tested locally with Ubuntu 24.04 LTS

Comment thread .github/workflows/build.yml
Co-authored-by: ecyht2 <94816144+ecyht2@users.noreply.github.com>
@ecyht2 ecyht2 merged commit c04c77a into mmwillet:main Oct 5, 2025
3 checks passed
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.

Compiling on MacOS: Undefined symbols for architecture arm64: "dummy_register()"

3 participants