-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add GPU support for mac, multi gpu for linux #7
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
6fa667f
Reimplement GPU detection on Linux using lscpi -mm
triarius 564ec62
Add gpu detection for mac with system_profiler
triarius bede92a
Add test for GPU regex
triarius 6535070
Add a test for a non-empty no match
triarius 92d6d84
Add CI using github actions
triarius 03b8e96
Disable error on warnings for clippy
triarius 3c537e1
Fix obsolete github actions
triarius 496be98
Add comments to structs
triarius 11635a9
Make parsing of system_profiler more testable
triarius 360d0e0
Refactor gpu to format after gathering data
triarius 930ea00
Add tests for lspci and system_profiler outputs
triarius File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
on: [push, pull_request] | ||
|
||
name: Continuous integration | ||
|
||
jobs: | ||
check: | ||
name: Check | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Install Rust (Stable) | ||
run: | | ||
curl -sSL https://sh.rustup.rs | sh -s -- -y | ||
rustup default stable | ||
- name: Cargo Check | ||
run: cargo check | ||
|
||
test: | ||
name: Test Suite | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Install Rust (Stable) | ||
run: | | ||
curl -sSL https://sh.rustup.rs | sh -s -- -y | ||
rustup default stable | ||
- name: Cargo Test | ||
run: cargo test | ||
|
||
fmt: | ||
name: Rustfmt | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Install Rust (Stable) | ||
run: | | ||
curl -sSL https://sh.rustup.rs | sh -s -- -y | ||
rustup default stable | ||
- name: Cargo Fmt | ||
run: cargo fmt --all -- --check | ||
|
||
clippy: | ||
name: Clippy | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Install Rust (Stable) | ||
run: | | ||
curl -sSL https://sh.rustup.rs | sh -s -- -y | ||
rustup default stable | ||
- name: Install Clippy | ||
run: rustup component add clippy | ||
- name: Cargo Clippy | ||
# TODO: fix warnings and enable | ||
run: cargo clippy # -- -D warnings |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I'm thinking
unwrap()
here might not be the best option - even though it does seem unlikely to fail since the regex has been tested already. If it does, however, omitting routines dependent onGPU_RE
could be an option.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.
Generally, I too am averse to
unwrap()
- the docs discourage its use, although most people in the rust community don't seem to heed this.However, in this case, the error will only occur if the regex:
r#"^\S+? "(VGA|3D|Display).*?" ".*?" "(?P<gpu>.*?)""#
is not well-defined, not if there is an error when matching any particular string. Because the regex is a constant string, the value ofRegex::new(...)
will either always be anErr
, or always be anOk
. There is no variation in runtime behaviour. So pushing the error handling down to the consumers ofGPU_RE
at runtime is a waste. (Incidentally, the fact that the regex is constant is also why I'm using thelazy_static
macro to ensure it's only compiled once.)Ideally, this is something that could be detected at compile time, but it might be outside the design of the compiler to check this. I think this is the next best thing: any attempt to use this module will break, even if no methods from the module are invoked. One way to make this more robust is to have a test that asserts that the regex does not panic. I suspect any test that has
use crate::scanner
in it will achieve this. I think I might verify this and add one.LMK what you think.