libfprint-2-tod1-broadcom: init at 5.12.018

[pitkling]

Description of changes

Added a linux driver for broadcom fingerprint readers (used, e.g., in DELL Latitude 7440 laptops) to use with libfprint-tod.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 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
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[pitkling]

Note: I just saw that there is a PR draft for an older version of this driver, see #246503. As @totoroot mentions there, he hit a roadblock concerning the loading of the binary firmware files which the driver expects in a fixed location (outside /nix/store). My PR handles that by redirection fopen() calls to the corresponding location in the /nix/store (very similar to what was done in #260715 in another context).

[pitkling]

As indicated by the nixpkgs-check-by-name check that was/is failing, I moved the driver to pkgs/by-name/li/libfprint-2-tod1-broadcom/package.nix. However, the check is still failing, now stating that This attribute defined by […]/libfprint-2-tod1-broadcom/pacakge.nix is not a derivation. However, I think it is? Any hints on what I'm doing wrong?

[pitkling]

Ok, I fixed the nixpkgs-check-by-name check (I misinterpreted the check's output and was not aware that "by-name" packages by default are not required to be added to pkgs/top-level/all-packages.nix).

[nixos-discourse]

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

https://discourse.nixos.org/t/need-some-help-getting-a-new-driver-working-for-fprintd/31157/2

[nixos-discourse]

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/4252

[gador]

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 326272 run on x86_64-linux 1

1 package built:

• libfprint-2-tod1-broadcom

[gador]

awesome, lgtm. Could you just squash your commits?

[pitkling]

Thanks for the feedback, @gador and @SuperSandro2000. Should all be addressed.

[gador]

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 326272 run on x86_64-linux 1

1 package built:

• libfprint-2-tod1-broadcom

[totoroot]

@pitkling Thanks for working on this! I tried the changes on your branch, but I'm getting a test failure locally:

error: builder for '/nix/store/9gjlwh13fr53x9nsr0zmgg13ifdwxki7-fprintd-tod-1.90.9.drv' failed with exit code 12;
       last 10 log lines:
       > 153/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify_script                                                     FAIL            0.44s   exit status 1
       >
       > Ok:                 148
       > Expected Fail:      0
       > Fail:               12
       > Unexpected Pass:    0
       > Skipped:            0
       > Timeout:            0
       >
       > Full log written to /build/source/build/meson-logs/testlog.txt
       For full logs, run 'nix log /nix/store/9gjlwh13fr53x9nsr0zmgg13ifdwxki7-fprintd-tod-1.90.9.drv'.
error: 1 dependencies of derivation '/nix/store/byqsadln65n4v8h38k5sj2mvw1f96kfa-chfn.pam.drv' failed to build
error: 1 dependencies of derivation '/nix/store/i0lg3lhlyn80zh1w0agi636m3zmp11nd-dbus-1.drv' failed to build
error: 1 dependencies of derivation '/nix/store/5gnwb40h0fyy2sbwjjlbjn9lpk8sf59h-system-path.drv' failed to build
error: 1 dependencies of derivation '/nix/store/0figcyj70qsbli9j656r3b566x1khyja-nixos-system-xxx-24.11.20240712.7e7c39e.drv' failed to build

Partial output of nix log:

----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/source/tests/test_fprintd_utils.py", line 203, in setUp
    self.setup_device()
  File "/build/source/tests/test_fprintd_utils.py", line 90, in setup_device
    self.device_path = self.obj_fprintd_mock.AddDevice(
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/01mps72j158sz693xl6sjcixaz6wbld7-python3.12-dbus-python-1.3.2/lib/python3.12/site-packages/dbus/proxies.py", line 141, in __call__
    return self._connection.call_blocking(self._named_service,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/01mps72j158sz693xl6sjcixaz6wbld7-python3.12-dbus-python-1.3.2/lib/python3.12/site-packages/dbus/connection.py", line 634, in call_blocking
    reply_message = self.send_message_with_reply_and_block(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.InvalidArgs: Invalid arguments: More items found in D-Bus signature than in Python arguments

----------------------------------------------------------------------
Ran 1 test in 0.288s

FAILED (errors=1)


[154-160/160] 🌔 fprintd:dist / check-translations                                                                                                                                          0/30s^M154/160 fprintd:>
[155-160/160] 🌕 fprintd:dist / check-translations                                                                                                                                          0/30s^M155/160 fprintd:>
[156-160/160] 🌖 fprintd:dist / check-translations                                                                                                                                          0/30s^M156/160 fprintd:>
[157-160/160] 🌗 fprintd:daemon+fprintd+FPrintdVirtualDeviceVerificationTests / FPrintdVirtualDeviceVerificationTests.test_multiple_verify                                                  2/30s^M157/160 fprintd:>
[158-160/160] 🌘 fprintd:daemon+fprintd+FPrintdVirtualDeviceTest / FPrintdVirtualDeviceTest.test_verify_done_disconnect                                                                     2/30s^M158/160 fprintd:>
[159-160/160] 🌑 fprintd:daemon+fprintd+FPrintdVirtualDeviceTest / FPrintdVirtualDeviceTest.test_verify_done_disconnect                                                                     2/30s^M[159-160/160] 🌒>
[160/160] 🌓 fprintd:daemon+fprintd+FPrintdVirtualDeviceTest / FPrintdVirtualDeviceTest.test_verify_done_disconnect                                                                         2/30s^M[160/160] 🌔 fpr>

Summary of Failures:

133/160 fprintd:+test_fprintd_utils+TestFprintdUtils / TestFprintdUtils.test_fprintd_delete                                                                        FAIL            0.47s   exit status 1
135/160 fprintd:+test_fprintd_utils+TestFprintdUtils / TestFprintdUtils.test_fprintd_list                                                                          FAIL            0.45s   exit status 1
136/160 fprintd:+test_fprintd_utils+TestFprintdUtils / TestFprintdUtils.test_fprintd_enroll                                                                        FAIL            0.51s   exit status 1
137/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_list_all_fingers                                                  FAIL            0.42s   exit status 1
143/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_multiple_verify_fails                                             FAIL            0.48s   exit status 1
144/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify_any_finger_no_identification                               FAIL            0.41s   exit status 1
146/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify_any_finger_identification                                  FAIL            0.47s   exit status 1
147/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify                                                            FAIL            0.56s   exit status 1
148/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify_not_enrolled_fingers                                       FAIL            0.40s   exit status 1
150/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify_no_enrolled_fingers                                        FAIL            0.51s   exit status 1
151/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify_enrolled_fingers                                           FAIL            0.53s   exit status 1
153/160 fprintd:+test_fprintd_utils+TestFprintdUtilsVerify / TestFprintdUtilsVerify.test_fprintd_verify_script                                                     FAIL            0.44s   exit status 1

Ok:                 148
Expected Fail:      0
Fail:               12
Unexpected Pass:    0
Skipped:            0
Timeout:            0

Full log written to /build/source/build/meson-logs/testlog.txt

Any idea why this is happening?

[pitkling]

@gador Sorry for the re-requested review, but Github says I didn't address some of your requested changes although I think I did. Probably because of a recent force-push? I assume I should have not force-pushed my recent changes but squashed them later on…

[gador]

@gador Sorry for the re-requested review, but Github says I didn't address some of your requested changes although I think I did. Probably because of a recent force-push? I assume I should have not force-pushed my recent changes but squashed them later on…

All my concerns are addressed, thanks! I leave it up to @jtojnar to merge this, as I think the discussion with the installPhase isn't yet resolved?

[pitkling]

@gador @jtojnar

Just wanted to ping again, to see if there's anything else I can do or if this can be merged? I think @gador was happy with the current state and at least I think I addressed everything @jtojnar mentioned. But no hurry if anyone of you still needs more time to take a closer look at this.

In fact, I did just push two (in my view) very minor changes. The force push was just to flip the commit order and have the changes in maintainer-list.nix as the first commit (as requested in the contribution guidelines). I left the two actual changes as two separate commits, so that it is easier to see what I did; let me know if I should squash them. Quick summary of the two new commits:

• The first one adds a filter in the src definition of the wrapperLib derivation to avoid unnecessary files in the corresponding nix store path.
• The second commit uses the name attribute to (imho) improve the names of the generated store paths slightly.

[gador]

I think @gador was happy with the current state and at least I think I addressed everything @jtojnar mentioned.

Yes.

* The second commit uses the `name` attribute to (imho) improve the names of the generated store paths slightly.

I would advice against it. There is a PR from 2018 to generalize your idea (of including version attribute in the store name) (#49862) and even an RFC (NixOS/rfcs#171) which has some pro and cons (e.g. NixOS/rfcs#171 (comment)) but genereally I would like to see the package namining scheme unchanged for this PR.

This can be changed tree-wide if the RFC ever gets accepted.

[pitkling]

* The second commit uses the `name` attribute to (imho) improve the names of the generated store paths slightly.

[…] I would advise against it. There is a PR from 2018 to generalize your idea (of including version attribute in the store name) (#49862) and even an RFC (NixOS/rfcs#171) which has some pro and cons (e.g. NixOS/rfcs#171 (comment)) but genereally I would like to see the package namining scheme unchanged for this PR. […]

Ah, I didn't know this, sorry. Thanks for the quick response! I just added a commit that sticks again to the pname and version attributes instead of including the version manually in name. So together, the last two commits now simply remove some unnecessary parenthesis and add "wrapper-lib" into the name of the wrapper-lib's src attribute.

[gador]

@pitkling perfect, thanks. Maybe juist squash the last two commits?

[pitkling]

@pitkling perfect, thanks. Maybe juist squash the last two commits?

Sure, done.

[gador]

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 326272 run on x86_64-linux 1

1 package built:

• libfprint-2-tod1-broadcom

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

libfprint-2-tod1-broadcom:

warning: EvalError
Cannot evaluate attribute ‘libfprint-2-tod1-broadcom’ in ‘/nix/store/n5f84ql9vgyrrlw7xggw5p75q05jv0l5-nixpkgs’.