-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
initial Zig 12 support #36
base: master
Are you sure you want to change the base?
Conversation
} | ||
const os = builtin.os.tag; | ||
const arch = builtin.cpu.arch; | ||
return (comptime if (std.mem.eql(u8, @tagName(os), "macos")) "darwin" else @tagName(os)) ++ "-" ++ @tagName(arch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
android uses "darwin" tag instead of "macos" as zig does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect you're the first mac use to try this out! Thank you :)
@@ -1151,22 +1152,22 @@ const BuildOptionStep = struct { | |||
} | |||
return; | |||
}, | |||
std.builtin.Version => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't found this type in the standard library, so I removed it for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zig 0.11.0 release notes address this: https://ziglang.org/download/0.11.0/release-notes.html#Language-Changes
- Replaced builtin.Version with SemanticVersion.
Since SemanticVersion is already implemented below this code, we can go ahead and delete the std.builtin.Version
case entirely.
@@ -24,17 +24,19 @@ pub fn findUserConfig(b: *Builder, versions: Sdk.ToolchainVersions) !UserConfig | |||
// Check for a user config file. | |||
if (std.fs.cwd().openFile(config_path, .{})) |file| { | |||
defer file.close(); | |||
const bytes = file.readToEndAlloc(b.allocator, 1 * 1000 * 1000) catch |err| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't figured out the new Json api, it's hard because I haven't used the previous one.
I have added a few remarks. |
Currently getting following error when running
|
Co-authored-by: Nigel Baillie <nigel@baillie.dev>
how is the fork going? I can't just make this repository work no matter what zig version I use |
@MasterQ32 @desttinghim |
I'm sorry, i don't have time to review the code right now. I'm super busy doing RL stuff atm The file-not-founds are probably due to some misconfigured paths that aren't compatible with whatever changed. See how those paths are constructed in the code |
I may be able to take a look, most of my focus with Android at the moment is on my build tool cyborg though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the changes to build.zig
are done, there is still plenty of work needed to make everything compile on the 0.12 pre-release, but the changes should be fairly straightforward:
- Change
@alignCast()
calls to remove the@alignOf()
parameter being passed - Change
var
toconst
when the value is never mutated, the compiler should point out all of these
@@ -1151,22 +1152,22 @@ const BuildOptionStep = struct { | |||
} | |||
return; | |||
}, | |||
std.builtin.Version => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zig 0.11.0 release notes address this: https://ziglang.org/download/0.11.0/release-notes.html#Language-Changes
- Replaced builtin.Version with SemanticVersion.
Since SemanticVersion is already implemented below this code, we can go ahead and delete the std.builtin.Version
case entirely.
@@ -926,7 +926,8 @@ fn createLibCFile(sdk: *const Sdk, version: AndroidVersion, folder_name: []const | |||
try writer.writeAll("gcc_dir=\n"); | |||
|
|||
const step = sdk.b.addWriteFile(fname, contents.items); | |||
return step.getFileSource(fname) orelse unreachable; | |||
// return step.getFileSource(fname) orelse unreachable; | |||
return step.addCopyFile(.{ .path = fname }, ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addCopyFile
isn't the function you want here. We're already writing out the file using addWriteFile
, we just need to get a FileSource
from it. According to the message in the deprecated getFileSource()
function, we need to use step.files.items[0].getPath()
here.
} | ||
const os = builtin.os.tag; | ||
const arch = builtin.cpu.arch; | ||
return (comptime if (std.mem.eql(u8, @tagName(os), "macos")) "darwin" else @tagName(os)) ++ "-" ++ @tagName(arch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect you're the first mac use to try this out! Thank you :)
Thanks for the remarks! I'll look into them once I find the time, I'm also quite busy, but I should be able to find some free time to work on this. |
I did mange to build it using the zig I used the build-tools 32.0.0, I think we should auto detect it and just use the highest installed version. Do you guys whant to check it out? how do I push it? it was based on this work Edit: I got it working but I'm getting Edit2: The install step was copying the apk before the singning |
@lassade this looks like some sort of signing issue, but I'm unable to tell what the actually issue is. Can you open a PR with the changes you made? |
I started initial support for Zig 12 (and 11), but I can't figure out a few things.