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

[v2/Mac] Add Apple Silicon hardware detection to wails doctor #3129

Merged
merged 2 commits into from
Dec 15, 2023
Merged

[v2/Mac] Add Apple Silicon hardware detection to wails doctor #3129

merged 2 commits into from
Dec 15, 2023

Conversation

almas1992
Copy link
Contributor

@almas1992 almas1992 commented Dec 14, 2023

Description

wails doctor support detect Apple Silicon hardware info

Before :

# Wails
Version | v2.7.1

# System
┌─────────────────────────┐
| OS           | MacOS    |
| Version      | 14.1.2   |
| ID           | 23B92    |
| Go Version   | go1.21.5 |
| Platform     | darwin   |
| Architecture | arm64    |
| CPU          | Unknown  |
| GPU          | Unknown  |
| Memory       | Unknown  |
└─────────────────────────┘

# Dependencies
┌──────────────────────────────────────────────────────────────────┐
| Dependency                | Package Name | Status    | Version   |
| Xcode command line tools  | N/A          | Installed | 2403      |
| Nodejs                    | N/A          | Installed | 21.4.0    |
| npm                       | N/A          | Installed | 10.2.4    |
| *Xcode                    | N/A          | Available |           |
| *upx                      | N/A          | Installed | upx 4.1.0 |
| *nsis                     | N/A          | Available |           |
└──────────────────── * - Optional Dependency ─────────────────────┘

# Diagnosis
Optional package(s) installation details:
  - Xcode: Available at https://apps.apple.com/us/app/xcode/id497799835
  - nsis : More info at https://wails.io/docs/guides/windows-installer/

 SUCCESS  Your system is ready for Wails development!

 ♥   If Wails is useful to you or your company, please consider sponsoring the project:
https://github.com/sponsors/leaanthony

After:

# Wails
Version | v2.7.1

# System
┌────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
| OS           | MacOS                                                                                                                       |
| Version      | 14.1.2                                                                                                                      |
| ID           | 23B92                                                                                                                       |
| Go Version   | go1.21.5                                                                                                                    |
| Platform     | darwin                                                                                                                      |
| Architecture | arm64                                                                                                                       |
| CPU          | Apple M1 Pro                                                                                                                |
| GPU          | Chipset Model: Apple M1 Pro Type: GPU Bus: Built-In Total Number of Cores: 14 Vendor: Apple (0x106b) Metal Support: Metal 3 |
| Memory       | 32GB                                                                                                                        |
└────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

# Dependencies
┌──────────────────────────────────────────────────────────────────┐
| Dependency                | Package Name | Status    | Version   |
| Xcode command line tools  | N/A          | Installed | 2403      |
| Nodejs                    | N/A          | Installed | 21.4.0    |
| npm                       | N/A          | Installed | 10.2.4    |
| *Xcode                    | N/A          | Available |           |
| *upx                      | N/A          | Installed | upx 4.1.0 |
| *nsis                     | N/A          | Available |           |
└──────────────────── * - Optional Dependency ─────────────────────┘

# Diagnosis
Optional package(s) installation details:
  - Xcode: Available at https://apps.apple.com/us/app/xcode/id497799835
  - nsis : More info at https://wails.io/docs/guides/windows-installer/

 SUCCESS  Your system is ready for Wails development!

 ♥   If Wails is useful to you or your company, please consider sponsoring the project:
https://github.com/sponsors/leaanthony

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using wails doctor.

  • Windows
  • macOS
  • Linux

Test Configuration

Please paste the output of wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.

Checklist:

  • I have updated website/src/pages/changelog.mdx with details of this PR
  • My code follows the general coding style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • New Features

    • Enhanced system diagnostics to include detailed CPU, GPU, and memory information for macOS users.
  • Refactor

    • Integrated an external shell dependency to improve system information retrieval processes.

Copy link
Contributor

coderabbitai bot commented Dec 14, 2023

Walkthrough

The wails project has updated its doctor.go file to enhance system information retrieval on macOS (Darwin). It now includes external shell operations, specifically targeting CPU, GPU, and memory information gathering by using system-specific commands like sysctl and system_profiler.

Changes

File Path Change Summary
v2/cmd/wails/doctor.go Added external dependency for shell operations; updated CPU, GPU, and memory information retrieval for Darwin OS.
website/src/pages/changelog.mdx Semantic change related to the addition of Apple Silicon hardware detection to wails doctor.

Poem

In the code where rabbits hop,
A new change does drop.
🍎 For Mac, it's quite clear,
System checks, now more near.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can reply to a review comment made by CodeRabbit.
  • You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
  • You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 3aaa1f8 and b3b10e6.
Files selected for processing (1)
  • v2/cmd/wails/doctor.go (3 hunks)
Additional comments: 5
v2/cmd/wails/doctor.go (5)
  • 5-5: The addition of the shell package is consistent with the PR's objective to enhance hardware detection on macOS.

  • 97-104: The new logic to fetch CPU information using sysctl for macOS is in line with the PR's objective to improve hardware detection on Apple Silicon Macs.

  • 123-147: The implementation for retrieving GPU information using system_profiler for macOS is consistent with the PR's goal to display detailed hardware information on Apple Silicon Macs.

  • 154-163: The updated logic to retrieve memory information using sysctl for macOS aligns with the PR's aim to enhance the wails doctor command for better hardware detection on Apple Silicon Macs.

  • 97-97: Ensure that the assumption that a nil return value from ghw.CPU() or ghw.GPU() implies a macOS system is valid. If not, an explicit check for macOS should be performed before running macOS-specific commands.

Also applies to: 123-123, 154-154

cpuInfo := "Unknown"
if runtime.GOOS == "darwin" {
// Try to get CPU info from sysctl
if stdout, _, err := shell.RunCommand("", "sysctl", "-n", "machdep.cpu.brand_string"); err == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding error logging or user notification if the shell commands fail to execute, to improve the user experience by providing feedback on what went wrong.

Also applies to: 126-126, 157-157

@almas1992 almas1992 changed the title [v2/Mac] Add Apple Silicon hardware detection to wails doctor [v2/Mac] Add Apple Silicon hardware detection to wails doctor Dec 14, 2023
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between b3b10e6 and d5dfbd4.
Files selected for processing (1)
  • website/src/pages/changelog.mdx (1 hunks)
Additional comments: 1
website/src/pages/changelog.mdx (1)
  • 17-19: The changelog entry for the addition of Apple Silicon hardware detection to wails doctor is correctly placed and well-described, providing a clear link to the associated PR.

Comment on lines +17 to +19
### Changed

- Add Apple Silicon hardware detection to `wails doctor`. Changed by @almas1992 in [PR](https://github.com/wailsapp/wails/pull/3129)
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate entry for the addition of hardware detection to wails doctor found. The entry under "Added" for a previous version should be removed to maintain an accurate changelog.

- - Added CPU/GPU/Memory detection for `wails doctor`. Added by @leaanthony in #d51268b8d0680430f3a614775b13e6cd2b906d1c

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
### Changed
- Add Apple Silicon hardware detection to `wails doctor`. Changed by @almas1992 in [PR](https://github.com/wailsapp/wails/pull/3129)

@leaanthony leaanthony merged commit 5f457db into wailsapp:master Dec 15, 2023
10 checks passed
@leaanthony
Copy link
Member

Thanks @almas1992 !

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.

None yet

2 participants