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

use build.zig instead of build.sh #1

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Conversation

andrewrk
Copy link
Contributor

@andrewrk andrewrk commented Sep 28, 2023

In order to merge this, some more steps are needed:

  1. Support fetching dependencies over git+http(s) ziglang/zig#17277 needs to land in zig
  2. add the build.zig logic for building the orca tool itself (if orca upstream used build.zig then we could just grab the exe from there)
  3. testing

for step 2, since the orca tool depends on python, this will reduce system dependencies down to only python. alternately python could be provided by zig build but that may be a bit extreme, might be better to just have a system dependency on python, because I think the orca devs might decide to eliminate that dependency eventually.

Why do this?

  • the project already depends on zig, so let's use it for the build system instead of an additional dependency on shell scripting (which does not work on Windows)
  • no need to fiddle with ORCA_DIR, zig build can set everything up for the user so they can get started with only running zig build

-mbulk-memory is redundant with setting the target
-fno-sanitize=undefined is redundant with disabling c sanitization above
no standard libraries is already the default with freestanding
@andrewrk
Copy link
Contributor Author

andrewrk commented Oct 1, 2023

ziglang/zig#17277 has landed in zig, however ziglang/zig#16678 is also a blocker for this PR since the orca source tree uses symlinks.

@kristoff-it
Copy link
Owner

lgtm

@kristoff-it kristoff-it merged commit 50b6089 into kristoff-it:main Oct 10, 2023
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