Skip to content

Support custom OS defined in Root Source File #3796

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

Closed
wants to merge 2 commits into from
Closed

Support custom OS defined in Root Source File #3796

wants to merge 2 commits into from

Conversation

pixelherodev
Copy link
Contributor

No description provided.

@andrewrk
Copy link
Member

Thanks for starting the effort of #3784!

Is this the project for me to play with to see how this works?
https://git.sr.ht/~pixelherodev/LIMNOS

@pixelherodev
Copy link
Contributor Author

Not quite - I haven't pushed the source for that just yet. Actually, https://github.com/pixelherodev/indomitable should contain an example in src/os/limnos.zig (it's a more up-to-date version of the repo you linked, embedded as the backend for a game)

@pixelherodev
Copy link
Contributor Author

A few things of note: this only supports DWARF debugging info at the moment, and the only parts of the standard library that I can guarantee work with a custom OS layer are the file system and, possibly, debug info (worked a while back, broke at some point, not sure if that's a me problem or a regression in the library / compiler). That said, I'm not sure what other OS-dependent stuff could be supported (threading / mutexes, in theory?), and I haven't tested anything I didn't personally need.

@pixelherodev
Copy link
Contributor Author

What's needed before merging this? Tests? Examples in the docs? Broader support?

@@ -52,6 +52,9 @@ pub const direct_allocator = page_allocator;

const PageAllocator = struct {
fn alloc(allocator: *Allocator, n: usize, alignment: u29) error{OutOfMemory}![]u8 {
if (builtin.os == .freestanding) {
@compileError("PageAllocator is unusable in freestanding mode!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/unusable/unavailable/

@@ -51,7 +51,7 @@ test "" {
}

/// When linking libc, this is the C API. Otherwise, it is the OS-specific system interface.
pub const system = if (builtin.link_libc) std.c else switch (builtin.os) {
pub const system = if (@import("root") != @import("std") and @hasDecl(@import("root"), "os")) @import("root").os else if (builtin.link_libc) std.c else switch (builtin.os) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put const root = @import("root") on another line?

@@ -310,7 +310,7 @@ pub fn read(fd: fd_t, buf: []u8) ReadError!usize {
EINTR => continue,
EINVAL => unreachable,
EFAULT => unreachable,
EAGAIN => if (std.event.Loop.instance) |loop| {
EAGAIN => if (std.io.mode == .evented) if (std.io.mode == .evented) if (std.event.Loop.instance) |loop| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you added this twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it once, then did a find + replace in the document. Whoops

@@ -11,7 +11,8 @@ pub usingnamespace switch (builtin.os) {
.netbsd => @import("bits/netbsd.zig"),
.wasi => @import("bits/wasi.zig"),
.windows => @import("bits/windows.zig"),
else => struct {},
else => if (@hasDecl(@import("root"), "os") and @hasDecl(@import("root").os, "bits")) @import("root").os.bits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems sort of weird to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How so?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like.... if you wanted to import os.bits into your os struct, you would just do it yourself

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not what this does - this doesn't import std.os.bits into the custom os struct, it imports the custom os struct's bits struct instead of e.g. bits/windows.zig

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the main use of os/bits.zig to just be used in os.zig as: pub usingnamespace @import("os/bits.zig");?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but if there's a reason to keep e.g. bits/linux.zig separate from linux.zig itself, then that reasoning applies equally to custom OS layers.

@andrewrk
Copy link
Member

andrewrk commented Dec 2, 2019

Thanks! This PR was really helpful when implementing ad214c7. I did some slightly different stuff, check it out, and let me know if you'd like to see any more improvements.

I sent you a PR, tested and verified that it works with the above commit: https://github.com/pixelherodev/Indomitable/pull/1

@andrewrk andrewrk closed this Dec 2, 2019
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