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

qmk: init at 0.0.45 #118340

Merged
merged 2 commits into from
Apr 5, 2021
Merged

qmk: init at 0.0.45 #118340

merged 2 commits into from
Apr 5, 2021

Conversation

dotlambda
Copy link
Member

Motivation for this change

closes #118016

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@legendofmiracles I have no way of testing whether this works and hence don't feel comfortable being the maintainer of this package. Would you like to do that?

@bb2020
Copy link
Member

bb2020 commented Apr 2, 2021

We are actually working on this in #109639. It is still draft as there is a blocking issue about compiling custom keyboards.

Copy link
Contributor

@bhipple bhipple left a comment

Choose a reason for hiding this comment

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

I'm not sure the ability to compile custom keyboards is a hard blocker for this PR, since it's relatively straightforward to change src to your qmk fork in a NUR overlay [1], and easier to maintain if the base qmk itself is in the nixpkgs tree for overriding.

Of course if you can think of a more simple/efficient/clear way to do this that's even better.

[1] Something like

qmk = super.qmk.overrideAttrs(_: { src = /home/bhipple/src/qmk_cli; });

or a deterministic fetchFromGitHub on your qmk fork if you'd like something more stable.

pkgs/tools/misc/qmk/default.nix Outdated Show resolved Hide resolved
@ofborg ofborg bot requested a review from bhipple April 2, 2021 21:18
@legendofmiracles
Copy link
Contributor

Is this cli tool only for the clueboard?

@dotlambda dotlambda marked this pull request as ready for review April 4, 2021 11:57
@SuperSandro2000 SuperSandro2000 merged commit 4a87f88 into NixOS:master Apr 5, 2021
@dotlambda dotlambda deleted the qmk-init branch April 5, 2021 15:24
@legendofmiracles
Copy link
Contributor

@dotlambda it doesn't work.
I ran qmk and it returns this, no matter what args:

Could not find module dotty-dict!
Could not find module hjson!
Could not find module jsonschema!
Could not find module pygments!

Would you like to install the required Python modules? [y/n] y

/nix/store/5470xw15wnn972ap0c4f7q642z4nvh6f-python3-3.8.8/bin/python3.8: No module named pip
Error: %s: %s ('ModuleNotFoundError', ModuleNotFoundError("No module named 'pygments'"))
Traceback (most recent call last):
  File "/nix/store/hr3rr1s3gjdvpkd11m5dg0rghij83jxj-qmk-0.0.45/lib/python3.8/site-packages/qmk_cli/script_qmk.py", line 99, in main
    import qmk.cli  # noqa
  File "/home/nix/qmk_firmware/lib/python/qmk/cli/__init__.py", line 9, in <module>
    from . import c2json
  File "/home/nix/qmk_firmware/lib/python/qmk/cli/c2json.py", line 7, in <module>
    import qmk.keymap
  File "/home/nix/qmk_firmware/lib/python/qmk/keymap.py", line 8, in <module>
    from pygments.lexers.c_cpp import CLexer
ModuleNotFoundError: No module named 'pygments'

I do not have python installed, nor pip - so those modules should be added as dependencies.

@dotlambda
Copy link
Member Author

dotlambda commented Apr 10, 2021

@legendofmiracles You are importing stuff from /home/nix/qmk_firmware/lib/python/qmk. That's unlikely to work. Instead, someone should package qmk_firmware. See #109639.

@AndersonTorres AndersonTorres mentioned this pull request Apr 12, 2021
10 tasks
@AndersonTorres
Copy link
Member

Firmwares are not meant to be packaged as if they were libraries.

@dotlambda
Copy link
Member Author

@AndersonTorres @legendofmiracles As stated above, I don't use this software. So if some dependency needs to be added, please go ahead and make a pull request. Don't forget to add yourself as a maintainer.

@afontaine
Copy link
Contributor

@legendofmiracles @dotlambda it seems the QMK shell.nix needs an update as well, as it has a local copy (bin/qmk) that runs that is trying to import all these extra dependencies.

@afontaine
Copy link
Contributor

An updated shell.nix file from @ralsei (thanks for this!)

shell.nix
{ avr ? true, arm ? true, teensy ? true }:

let
  pkgs = import <nixos-unstable> { };

  hjson = with pkgs.python3Packages; buildPythonPackage rec {
    pname = "hjson";
    version = "3.0.1";

    src = fetchPypi {
      inherit pname version;
      sha256 = "1yaimcgz8w0ps1wk28wk9g9zdidp79d14xqqj9rjkvxalvx2f5qx";
    };

    doCheck = false;
  };

  milc = with pkgs.python3Packages; buildPythonPackage rec {
    pname = "milc";
    version = "1.0.10";

    src = fetchPypi {
      inherit pname version;
      sha256 = "1q1p7qrqk78mw67nhv04zgxaq8himmdxmy2vp4fmi7chwgcbpi32";
    };

    propagatedBuildInputs = [
      appdirs
      argcomplete
      colorama
    ];

    doCheck = false;
  };

  pythonEnv = pkgs.python3.withPackages (p: with p; [
    # requirements.txt
    appdirs
    argcomplete
    colorama
    dotty-dict
    hjson
    jsonschema
    milc
    pygments
    # requirements-dev.txt
    nose2
    flake8
    pep8-naming
    yapf
  ]);
in

with pkgs;
let
  avrlibc = pkgsCross.avr.libcCross;

  avr_incflags = [
    "-isystem ${avrlibc}/avr/include"
    "-B${avrlibc}/avr/lib/avr5"
    "-L${avrlibc}/avr/lib/avr5"
    "-B${avrlibc}/avr/lib/avr35"
    "-L${avrlibc}/avr/lib/avr35"
    "-B${avrlibc}/avr/lib/avr51"
    "-L${avrlibc}/avr/lib/avr51"
  ];
in
mkShell {
  name = "qmk-firmware";

  buildInputs = [ clang-tools dfu-programmer dfu-util diffutils git pythonEnv ]
    ++ lib.optional avr [
      pkgsCross.avr.buildPackages.binutils
      pkgsCross.avr.buildPackages.gcc8
      avrlibc
      avrdude
    ]
    ++ lib.optional arm [ gcc-arm-embedded ]
    ++ lib.optional teensy [ teensy-loader-cli ];

  AVR_CFLAGS = lib.optional avr avr_incflags;
  AVR_ASFLAGS = lib.optional avr avr_incflags;
  shellHook = ''
    # Prevent the avr-gcc wrapper from picking up host GCC flags
    # like -iframework, which is problematic on Darwin
    unset NIX_CFLAGS_COMPILE_FOR_TARGET
    export PATH="<path to qmk_firmware>/bin:$PATH"
  '';
}

@ralsei
Copy link
Contributor

ralsei commented Jun 18, 2021

You don't need the extra export PATH=... at the end if you're just willing to call bin/qmk. But I'm not.

@AndersonTorres
Copy link
Member

@afontaine if hjson and milc were packaged for Nixpkgs, it would be useful?

@afontaine
Copy link
Contributor

@afontaine if hjson and milc were packaged for Nixpkgs, it would be useful?

I think they already are?

I was able to look into things a bit more.

At some point, a kind soul refactored the qmk_firmware's shell.nix file to use poetry2nix, and I was able to update it locally and get things working using the included bin/qmk script

However, it looks like that script has been deprecated and that we will need the qmk binary add here at some point moving forward.

Fortunately, all this poetry updating almost got the qmk cli working! The only trouble is that it couldn't detect the fact that pyusb was available. I'm not a python person, so I'm not entirely sure what the issue is. It does seem as though keeping the qmk cli reasonably up-to-date might be essential to having it work smoothly with the qmk_firmware... maybe the answer here is to remove qmk from nixpkgs and instead package it in qmk_firmware? It might be easier to keep them in sync there.

@AndersonTorres
Copy link
Member

QMK is a different thing.

The workflow of a QMK hacker (?) is

  1. Download QMK Firmware source code
  2. Modify it (more accurately, only the specific parts of the code related to the specific firmware you are interested)
  3. Compile it (more accurately, only the firmware you are interested)
  4. Flash it to the keyboard/mouse/trackpad/&c.

QMK CLI is a tool that implements this workflow. Packaging qmk_firmware would not be so useful in this regard, it was not made to be distributed as an opaque binary file.

Maybe an exaggerated analogy can be useful. Think on qmk_firmware as the Linux kernel, and qmk_cli as make menuconfig.

As an aside, the state of Python packaging is confusing at least, but I would say Dantesque.

@afontaine
Copy link
Contributor

QMK CLI is a tool that implements this workflow. Packaging qmk_firmware would not be so useful in this regard, it was not made to be distributed as an opaque binary file.

I understand that. I am saying that there may be no point in packaging qmk_cli within nixpkgs either, as it would be easier to have it packaged within the existing qmk_firmware's shell.nix, and is relatively useless outside of the repository, especially as it seems the dependencies of the firmware repository and the cli seem to be kept pretty well in sync.

@legendofmiracles
Copy link
Contributor

legendofmiracles commented Jun 21, 2021

When I requested the package, I was still extremely new to nix in general. I didn't realize that they had a packaged shell.nix and my first instinct was to request the package.

Currently, I'm also using the qmk provided by the shell.nix, which was fixed yesterday.
There's no reason for this to be packaged, since the tool isn't useful without qmk_firmware, which is missing and impossible/infeasible to package.

@afontaine
Copy link
Contributor

Currently, I'm also using the qmk provided by the shell.nix, which was fixed yesterday.

is that the one available under the bin folder? I am concerned by those deprecation notices...

@AndersonTorres
Copy link
Member

and is relatively useless outside of the repository, especially as it seems the dependencies of the firmware repository and the cli seem to be kept pretty well in sync.

But there are other distros packaging it, and they are not in a different situation. QMK is useless outside the repo for all these distros, because QMK is over specialized. On the other hand, the qmk_cli tool can also download the QMK tree.

@afontaine
Copy link
Contributor

yeah that is a good point @AndersonTorres.

OK, in that case, we should look at bumping it to 0.51.0. Can we declare in the package definition that it should be tested within the qmk_firmware repo as well to ensure things are working smoothly?

@AndersonTorres
Copy link
Member

AndersonTorres commented Jun 21, 2021

Well, the qmk_cli github page says it can Interact with your qmk_firmware tree from any location (including forks). Therefore it is already documented.

Anyway I will bump the version.

@AndersonTorres
Copy link
Member

@afontaine ping

#127863

@AndersonTorres AndersonTorres mentioned this pull request Jun 24, 2021
11 tasks
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.

qmk_cli should have a nix package
8 participants