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

change the init-exe example to avoid confusion with ReleaseFast and std.log.info #9945

Closed
M0n7y5 opened this issue Oct 13, 2021 · 8 comments · Fixed by #11626
Closed

change the init-exe example to avoid confusion with ReleaseFast and std.log.info #9945

M0n7y5 opened this issue Oct 13, 2021 · 8 comments · Fixed by #11626
Labels
docs proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@M0n7y5
Copy link

M0n7y5 commented Oct 13, 2021

Zig Version

0.9.0-dev.1352+6d6cf5984

Steps to Reproduce

just init simple zig project and build it with -Drelease-fast=true or -Drelease-small=true

const std = @import("std");

pub fn main() anyerror!void {
    std.log.info("All your codebase are belong to us.", .{});
}

Expected Behavior

Produced binary should print All your codebase are belong to us.

Actual Behavior

It doesn't print anything. Instead it just exits immediately.

@M0n7y5 M0n7y5 added the bug Observed behavior contradicts documented or intended behavior label Oct 13, 2021
@M0n7y5 M0n7y5 changed the title [Windows] Optimization flags ReleaseFast and ReleaseSmall produce empty binary [Windows] Optimization flags ReleaseFast and ReleaseSmall produces empty binary Oct 13, 2021
@leecannon
Copy link
Contributor

leecannon commented Oct 13, 2021

Only log messages equal to or higher than the log level will be printed and the default log level of release modes is higher than info:

zig/lib/std/log.zig

Lines 117 to 122 in e851d89

pub const default_level: Level = switch (builtin.mode) {
.Debug => .debug,
.ReleaseSafe => .notice,
.ReleaseFast => .err,
.ReleaseSmall => .err,
};

To override this in the main file add pub const log_level = std.log.Level.info

However it does make sense to still treat this as an issue as it would make sense to note this in a comment in the file generated by init-exe. As this is definitely unexpected.

@M0n7y5
Copy link
Author

M0n7y5 commented Oct 13, 2021

So this is intended behavior? Also i was trying to inspect generated binary using IDA , but for some reason generated PDB files is crashing IDA once its loaded.

@M0n7y5
Copy link
Author

M0n7y5 commented Oct 13, 2021

To override this in the main file add pub const log_level = std.log.LogLevel.info

LogLevel doesn't exist so after looking into log.zig i found out that enum is just Level.
pub const log_level = std.log.Level.info;
Now it compiles without errors.

@leecannon
Copy link
Contributor

leecannon commented Oct 13, 2021

Yes, by default info level log messages are not emited in release modes. It is unfortunate that due to this the template executable produces no output when built non-debug.

To improve this there are really only three options:

  • Add a comment to the file generated by init-exe explaining log levels (including the different defaults based on build mode), the only negitive to this is a bit of additional noise, but this has precedence with the comments in the generated build.zig
  • Use a higher log level message, it would need to be atleast err to be output on every build mode, the problem with this is a "hello world" message doesnt make sense to be logged at error or higher
  • Directly write to stdout/std.debug.print, this is what the generated file contained before being replaced with the current call to std.log, std.log is a useful system with many features (scoped messages, different log levels per scope, libraries can mindlessly use it without worrying as the application can chosse to filter its messages out, custom logging implementation) due to these prefering to use std.log in the template to prompt its usage is a good idea

@Prince213
Copy link
Contributor

I think it's an overkill to use std.log for a template executable. Maybe we should do something simple, like writing to stdout directly.

@g-w1
Copy link
Contributor

g-w1 commented Oct 14, 2021

It was decided to log because this is better scaffolding for larger projects.

@Prince213
Copy link
Contributor

If the intent is to remind users to use std.log, then it should be stated in the docs, not in a simple example program that is expected to do something simple.

@wizzard0
Copy link

wizzard0 commented Oct 23, 2021

To me, a simplest improvement would be to add a comment to the generated source file, saying "log messages with info level would not be emitted in ReleaseFast, if you intended to use stdout to output some processing results, use X (link to example)"

(edit): Also, maybe remove [windows] from the issue title? Looks cross-platform to me

@andrewrk andrewrk added docs proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. and removed bug Observed behavior contradicts documented or intended behavior labels Nov 20, 2021
@andrewrk andrewrk changed the title [Windows] Optimization flags ReleaseFast and ReleaseSmall produces empty binary change the init-exe example to avoid confusion with ReleaseFast and std.log.info Nov 20, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 20, 2021
mewmew added a commit to mewpull/zig that referenced this issue May 10, 2022
mewmew added a commit to mewpull/zig that referenced this issue May 13, 2022
jedisct1 pushed a commit that referenced this issue May 14, 2022
@Vexu Vexu modified the milestones: 0.12.0, 0.10.0 May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants