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

hw-probe: init at 1.6.4 #168787

Merged
merged 3 commits into from
Aug 25, 2022
Merged

Conversation

rehno-lindeque
Copy link
Contributor

Description of changes

A tool to probe for hardware, check operability and find drivers with the help of Linux hardware database.

Results can be uploaded to the database used by https://linux-hardware.org/ (or https://bsd-hardware.info/)

hw-probe was previously mentioned on /r/NixOS: People appear to use docker to run it on NixOS.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

, edid-decode

# Recommended
, withRecommended ? false # Install recommended tools
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this as false because I noticed that a similar flag is false for inxi:

, withRecommends ? false # Install (almost) all recommended tools (see --recommends)

Not sure what best practice is

Copy link
Member

Choose a reason for hiding this comment

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

If you use the tool normally would you always want those programs? If yes, it should be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, I'll fix this

, withSuggested ? false # Install (most) suggested tools
, hplip
, sane-backends
, pnputils ? null # pnputils (lspnp) isn't current in nixpkgs and appears to be poorly maintained
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that pnputils is not available in nixpkgs and appears to have been removed from debian I think it was - for being poorly maintained

memtester
sysstat # (iostat)
cpuid
util-linuxMinimal # (rfkill)
Copy link
Contributor Author

@rehno-lindeque rehno-lindeque Apr 15, 2022

Choose a reason for hiding this comment

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

I've used util-linuxMinimal instead of util-linux to copy similar code in inxi. Not sure what best practice is

Copy link
Member

Choose a reason for hiding this comment

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

util-linuxMinimal is probably just for bootstrapping systemd and related friends. Shouldn't matter ot much which we use if minimal has the features compiled in.

recommendedPrograms = [
mcelog
hdparm
systemd # (systemd-analyze)
Copy link

Choose a reason for hiding this comment

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

I don't think it's a good idea to require installation of systemd.

The tool should probe for systemd-analyze only if systemd is already installed and used by the computer owner.

Copy link
Contributor Author

@rehno-lindeque rehno-lindeque Apr 17, 2022

Choose a reason for hiding this comment

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

Thanks @linuxhw - seeing you helpfully drop by motivates me to try and get the PR accepted.

I don't think it's a good idea to require installation of systemd.

The tool should probe for systemd-analyze only if systemd is already installed and used by the computer owner.

Fair enough, I should probably add a systemdSupport input argument to support non-NixOS use cases.

(Note that nix has a slightly different interpretation of "installation" which means something more like "make binaries available in a specific context", rather than what I would call "activation". Never-the-less your point is well taken.)


Quick suggestion that may require a separate PR to make the change treewide:

How do others feel about a nixpkgs config flag for config.systemdSupport so that the input argument can be implemented as systemdSupport ? config.systemdSupport or stdenv.isLinux?

I.e. config.systemdSupport would be similar to other nixpkgs config flags like config.cudaSupport.

I've noticed that some other packages have input flags like systemdSupport ? stdenv.isLinux or systemdSupport ? stdenv.hostPlatform.isLinux. However I've personally worked with nix + non-systemd linux, so I find that default to be a little off target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Related: There are well defined config options now #166792

@rehno-lindeque rehno-lindeque force-pushed the hw-probe-1.6.4 branch 2 times, most recently from 1fe3658 to ad9db13 Compare April 17, 2022 14:58
++ lib.optionals withRecommended recommendedPrograms
++ lib.optionals withSuggested suggestedPrograms;
in [
"--set" "PERL5LIB" "${makePerlPath [ LWP ]}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know much about perl - I'm guessing LWPProtocolHttps is not necessary since hw-probe just specifies libwww-perl (?)

@rehno-lindeque
Copy link
Contributor Author

rehno-lindeque commented Apr 17, 2022

@linuxhw Since you're around, I thought I'd just check.

I appear to be missing these logs:

missing-logs

I'm guessing these are all just due to boot flags and things of that nature rather than anything packaging related?

  • Biosdecode: Here's the binary from dmidecode
$ nix-shell -p dmidecode --run 'ls -l $(which biosdecode) ; printf "VERSION: " ; biosdecode --version'
-r-xr-xr-x 1 root root 27680 Dec 31  1969 /nix/store/vqd0a65pxla9qnas0bq7i8rfgmadizxz-dmidecode-3.2/bin/biosdecode
VERSION: 3.2
  • I2cdetect: Binary from i2c-tools. I'm guessing this is not a problem and I simply don't have any i2c busses on my laptop.
$ nix-shell -p i2c-tools --run 'ls -l $(which i2cdetect) ; printf "LIST: " ; i2cdetect -l'
-r-xr-xr-x 1 root root 30792 Dec 31  1969 /nix/store/98js1cgw8g4pa2xigqg6nn3fvwicgwwn-i2c-tools-4.3/bin/i2cdetect
LIST: 
  • mcelog: Binary from mcelog
$ nix-shell -p mcelog --run 'ls -l $(which mcelog) ; printf "VERSION: " ; mcelog --version'
-r-xr-xr-x 1 root root 166384 Dec 31  1969 /nix/store/4xgy1v8pl0sb8vnrnrv1v58kbkjs19kk-mcelog-178/bin/mcelog
VERSION: mcelog 178
  • Xorg.log: I'm not sure about this one. I think these logs are available via journalctl -b -u display-manager.service

@Luz Luz mentioned this pull request Jun 18, 2022
13 tasks
@winny-
Copy link
Contributor

winny- commented Jun 19, 2022

EDIT: Added more missing packages. For some reason hdparm results are not appearing yet.

Hello, I recently created a PR to also package hw-probe, however, decided to close it after realizing this one exists.

hw-probe is a bit of a prickly package because it looks for several tools that are assumed to be installed and are not explicitly declared as dependencies by the project. For this reason I suggest testing using a command like the following:

sudo -E env PATH= $(nix-build --pure -A hw-probe)/bin/hw-probe -all -upload

Did a little testing and found most of the perl packages needed:

LWPProtocolHttps
HTTPMessage
HTTPDate
URI

hw-probe also needs the following if ran in a pure environment:

gnutar (omitting this will manifest in a failed tar invocation being silently ignored causing a more confusing error message)
xz (or gzip)
iproute2  (or inetutils - either ip or ifconfig is needed)
coreutils (ls, sort)
gnugrep (grep)
kmod (lsmod)

Here's the branch comparison (diff) from this PR branch that includes the above changes: rehno-lindeque/nixpkgs@hw-probe-1.6.4...winny-:hw-probeIsolatedBuildFixup

Example probe using the above pure nix build incantation: https://linux-hardware.org/?probe=5f385cfbee

@rehno-lindeque
Copy link
Contributor Author

@winny- thank you for this, I'll review and cherry-pick into my branch as soon as possible.

(I'm traveling so it might be a little while)

@rehno-lindeque
Copy link
Contributor Author

Finally got around to cherry picking your additions, thanks @winny-. I'll ask on discourse for a review since no one has looked at this yet.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1126

pkgs/tools/system/hw-probe/default.nix Outdated Show resolved Hide resolved
, edid-decode

# Recommended
, withRecommended ? false # Install recommended tools
Copy link
Member

Choose a reason for hiding this comment

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

If you use the tool normally would you always want those programs? If yes, it should be true.

pkgs/tools/system/hw-probe/default.nix Outdated Show resolved Hide resolved
pkgs/tools/system/hw-probe/default.nix Outdated Show resolved Hide resolved
memtester
sysstat # (iostat)
cpuid
util-linuxMinimal # (rfkill)
Copy link
Member

Choose a reason for hiding this comment

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

util-linuxMinimal is probably just for bootstrapping systemd and related friends. Shouldn't matter ot much which we use if minimal has the features compiled in.

pkgs/tools/system/hw-probe/default.nix Outdated Show resolved Hide resolved
pkgs/tools/system/hw-probe/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@rehno-lindeque
Copy link
Contributor Author

rehno-lindeque commented Aug 22, 2022

@SuperSandro2000 thank you, suggestions applied. I'm happy to squash and rebase if everything looks good?

@SuperSandro2000
Copy link
Member

Can you squash the hw-probe review commits into the init commit? Other than that LGTM.

@rehno-lindeque
Copy link
Contributor Author

@SuperSandro2000 hw-probe commits squashed, and rebased on master.

Should be good to merge now

@SuperSandro2000 SuperSandro2000 merged commit c59d300 into NixOS:master Aug 25, 2022
@FlorianFranzen
Copy link
Contributor

It seems the vulkan-utils derivation is actually empty, did you mean to use vulkan-tools?

@rehno-lindeque
Copy link
Contributor Author

rehno-lindeque commented Nov 17, 2022

Hmm, thank you for pointing that out @FlorianFranzen, this is a bit of a 🤦‍♂️ moment. I'll create a PR to remove vulkan-utils and replace the input.

I'm not 100% sure, but based on linuxhw/hw-probe@1d743ad I think it may be used for glxgears / glxinfo / etc...


@linuxhw By the way, I think that this should be vulkan-tools in the hw-probe installation instructions as well:

It seems like vulkan-utils (previously KhronosGroup/Vulkan-LoaderAndValidationLayers?) is replaced by vulkan-tools (or KhronosGroup/Vulkan-Tools)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants