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

pkgs/applespi: add experimental Linux support for MacBook keyboards #57419

Closed
wants to merge 1 commit into from

Conversation

krav
Copy link
Contributor

@krav krav commented Mar 11, 2019

Motivation for this change

According to https://github.com/Dunedan/mbp-2016-linux#keyboard--touchpad, this adds support for the keyboard and touchpad in MacBookPro1[3,4],[1-3]. Confirmed working on a MacBookPro14,1.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@aanderse
Copy link
Member

@krav Are you still motivated to continue with this PR? If so please address the issue listed.

@krav
Copy link
Contributor Author

krav commented Jul 14, 2019

@aanderse Oh, I somehow missed that the check was failing. I'll fix it and see if I can't get this to build with Linux 5.2, then NixOS might actually be usable on newer Macbook Pro models.

@samueldr samueldr mentioned this pull request Jul 27, 2019
10 tasks
@samueldr
Copy link
Member

I think it'd be interesting to have either this or #51539 merged by 19.09, so that newer apple laptops can be made more reasonably usable. Though, I think #51539 is more appropriate, as it also adds an option to make it more usable.

@@ -0,0 +1,38 @@
{ stdenv, fetchgit, pkgs }:
Copy link
Member

Choose a reason for hiding this comment

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

Don't use pkgs in the parameters, always list the required packages. For kernel modules, adding kernel is the way to go.

@@ -0,0 +1,38 @@
{ stdenv, fetchgit, pkgs }:
let
version = "git";
Copy link
Member

Choose a reason for hiding this comment

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

Don't add the version in a let block. Additionally, when the package is from a version-less development release, use unstable-YYYY-MM-DD as the version number, with the date being the date of the commit.

Add pname = "applespi"; and version = "YYYY-MM-DD-${kernel.version}" to the mkDerivation call in this particular case.

stdenv.mkDerivation {
name = "applespi-${version}-${kernel.version}";

src = fetchgit {
Copy link
Member

Choose a reason for hiding this comment

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

Prefer fetchFromGitHub for GitHub, as it will fetch only the revision by using the GitHub specific ways to fetch a single commit's tree.

@krav
Copy link
Contributor Author

krav commented Aug 9, 2019

Though, I think #51539 is more appropriate, as it also adds an option to make it more usable.

I agree, also I'm not actively using this since there are too many other issues I don't have time to deal with right now. Closing this PR in favour of #51539.

@krav krav closed this Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants