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

Merged
merged 3 commits into from
Jan 24, 2025

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).

septechx added a commit to septechx/lush that referenced this pull request Jan 10, 2025
@nurpax
Copy link
Contributor Author

nurpax commented Jan 24, 2025

I think this issue is still unresolved. Neither this nor the change mentioned above is in.

+ this includes fixing things to enable running "zig build test"

for natecraddock#130
@nurpax
Copy link
Contributor Author

nurpax commented Jan 24, 2025

@natecraddock AFAIC this PR is good to go. I checked that it also implements what was pointed out in the above comment but also adds some fixes to newer zig versions.

I also snuck in a new test for pushAny of an anonymous struct. I didn't see one in the existing tests, and that was the usage pattern in my code where I noticed these new breakages.

@natecraddock
Copy link
Owner

Hi! So sorry for the long delays and silence. Life has been crazy

Thanks for working on this

@natecraddock natecraddock merged commit 4322c27 into natecraddock:main Jan 24, 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.

4 participants