Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build break for zig 0.14.0-dev.2596 #131

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nurpax
Copy link
Contributor

@nurpax nurpax commented Jan 3, 2025

  • There's no root_module.addCMacro anymore

  • There's also no root_module.unwind_tables boolean anymore. My fix only fixes the break but I don't know what's the right value to set here. The choices are null, sync, and async. I couldn't get the luajit variant to build on Windows. Maybe merge this and ask the luajit contributor for a proper fix?

for #130

@nurpax
Copy link
Contributor Author

nurpax commented Jan 3, 2025

I think CI is not testing luajit. But at least my superficial attempt to look at it failed on Windows, so it might not work to enable it in the makefile with the current GitHub Actions config.

@efjimm
Copy link
Contributor

efjimm commented Jan 5, 2025

The sync option for unwind_tables matches the behaviour as before. The build runner previously passed the -funwind-tables flag when unwind_tables was set to true, it now passes that same flag when unwind_tables is set to sync.

@nurpax
Copy link
Contributor Author

nurpax commented Jan 6, 2025

Thanks! I'll update the PR to use sync.

- There's no root_module.addCMacro anymore

- There's also no root_module.unwind_tables boolean anymore.  Setting
  "unwind_tables = .sync" should match earlier behavior.

for natecraddock#130
@nurpax nurpax force-pushed the build-fix-0.14-dev.2596 branch from 2233e8f to 3d36e37 Compare January 6, 2025 10:43
@nurpax
Copy link
Contributor Author

nurpax commented Jan 6, 2025

PR updated to use sync. This change is good to go as far as I'm concerned.

@freegamerskids
Copy link

This issue was partially fixed in #127, and already fully fixed in #129 (if needed, i can bump the zig version).

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