Update Python dependencies for Nix

[sigprof]

Description

Update Python dependencies for Nix to account for the recent changes:

• The qmk console command implementation now requires the hid and pyusb Python modules.
• The Wave Python dependency was a mistake; it was already removed from requirements-dev.txt, but still left in nix/pyproject.toml.
• The build scripts now complain that bin/qmk is deprecated, therefore the QMK CLI should be provided in the nix-shell environment.

The hid and pyusb Python modules required adding some overrides to shell.nix to make them actually work (without those overrides the packages seem to build, but break at runtime, because they cannot find the needed native libraries in Nix-specific directories). The override for pyusb was copied from nixpkgs; the override for hid is written by me based on the pyusb example.

I tested these changes on NixOS (x86_64-linux); the override for hid might also work on MacOS, but I cannot actually test it. With nix-direnv things like qmk compile or qmk info -m transparently work in the qmk_firmware directory or any subdirectories without any other configuration.

There is one potential problem that I noticed: The Nix-generated wrapper for the qmk executable from QMK CLI prepends some directories to $PATH before running the actual Python code, and one of those directories contains a python3 executable which does not have the Python module path configured to find the packages included in the Python environment. The QMK CLI itself still works because of yet another Nix-specific modification which adds the required directories to the Python path in the executable script itself; however, when that qmk executable finally calls make, any calls to bin/qmk from makefiles would fail, because bin/qmk would try to use that unconfigured python3 interpreter instead of the one that is provided by the nix-shell environment. One such error was recently fixed in #12832; in the nix-shell environment it would be a hard error instead of just a warning. It is possible to make a workaround for this in shell.nix, but I'm not sure if the added complexity is worth it, especially if it's just for supporting some broken code.

Hope that other people who touched the Nix-specific code recently (@purcell, @andresilva, @pepyakin) could test and add their comments on this.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[sigprof]

It turned out that one bin/qmk usage still remained in some obscure place; #12856 fixes that problem.

Also shell.nix is currently just broken in the develop branch without at least part of these changes — this is what happens in nix-shell with the unmodified develop code:

❯ ./bin/qmk 
Warning: The bin/qmk script is being deprecated. Please install the QMK CLI: python3 -m pip install qmk
Traceback (most recent call last):
  File "./bin/qmk", line 103, in <module>
    main()
  File "./bin/qmk", line 84, in main
    import qmk.cli  # noqa
  File "/home/vsu/Projects/qmk/qmk_firmware/lib/python/qmk/cli/__init__.py", line 15, in <module>
    from . import console
  File "/home/vsu/Projects/qmk/qmk_firmware/lib/python/qmk/cli/console.py", line 9, in <module>
    import hid
ModuleNotFoundError: No module named 'hid'

(However, this PR is not compatible with the code in the master branch at the moment, because bin/qmk is still used there, therefore the qmk command won't work there without some workaround.)

[sigprof]

In case keeping bin/qmk working is desirable, the workaround which I came up with is sigprof@928b945; with the addition of that change these Nix updates could even be merged into master in its current state (where bin/qmk is used everywhere).

[purcell]

Thanks, this looks thoroughly reasonable, though I'm -1 on the fiddly further workaround in sigprof@928b945, which I think we can/should arrange to be unnecessary by ensuring that bin/qmk is not referenced. So I think we can proceed with this PR in its current form.

(Separately we should upstream the overrides to poetry2nix, and then use that directly via niv too: it periodically gets synched into nixpkgs itself, so using it directly gets us the latest overrides more directly. Once upstreamed, the overrides here can be deleted. Ideally we shouldn't need any local overrides at all.)

[purcell]

Overrides submitted upstream in nix-community/poetry2nix#300

[purcell]

Here's a follow-up patch you can add to your PR which eliminates the local overrides now that they are upstreamed:

https://gist.github.com/56f4434e3514e37fd0b88e34f0e04575

git am <(curl https://gist.githubusercontent.com/purcell/56f4434e3514e37fd0b88e34f0e04575/raw/be67363864f8b99ad82b7911bfce07f354c5de46/0001-Use-upstreamed-poetry2nix-overrides-for-hid-and-pyus.patch)

[sigprof]

https://gist.github.com/56f4434e3514e37fd0b88e34f0e04575

Thanks, applied your patch, and now shell.nix appears to work without any local overrides, including the new qmk console part.

[purcell]

Looks very good to me 👍

[sigprof]

OK, I updated the Python dependencies, but apparently I missed another problem: qmk doctor does not work, because it actually attempts to run bin/qmk during its checks 😞

❯ qmk doctor                                
Ψ QMK Doctor is checking your environment.
Ψ QMK home: /home/vsu/Projects/qmk/qmk_firmware
Ψ Detected Linux.
☒ Can't run `bin/qmk --version`

Would you like to install dependencies? [Y/n] n

Ψ Found arm-none-eabi-gcc version 10.2.1
Ψ Found avr-gcc version 8.4.0
Ψ Found avrdude version 6.3
Ψ Found dfu-util version 0.10
Ψ Found dfu-programmer version 0.7.2
Ψ Submodules are up to date.
Ψ Major problems detected, please fix these problems before proceeding.
Ψ Check out the FAQ (https://docs.qmk.fm/#/faq_build) or join the QMK Discord (https://discord.gg/Uq7gcHh) for help.

./bin/qmk doctor succeeds, but complains that bin/qmk is deprecated.

So apparently some workaround for calling bin/qmk from QMK CLI is still needed (or the bin/qmk check in lib/python/qmk/os_helpers/__init__.py should be removed, because bin/qmk is deprecated anyway, and if the code performing this check is running at all, then we definitely have the ability to run that Python code somehow). The qmk doctor command is still somewhat useful for NixOS, because it can detect missing udev rules (although the suggested solution won't work for NixOS, because /etc/udev/rules.d is not writable — udev rules need to be configured using services.udev.packages in the NixOS configuration).

Another command which fails is qmk new-keyboard — the problem is #!/bin/bash in util/new_keyboard.sh:

❯ qmk new-keyboard
<class 'FileNotFoundError'>
☒ [Errno 2] No such file or directory: 'util/new_keyboard.sh'
Traceback (most recent call last):
  File "/nix/store/z3i5lgj08x014pz6kampg1jpd0agici7-python3-3.8.8-env/lib/python3.8/site-packages/milc/milc.py", line 467, in __call__
    return self.__call__()
  File "/nix/store/z3i5lgj08x014pz6kampg1jpd0agici7-python3-3.8.8-env/lib/python3.8/site-packages/milc/milc.py", line 472, in __call__
    return self._entrypoint(self)
  File "/home/vsu/Projects/qmk/qmk_firmware/lib/python/qmk/cli/new/keyboard.py", line 11, in new_keyboard
    cli.run(['util/new_keyboard.sh'], capture_output=False)
  File "/nix/store/z3i5lgj08x014pz6kampg1jpd0agici7-python3-3.8.8-env/lib/python3.8/site-packages/milc/milc.py", line 142, in run
    return subprocess.run(command, **kwargs)
  File "/nix/store/v3bj7jrns4sk6yj2rp30p6v2l7p707az-python3-3.8.8/lib/python3.8/subprocess.py", line 493, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/nix/store/v3bj7jrns4sk6yj2rp30p6v2l7p707az-python3-3.8.8/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/nix/store/v3bj7jrns4sk6yj2rp30p6v2l7p707az-python3-3.8.8/lib/python3.8/subprocess.py", line 1706, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'util/new_keyboard.sh'

There are many other scripts which also start with #!/bin/bash; if we want to make them compatible with NixOS, that line would need to be replaced with #!/usr/bin/env bash (NixOS has only /bin/sh and /usr/bin/env in standard locations, and everything else can be found only somewhere in $PATH).

[purcell]

qmk doctor does not work, because it actually attempts to run bin/qmk during its checks 😞

Sounds like it shouldn't do that? I wouldn't consider that a blocker for this PR at least.

if we want to make them compatible with NixOS, that line would need to be replaced with #!/usr/bin/env bash

I think it would be entirely reasonable to make that change tree-wide in a separate PR for NixOS compatibility, or use #!/bin/bash for scripts that don't use any bashisms.

Overall what I'm getting from this is that there's not necessarily more to be done in the PR, and that it is a step forward in terms of having things working correctly via Nix, so I feel we can safely go ahead and merge.

[moben]

Some testing feedback:
I gave this a shot today to build for my kyria on NixOS unstable and make kyria:moben:flash worked without issues inside nix-shell again. 👍

[purcell]

I don't see any reason this PR couldn't be merged for now: it at least moves the current state forwards for us Nix users.

(but I'm not sure what are the actual limits on the free usage of that service, apart from the 5 GB storage size limit — it seems to count the monthly bandwidth usage, but I have not been able to find what limit is imposed on free accounts).

I don't think there's any limit there for open source projects which we would need to worry about. I've got a cachix cache for another popular github project which seems to routinely serve terabytes (apologies to Domen). I'd quite like to add a secondary GitHub Actions workflow here that would build qmk with cachix enabled, and serve to give an automated check for the nix build rules.

[sigprof]

I'd quite like to add a secondary GitHub Actions workflow here that would build qmk with cachix enabled, and serve to give an automated check for the nix build rules.

I did exactly that, but in my own repository (currently the workflow is disabled, because there is no point testing what is known to be broken); on the first run it populated the cache (the build took about 2 hours on ubuntu-latest machines, and about 3 hours on macos-latest). When this PR gets merged, I intend to reenable the workflow, so that I would notice any new breakage (maybe I should also watch for new releases of the qmk_cli and milc packages; unfortunately, the GitHub “watch” feature does not work for qmk_cli, because that project does not have real releases, only tags).

BTW, another option that could be possible in theory is to build that qmk_cli as a separate Nix package, which then could be used as on more traditional systems (install qmk using the “normal” package manager, then run qmk setup to clone the qmk_firmware repository, then run qmk compile and qmk flash as needed). However, this won't work with Nix as well as it could work with more traditional packaging systems:

1. Installing the qmk package into your profile won't make tools like git, make, or the compilers available, as it would do in a traditional system (unless you republish the binaries in the qmk package with a low priority, but that would look like a gross hack). Although commands like qmk compile could still work, because the qmk wrapper could set $PATH appropriately to make build tools and compilers available, the QMK source code would become like a “black box” that could be interacted with only through the qmk tool, which I don't really like. And at the moment qmk does not really wrap git, except for the initial git clone.

   One possible workaround could be adding a qmk shell command, which would amount to export PATH=...; cd $QMK_HOME; $SHELL.

2. Making the qmk wrapper provide also git, make and other build tools in its environment would mean that the versions of those tools would be frozen — they would be updated only together with the qmk package. This is especially bad in case of git, because it needs to interact with the network, and outdated versions might have vulnerabilities that could be triggered remotely. So we cannot completely rely on using a frozen nixpkgs snapshot, as we do for compilers, and would need to use some other way to build the qmk wrapper that provides a way to use the current version of nixpkgs (e.g., a package overlay, or a flake with a nixpkgs input that could then be replaced).

   BTW, this might already be a problem with the current shell.nix code — we also take Python from the frozen nixpkgs snapshot, and running poetry in that environment might not be 100% secure (although that tool is needed only to maintain poetry.lock, and it is possible to use another Python environment for that). Any fetchers that are run during the build could also use an obsolete and vulnerable curl version; at least their outputs should be verified using hashes (and this is absolutely required, because, e.g, fetch-wheel.sh from poetry2nix explicitly uses curl --insecure), but the output verification does not protect against potential RCE vulnerabilities. But maybe worrying about that is above the healthy level of paranoia…

[purcell]

BTW, another option that could be possible in theory is to build that qmk_cli as a separate Nix package

Yeah, there's already a qmk in nixpkgs IIRC, no idea what state that's in. My only personal interest here is having a working nix shell for this repo tbh.

[sigprof]

Yeah, there's already a qmk in nixpkgs IIRC, no idea what state that's in.

Apparently it does not actually work: NixOS/nixpkgs#118340 (comment) (and the person who packaged it does not actually use it).

Given that QMK often changes the required versions of Python modules (and, even worse, updated versions of those modules are not really backwards compatible), the best way is to have the dependency information stored in the qmk_firmware repository itself, and make sure that it is updated timely; however, at the moment the “timely” part seems to be problematic.

[sigprof]

I found a much simpler way to make calls to bin/qmk from the QMK CLI work: sigprof@cc8f304

diff --git a/shell.nix b/shell.nix
index a04e251b5..4db217196 100644
--- a/shell.nix
+++ b/shell.nix
@@ -12,6 +12,13 @@ let
   # files if the requirements*.txt files change
   pythonEnv = poetry2nix.mkPoetryEnv {
     projectDir = ./nix;
+    overrides = poetry2nix.overrides.withDefaults (self: super: {
+      qmk = super.qmk.overridePythonAttrs(old: {
+        # Allow QMK CLI to run "bin/qmk" as a subprocess (the wrapper changes
+        # $PATH and breaks these invocations).
+        dontWrapPythonPrograms = true;
+      });
+    });
   };
 in

@purcell What do you think about this workaround? It fixes the qmk doctor command correctly, and also fixes the qmk pytest command (in that case the problem is that nose2 invoked from qmk is not able to import the test modules in lib/python/qmk/tests/, because their dependencies like milc or argcomplete are not available, so that's not even a case of invoking a deprecated executable).

I'm not sure whether this override should be upstreamed into poetry2nix though — the qmk package is not really a library, and that dontWrapPythonPrograms = true actually prevents the qmk executable from working if it is invoked outside of the Python environment like the one built by mkPoetryEnv (the environment provides yet another wrapper, which is now required). Maybe the qmk Python package should actually be built using (poetry2nix.mkPoetryApplication { ... }).dependencyEnv, but that would be a much bigger change, and a shell.nix file does not seem to be a correct place for that.

[purcell]

What do you think about this workaround?

Probably fine.

I'm not sure whether this override should be upstreamed into poetry2nix though — the qmk package is not really a library

Correct, it's just a local hack, not a widely applicable override for a package on pypi.

[dnaq]

I just want to chime in to say that since the nix-shell is currently broken in master it would be great if this could be merged in its current form.

[purcell]

I just want to chime in to say that since the nix-shell is currently broken in master it would be great if this could be merged in its current form.

Totally agree. Could somebody with the appropriate permissions help make that happen please? Perhaps @fauxpark?

[IndianBoy42]

+1 to merging this: I have tested building and it works with a few minor annoyances:

There is a warning about the which command not found, and there are warnings about bin/qmk being deprecated

But it is able to build firmware no problem regardless

[legendofmiracles]

This also works for me, without any of the annoyances.

[purcell]

Thanks @Erovia