-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 support for haiku #7546
Conversation
archive of patches that can be applied to llvm to get it to build for haiku (can apply llvm in zig-bootstrap) |
lib/std/c/haiku.zig
Outdated
pub extern "c" fn getdents(fd: c_int, buf_ptr: [*]u8, nbytes: usize) usize; | ||
|
||
pub const dl_iterate_phdr_callback = fn (info: *dl_phdr_info, size: usize, data: ?*c_void) callconv(.C) c_int; | ||
//pub extern "c" fn dl_iterate_phdr(callback: dl_iterate_phdr_callback, data: ?*c_void) c_int; |
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.
Remove this.
lib/std/debug.zig
Outdated
@@ -1677,6 +1692,17 @@ pub const ModuleDebugInfo = switch (builtin.os.tag) { | |||
unreachable; | |||
} | |||
}, | |||
.haiku => struct { | |||
// Haiku should implement dl_iterat_phdr (https://dev.haiku-os.org/ticket/15743) |
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.
Not really, see above.
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.
for the time being, i have backed out most of the changes except for more minimal stub logic for debug and will try to tackle this as a separate item (per above)
lib/std/fs.zig
Outdated
//const name = @ptrCast([*]u8, "placeholder-please-implement-me"); | ||
//const name = "placeholder-please-implement-me"; | ||
return Entry{ | ||
.name = base64_alphabet, |
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.
Ehh.
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've updated the logic (hopefully with something slightly less worse)
lib/std/process.zig
Outdated
@@ -775,6 +775,19 @@ pub fn getSelfExeSharedLibPaths(allocator: *Allocator) error{OutOfMemory}![][:0] | |||
} | |||
return paths.toOwnedSlice(); | |||
}, | |||
// Haiku should implement dl_iterat_phdr (https://dev.haiku-os.org/ticket/15743) |
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.
Ditto.
Season's greetings from the Zig project to the Haiku project! Thanks for the patches! |
Seeing this PR begs the question, what criteria does Zig use to support platforms in the standard library? |
thank you for the detailed review of the PR. i have pushed updates that hopefully addresses a good portion of the suggestions posted. i have tried to focus on ensuring it can at least build on Haiku and leave the fun parts for later adventures. after some more tinkering with the build as is on Haiku things i was able to confirm:
|
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.
Haiku developer here. Always exciting to see new languages add support for Haiku ... and especially when the code was not written by one of the usual HaikuPorts crew, to boot!
lib/std/fs/get_app_data_dir.zig
Outdated
@@ -56,6 +56,13 @@ pub fn getAppDataDir(allocator: *mem.Allocator, appname: []const u8) GetAppDataD | |||
}; | |||
return fs.path.join(allocator, &[_][]const u8{ home_dir, ".local", "share", appname }); | |||
}, | |||
.haiku => { | |||
const home_dir = os.getenv("HOME") orelse { |
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.
Haiku applications should store their data in ~/config/settings
, which you can determine by using the find_directory
API.
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.
see 3 lines below where they are creating the path ~/config/settings
But yes, it looks like find_directory
would be the more official API
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 have added in a very bare-bones implementation using the find_directory() as suggested.
lib/std/os/bits/haiku.zig
Outdated
const std = @import("../../std.zig"); | ||
const maxInt = std.math.maxInt; | ||
|
||
// See https://svnweb.freebsd.org/base/head/sys/sys/_types.h?view=co |
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.
This looks like a copy/paste error. (Haiku's equivalent types are defined in sys/types.h
.)
@@ -775,6 +775,24 @@ pub fn getSelfExeSharedLibPaths(allocator: *Allocator) error{OutOfMemory}![][:0] | |||
} | |||
return paths.toOwnedSlice(); | |||
}, | |||
// revisit if Haiku implements dl_iterat_phdr (https://dev.haiku-os.org/ticket/15743) |
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.
You could also submit a patch to Haiku implementing it ;)
Otherwise, yes, get_next_image_info
is the way to go.
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.
thanks for the invitation :) and might consider it in the future if someone else doesn't beat me to it. for the nearer term, i prefer to keep my focus on having something that can get this in a better compiling state on Haiku
lib/std/target.zig
Outdated
@@ -1501,6 +1505,10 @@ pub const Target = struct { | |||
.other, | |||
=> return result, | |||
|
|||
// Operating systems in this list have been verified as not having a standard | |||
// dynamic linker path. |
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.
Looks like a copy/paste error?
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.
yes, updated with hopefully a better note.
@@ -1501,6 +1505,10 @@ pub const Target = struct { | |||
.other, | |||
=> return result, | |||
|
|||
// Operating systems in this list have been verified as not having a standard | |||
// dynamic linker path. | |||
.haiku => return copy(&result, "/system/runtime_loader"), |
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.
FWIW, while this is correct at the present time, in the future when/if Haiku introduces proper multiarch (i.e. 32-bit on 64-bit) support, there will be multiple dynamic linkers for each arch, and invoking the wrong one will probably cause problems.
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.
So should this follow the pattern above (in e.g. linux) of .haiku => switch (self.cpu.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.
for the time being i have left a note regarding multiarch support.
lib/std/thread.zig
Outdated
@@ -484,6 +484,9 @@ pub const Thread = struct { | |||
}; | |||
return @intCast(usize, count); | |||
} | |||
if (std.Target.current.os.tag == .haiku) { |
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.
You can get this either via get_system_info
(a Haiku-specific API), or via sysconf(_SC_NPROCESSORS_CONF)
(which probably works on other OSes, too.)
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.
added
lib/std/c/haiku.zig
Outdated
|
||
pub const _errno = _errnop; | ||
|
||
pub extern "c" fn _kern_read_dir(fd: c_int, buf_ptr: [*]u8, nbytes: usize, maxcount: u32) usize; |
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.
_kern...
functions are syscall hooks, and we make no guarantees whatsoever about their ABIs, which are subject to change without notice, etc. (Calling syscalls via hooks is at least better than by number, though, as the numbers are generated at build time and so change whenever syscalls are touched in pretty much any way.)
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.
thanks! i will keep that in mind, i think some of these exposed syscall hooks might not be as necessary when dl_iterat_phdr support is available
lib/std/fs.zig
Outdated
continue :start_over; | ||
} | ||
|
||
// TODO: determine entry kind |
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.
You can do this by masking the mode using S_IFMT
and then checking the type (S_IFDIR etc.)
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.
that helps, thanks :)
If this is because of "read-only file system" errors, you will need to adjust your install paths to put things in |
@waddlesplash what is the status of "libbe.so" on haiku (which is where I see |
There has been discussion of breaking up |
Actually, the documentation page is just wrong, |
lib/std/thread.zig
Outdated
@@ -484,6 +484,9 @@ pub const Thread = struct { | |||
}; | |||
return @intCast(usize, count); | |||
} | |||
if (std.Target.current.os.tag == .haiku) { | |||
return 1; // TODO: implement me |
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.
Please remove all bugs such as this from this pull request. You can replace this with
return 1; // TODO: implement me | |
@panic("TODO implement std.Thread.cpuCount for Haiku"); |
Or better yet:
return 1; // TODO: implement me | |
@compileError("TODO implement std.Thread.cpuCount for Haiku"); |
No wrong implementations please!
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.
makes sense, and have placed in the suggestions in (hopefully) the appropriate places. might have implemented some parts in other areas which hopefully also addresses the problematic code spots
Lots of feedback! Appreciate all of the helpful comments. I will be going over the suggestions and work on getting the PR in a more ready state. Hopefully will have something ready soon! |
i have pushed more updates that should handle feedback up to this point and still am trying to stick with the goal of compiling under Haiku. i have separated out the changes for debug and plan to defer that for later efforts update on what i was able to confirm:
thanks again for the feedback! |
Part of it is whether an active person is around to do the labor, and the other part of it is whether supporting given platform results in any compromises for more mainstream use cases. In the future, I'd like to investigate OS support via packages; for now we do it by upstreaming into the std lib. In this specific case, I'm happy to add experimental Haiku support and see where it goes. @hoanga thanks for your patience on this one. I'm in the process of merging it now. |
* remove unused definitions * setup os specific blocks
* add cpu count * use haiku find_directory * add definitions and exports for building in haiku
* no isHaiku() function since there is not more than one os tag that this applies to. * clean up some control flow into a switch * add some TODO comments to investigate panics that suspiciously look like they should be compile errors (see ziglang#363)
413454d
to
37a1d08
Compare
seasons greetings zig community!
the following PR adds initial support for getting zig to build stage1 on Haiku. i have confirmed it compiles zig and zig0 and also still builds on other OSes (tested on mac).
the following is a short excerpt of building (requires llvm11)
there are some pieces that could use a little more polish but wanted to submit a rough working version for review and suggestions on if the PR can be made more modular or cleaner to merge.
some things i know that could be a little better and will see about getting them fixed when i have some more time if possible (suggestions / help would be welcome)