-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: display sys infos #46
Conversation
mac is also a Unix ;-)
I've modified a few thing on the Linux side of things. Overall, I think this is a cool feature :) I'm not sure if /proc/cpuinfo (meminfo) is available on all Linuxes (there are plenty...) but we can fix that later on I guess, if somebody really needs it. |
Thanks for improving & testing it 👍 Totally agree, if there is a distro where its missing we will fix this in the future. |
} | ||
|
||
fn exec(allocator: std.mem.Allocator, args: []const []const u8) ![]const u8 { | ||
const stdout = (try std.ChildProcess.exec(.{ .allocator = allocator, .argv = args })).stdout; |
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 works when compiled in Debug and ReleaseFast mode but causes an "unreachable code" exception in ReleaseSafe. Haven't found out why yet...
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.
Most likely that happens due to a failed assertion or an error that is not properly handled. I don't really know the zig internals for handling this case in ReleaseSafe
.
const result = std.ChildProcess.exec(.{ .allocator = allocator, .argv = args }) catch |err| {
return err;
};
const stdout = result.stdout;
`
``
Sorry for being late to the party! First of all this a cool feature. I've skimmed through the code but I'll need more time to digest some of it because I'm not too familiar with platform-specific problems like this. I did test it on my Linux machine and it works fine, however when I quickly tested it on my Windows machine I got the compilation errors Error message
Which point to an error on this line and shouldn't even be related to this PR I'm pretty sure 😅 I'll look into this later when I get some time |
I've been gone for a while so I haven't been up to date with the latest issues, my bad! Yeah I'll sort this out ASAP (I'm confused how the PR that introduced the change passed the CI though? 🤔) |
good point; I'm not sure what exactly edit: it's Windows Server. Historically, those have been quiet different compared to "normal" Windows OS |
Alright #51 should sort the windows issue out, once that gets merged we should be able to test this feature on windows. |
Implement enhanced system information retrieval for Linux, macOS, and Windows within
platform.zig
. This includes accurate fetching of CPU name, core count, and total memory.Tested on: