Add GPU support for mac, multi gpu for linux

[triarius]

Used system_profiler to obtain the GPU name in macOS. I've only tested it on a M1 MacBook Air, it givs the output "Apple M1".

For linux, I dug a little into the data that lspci gives back instead of just presenting the entire line. On my machine, it previously gave:

GPU: 01:00.0 3D controller: NVIDIA Corporation GA107M [GeForce RTX 3050 Ti Mobile] (rev a1)

But now it is just

GPU: GA107M [GeForce RTX 3050 Ti Mobile]

which I think is more in line with what users expect.

Also, in both macOS and linux, multiple gpus will be returned if the respective commands detect them.

You can see this in the screenshot from my system where the new behaviour is on top, and the old is below.

Some implementations notes

I used serde_json to parse the output of system_profiler, but a regex for lspci -mm. This could be improved by finding an approproite parser for the ouput of lspci -mm, but it's a mixture of quoted and unquoted space seperated fields. Perhaps some csv style parser will work, but I didn't try too hard to find one. According to the man, the fields are quoted "if necessary". So, the regex implementation will break if some of the quoted fields are not quoted. However, I think we can ship this and fix it if we find reports of failure.

To allow mutiple GPUs, I collected the gpu names into a vector and then joined them with a newline separator. There is no need to allocate that vecotor, but I think it's a microoptimisation to improve it. It's highly likely that the number of GPUs is one or two, so improveing this is not going to yeild a massive performance improvement.

[nidnogg]

Heya! I wasn't aware that lscpi provided machine-readable input, nice catch. This should prevent a fair bit of edge cases. I agree with the expected output, especially on Linux.

Regarding macOS, for m1 machines I have seen just Apple M1 pop up before, and I think it might be the suitable output, after all.

I left some comments in the PR review, mostly out of curiosity. I'll be testing the changes out on my machines and seeing how it goes. And as always, thanks for chipping in! ✌️ Hoping to merge these soon.

[nidnogg]

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 on GPU_RE could be an option.

[triarius]

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 of Regex::new(...) will either always be an Err, or always be an Ok. There is no variation in runtime behaviour. So pushing the error handling down to the consumers of GPU_RE at runtime is a waste. (Incidentally, the fact that the regex is constant is also why I'm using the lazy_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.

[triarius]

@nidnogg I've verified that the tests fail if regex is not well defined:

🕙 15:39:07 ❯ cargo test
   Compiling zeitfetch v0.1.6 (/home/narthana/devel/triarius/zeitfetch)
    Finished test [unoptimized + debuginfo] target(s) in 0.50s
     Running unittests src/main.rs (target/debug/deps/zeitfetch-dfb92a9cf192c76e)

running 3 tests
test scanner::tests::gpu_regex_empty ... FAILED
test scanner::tests::gpu_regex_nvidia ... FAILED
test scanner::tests::gpu_regex_not_gpu ... FAILED

failures:

---- scanner::tests::gpu_regex_empty stdout ----
thread 'scanner::tests::gpu_regex_empty' panicked at 'called `Result::unwrap()` on an `Err` value: Syntax(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
regex parse error:
    ^\S+? "(VGA|3D|Display.*?" ".*?" "(?P<gpu>.*?)"
           ^
error: unclosed group
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
)', src/scanner.rs:26:74
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- scanner::tests::gpu_regex_nvidia stdout ----
thread 'scanner::tests::gpu_regex_nvidia' panicked at 'Once instance has previously been poisoned', /home/narthana/.local/share/cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/lazy_static-1.4.0/src/inline_lazy.rs:30:16

---- scanner::tests::gpu_regex_not_gpu stdout ----
thread 'scanner::tests::gpu_regex_not_gpu' panicked at 'Once instance has previously been poisoned', /home/narthana/.local/share/cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/lazy_static-1.4.0/src/inline_lazy.rs:30:16


failures:
    scanner::tests::gpu_regex_empty
    scanner::tests::gpu_regex_not_gpu
    scanner::tests::gpu_regex_nvidia

test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

The GitHub actions won't run on this repo, but they seem to work on my fork: https://github.com/triarius/zeitfetch/actions/runs/4645677080.

They will probably start to run once you've merged this in.

[nidnogg]

@triarius this looks pretty much good to. There's just one comment I'd like some clarification on - it's the SPDisplay used as a name for a few structs. I'm not sure what SP stands for there. Could you rename structs and consts using that name? Other than that, looks good to me!

[triarius]

I'm not sure what SP stands for there. Could you rename structs and consts using that name?

@nidnogg The SP initialism is for the macOS command system_profiler. I guess I could name them something else, but I think it's best to keep the connection in the names. I've added some comments about it, though. And I've included some realistic test data, so the format of the output is recorded in the tests.