hexops/mach

Fork 0

mach: allow setting magical declarations used by the zig standard library #360

New issue

Closed

opened 2022-06-20 09:04:25 +00:00 by dweiller · 7 comments

dweiller commented

2022-06-20 09:04:25 +00:00

(Migrated from github.com)

Grepping Zig's standard library for 'hasDecl(root' suggests that the magical declarations that might makes sense to export are:

log
log_level
scope_levels
allow_todo_in_release
panic
enable_segfault_handler
event_loop
io_mode

There are roughly in order of how clear I think it is that they be re-exported by Mach "root" package. I would default to saying all of these should be re-exported unless Mach wants to impose its own choice (e.g. maybe Mach just has its own event loop and doesn't let you use your own with std).

The other magic decls that std looks for are either os or ones used by startup code (e.g. main, _start, WinMainCTRStartup etc), which presumably don't need to be considered.

I'm not totally sure how to handle log for wasm.

The discussion in #309 says that Mach won't use magical public declarations to set things such as options, however I think it makes sense to use them for (any applicable) magical public declarations that `std` looks for. This shouldn't impose any extra cognitive overhead compared to what `std` already does and not re-exporting them may make somethings very hard; for example at the moment if you want to `std.log` with a custom logging function or logging level you have to modify `src/platform/{native,wasm}.zig`. Grepping Zig's standard library for 'hasDecl\(root' suggests that the magical declarations that might makes sense to export are: - `log` - `log_level` - `scope_levels` - `allow_todo_in_release` - `panic` - `enable_segfault_handler` - `event_loop` - `io_mode` There are roughly in order of how clear I think it is that they be re-exported by Mach `"root"` package. I would default to saying all of these should be re-exported unless Mach wants to impose its own choice (e.g. maybe Mach just has its own event loop and doesn't let you use your own with `std`). The other magic decls that `std` looks for are either `os` or ones used by startup code (e.g. `main`, `_start`, `WinMainCTRStartup` etc), which presumably don't need to be considered. I'm not totally sure how to handle `log` for wasm.

iddev5 commented

2022-06-20 11:11:06 +00:00

(Migrated from github.com)

Mach provides a log for wasm. But it cannot be that easily replaced. Providing an alternative reimplementation for the user would be complicated. Same goes for panic for wasm.

I don't see why this would be useful. Mach being a high level application framework should always provide the most sensible defaults out of the box.

Mach provides a log for wasm. But it cannot be that easily replaced. Providing an alternative reimplementation for the user would be complicated. Same goes for panic for wasm. I don't see why this would be useful. Mach being a high level application framework should always provide the most sensible defaults out of the box.

dweiller commented

2022-06-20 11:31:36 +00:00

(Migrated from github.com)

Yes, I agree that custom log and panic for wasm is not straightforward - you could either not allow it, or force the user to get it right at their own risk. I definitely think that things like logging levels should be left to the user to set though.

emidoots commented

2022-06-21 05:46:24 +00:00

(Migrated from github.com)

I think it's very important we consider each of these in isolation, not all together. Merely re-exporting all of these is probably not a good idea.

It's important that we consider the use case of each, how they apply to different platforms, and consequences of supporting them.

For logging in specific

I don't know enough about Zig's logging library, I haven't read it's source yet, but the full impact of this change is not clear to me. Presumably std.log.info, std.log.err, etc. work - but it sounds like they're not sufficient from your POV?

The other thing here is, beyond WASM support (which also needs to be considered), is this:

a custom logging function

How exactly we support this needs a lot of thought, especially defaults. Once we have an editor, for example, I can imagine it being quite useful to have logs show up in the editor. In contrast, for most release-mode applications it seems likely that you'd want logs to go to a file - because someone opening your .exe isn't going to be storing those for you in the event something goes wrong. Crash reporting is another thing in this area.

All this to say, I don't know what the right answer is here - but I'm hesitant to support a change that is native-only right now, and that we may need to break as an API in the future to support the above functionality.

I'd be very supportive of anyone interested in pitching a more clear proposal around how we could achieve such things in the future, otherwise I'll get to it later on.

I think it's very important we consider each of these in isolation, not all together. Merely re-exporting all of these is probably not a good idea. It's important that we consider the use case of each, how they apply to different platforms, and consequences of supporting them. ## For logging in specific I don't know enough about Zig's logging library, I haven't read it's source yet, but the full impact of this change is not clear to me. Presumably `std.log.info`, `std.log.err`, etc. work - but it sounds like they're not sufficient from your POV? The other thing here is, beyond WASM support (which also needs to be considered), is this: > a custom logging function How exactly we support this needs a lot of thought, especially defaults. Once we have an editor, for example, I can imagine it being quite useful to have logs show up in the editor. In contrast, for most release-mode applications it seems likely that you'd want logs to go to a file - because someone opening your `.exe` isn't going to be storing those for you in the event something goes wrong. Crash reporting is another thing in this area. All this to say, I don't know what the right answer is here - but I'm hesitant to support a change that is native-only right now, and that we may need to break as an API in the future to support the above functionality. I'd be very supportive of anyone interested in pitching a more clear proposal around how we could achieve such things in the future, otherwise I'll get to it later on.

emidoots commented

2022-06-21 05:48:17 +00:00

(Migrated from github.com)

Event loop, IO mode, etc.

This hasn't been considered yet, but is also another example where just exporting would be a mistake. We need to consider what the async event loop would look like in Mach applications, and likely force the evented IO mode to be always on.

Simply turning on evented IO right now, for example, wouldn't work AFAIK (Zig's builtin event loop wouldn't have a chance to run I believe?)

## Event loop, IO mode, etc. This hasn't been considered yet, but is also another example where just exporting would be a mistake. We need to consider what the async event loop would look like in Mach applications, and likely force the evented IO mode to be always on. Simply turning on evented IO right now, for example, wouldn't work AFAIK (Zig's builtin event loop wouldn't have a chance to run I believe?)

dweiller commented

2022-06-21 11:46:37 +00:00

(Migrated from github.com)

I can try to explain a bit more about my thoughts on logging (which is the main feature of the above I am interested in at the moment).

Logging

The current Zig standard library has logging functions that take a scope, level, format string and arguments, and then write a message using a global log function (the private decl std.log.log). The behaviour of std.log can be controlled by the above mentioned "magic" declarations of the root file (log, log_level and level_scopes).

Presumably std.log.info, std.log.err, etc. work - but it sounds like they're not sufficient from your POV?

I'm happy enough with std.log.{debug,info,warn,err} and std.log.scoped, what I think is undesirable at the moment is not being able to set log_level, level_scopes and (to a lesser degree) log.

Note that regardless of whether Mach wants to use the standard library's logging framework, it should be aware of it (and, I think, allow users to set the "magic" decls) as package dependencies may be using it.

`log`

The global log function is chosen by std.log checking for a public decl log in the compilations "root" package and falling back to std.log.defaultLog. std.log wraps this function to filter messages based on log_level and level_scopes. Mach current sets this logging function for wasm targets, but does not do anything on native targets, meaning that std.log.defaultLog is used (and cannot be overridden by users) on native targets.

I think there are a number of reasons a user might want to override the native logging function (I am not really familiar with wasm). The most obvious to me are

changing how the level/scope/message information is formatted (e.g. maybe you want to add a timestamp)
sending log output somewhere else (it defaults to stderr), like a file
do something custom for a particular scope/level

Re-exporting log on wasm adds some complication, as the function has to be suitable for the wasm target, and I'm am not sure what this entails.

Once we have an editor, for example, I can imagine it being quite useful to have logs show up in the editor. In contrast, for most release-mode applications it seems likely that you'd want logs to go to a file - because someone opening your .exe isn't going to be storing those for you in the event something goes wrong. Crash reporting is another thing in this area.

This is something I expect Mach will want to do as well. If Mach uses the standard library's logging framework, it will also capture any logging that third party packages might do (assuming they use it). The way packages would use it at the moment would be to pick a scope (this could clash with other packages at the moment, but there an issue in the Zig issue tracker for this)) or use the default scope. It's quite possible that you may not want log messages for some packages showing up in your editor, in which case I think Mach should probably not use the standard logging framework (so by default it does not capture logging from external packages), but either has a comptime/build option to use it, or (for more flexibility) provides a function the user can to set log to or use in their own custom log function that outputs messages to Mach's logging framework.

How Mach wants to handle logging will probably affect whether log should be re-exported - if Mach wants to use the standard library's logging framework, than mostly likely it won't re-export log.

`log_level` and `scope_levels`

There are 4 logging levels: .debug, .info, .warn, .err. The log_level decl is an enum that sets the minimum logging level that should be output (unless overridden by scope_levels). It defaults to .debug for Debug builds, .info for ReleaseSafe builds and .err for ReleaseFast and ReleaseSmall builds. Unless Mach allows you to set this, you will be stuck with these logging levels, even though you may want a ReleaseFast build that logs warnings.

The scope_levels decl works the same way as log_level but it is scope-specific. It is an array of std.log.ScopeLevels, which determine a minimum logging level for an individual scope, overriding log_level.

Re-exporting both of these should not be an issue for Mach, whether Mach uses std.log or not, and I can't see a reason to prevent people from picking the log_level and scope_levels. Their effect is just to remove logging calls from the program (message level and scopes are comptime parameters to std.log.log, so if they cause a logging statement to get filtered out, it should be elided from the program at comptime).

Other decls

Similar to log_level and scope_levels, allow_todo_in_release should be safe to re-export; it just chooses whether calls to std.debug.todo() are compile errors or panics in release builds.

The other four (panic, enable_segfault_handler, event_loop and io_mode) all would warrant consideration of how they might integrate with other aspect of Mach. But there are probably use cases for people who want to use Mach just for cross platform windowing and input, with a low-level application like was proposed in https://github.com/hexops/mach/issues/309#issuecomment-1140479760.

Simply turning on evented IO right now, for example, wouldn't work AFAIK (Zig's builtin event loop wouldn't have a chance to run I believe?)

I'm pretty sure that turning on io_mode = .evented would cause the event loop to be initialised on native targets at the moment (the relevant code to look at is in std/io.zig, std/event/loop.zig and std/start.zig). The event loop is started by start.zig in this case - it happens before main() gets called. On wasm, the event loop wouldn't get started.

I can try to explain a bit more about my thoughts on logging (which is the main feature of the above I am interested in at the moment). ## Logging The current Zig standard library has logging functions that take a scope, level, format string and arguments, and then write a message using a global log function (the private decl `std.log.log`). The behaviour of `std.log` can be controlled by the above mentioned "magic" declarations of the root file (`log`, `log_level` and `level_scopes`). > Presumably std.log.info, std.log.err, etc. work - but it sounds like they're not sufficient from your POV? I'm happy enough with `std.log.{debug,info,warn,err}` and `std.log.scoped`, what I think is undesirable at the moment is not being able to set `log_level`, `level_scopes` and (to a lesser degree) `log`. _Note that regardless of whether Mach wants to use the standard library's logging framework, it should be aware of it (and, I think, allow users to set the "magic" decls) as package dependencies may be using it._ ### `log` The global log function is chosen by `std.log` checking for a public decl `log` in the compilations `"root"` package and falling back to `std.log.defaultLog`. `std.log` wraps this function to filter messages based on `log_level` and `level_scopes`. Mach current sets this logging function for wasm targets, but does not do anything on native targets, meaning that `std.log.defaultLog` is used (and cannot be overridden by users) on native targets. I think there are a number of reasons a user might want to override the native logging function (I am not really familiar with wasm). The most obvious to me are - changing how the level/scope/message information is formatted (e.g. maybe you want to add a timestamp) - sending log output somewhere else (it defaults to stderr), like a file - do something custom for a particular scope/level Re-exporting `log` on wasm adds some complication, as the function has to be suitable for the wasm target, and I'm am not sure what this entails. > Once we have an editor, for example, I can imagine it being quite useful to have logs show up in the editor. In contrast, for most release-mode applications it seems likely that you'd want logs to go to a file - because someone opening your .exe isn't going to be storing those for you in the event something goes wrong. Crash reporting is another thing in this area. This is something I expect Mach will want to do as well. If Mach uses the standard library's logging framework, it will also capture any logging that third party packages might do (assuming they use it). The way packages would use it at the moment would be to pick a scope (this could clash with other packages at the moment, but there an issue in the Zig issue tracker for this)) or use the default scope. It's quite possible that you may not want log messages for some packages showing up in your editor, in which case I think Mach should probably not use the standard logging framework (so by default it does not capture logging from external packages), but either has a comptime/build option to use it, or (for more flexibility) provides a function the user can to set `log` to or use in their own custom `log` function that outputs messages to Mach's logging framework. How Mach wants to handle logging will probably affect whether `log` should be re-exported - if Mach wants to use the standard library's logging framework, than mostly likely it won't re-export `log`. ### `log_level` and `scope_levels` There are 4 logging levels: `.debug`, `.info`, `.warn`, `.err`. The `log_level` decl is an enum that sets the minimum logging level that should be output (unless overridden by `scope_levels`). It defaults to `.debug` for Debug builds, `.info` for ReleaseSafe builds and `.err` for ReleaseFast and ReleaseSmall builds. Unless Mach allows you to set this, you will be stuck with these logging levels, even though you may want a ReleaseFast build that logs warnings. The `scope_levels` decl works the same way as `log_level` but it is scope-specific. It is an array of `std.log.ScopeLevel`s, which determine a minimum logging level for an individual scope, overriding `log_level`. Re-exporting both of these should not be an issue for Mach, whether Mach uses `std.log` or not, and I can't see a reason to prevent people from picking the `log_level` and `scope_levels`. Their effect is just to remove logging calls from the program (message level and scopes are comptime parameters to `std.log.log`, so if they cause a logging statement to get filtered out, it should be elided from the program at comptime). ## Other decls Similar to `log_level` and `scope_levels`, `allow_todo_in_release` should be safe to re-export; it just chooses whether calls to `std.debug.todo()` are compile errors or panics in release builds. The other four (`panic`, `enable_segfault_handler`, `event_loop` and `io_mode`) all would warrant consideration of how they might integrate with other aspect of Mach. But there are probably use cases for people who want to use Mach just for cross platform windowing and input, with a low-level application like was proposed in https://github.com/hexops/mach/issues/309#issuecomment-1140479760. > Simply turning on evented IO right now, for example, wouldn't work AFAIK (Zig's builtin event loop wouldn't have a chance to run I believe?) I'm pretty sure that turning on `io_mode = .evented` would cause the event loop to be initialised on native targets at the moment (the relevant code to look at is in `std/io.zig`, `std/event/loop.zig` and `std/start.zig`). The event loop is started by `start.zig` in this case - it happens before `main()` gets called. On wasm, the event loop wouldn't get started.

emidoots commented

2022-06-24 20:01:33 +00:00

(Migrated from github.com)

Thanks for the detailed write-up, this makes a lot of sense. I've just merged https://github.com/hexops/mach/issues/360 and my conclusions for now are:

log_level, scope_levels, and allow_todo_in_release should be re-exported for now.
For the use-case of low-level applications, we should ensure we have a way to export arbitrary things like this (not just for the stdlib.)
We need to evaluate how async / evented-io fits into Mach (we already knew this, but as it stands at the time of writing Mach has no support for it.)
We need to evaluate how to handle logging in Mach, this can wait for a while - at least until we start to have the actual editor.

Thanks for the detailed write-up, this makes a lot of sense. I've just merged https://github.com/hexops/mach/issues/360 and my conclusions for now are: 1. `log_level`, `scope_levels`, and `allow_todo_in_release` should be re-exported for now. 2. For the use-case of low-level applications, we should ensure we have a way to export arbitrary things like this (not just for the stdlib.) 3. We need to evaluate how async / evented-io fits into Mach (we already knew this, but as it stands at the time of writing Mach has no support for it.) 4. We need to evaluate how to handle logging in Mach, this can wait for a while - at least until we start to have the actual editor.

emidoots commented

2023-08-20 05:08:55 +00:00

(Migrated from github.com)

This was fixed in a recent version of mach-core; your app.zig has its declarations re-exported in our main.zig so there is no issue.

This was fixed in a recent version of mach-core; your `app.zig` has its declarations re-exported in our `main.zig` so there is no issue.

👍 1