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

patch ati-drivers for kernel 4.6 #18237

Merged
merged 1 commit into from
Sep 16, 2016
Merged

Conversation

jerith666
Copy link
Contributor

Motivation for this change

fixes #18202

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • 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/)
  • Fits CONTRIBUTING.md.

this uses the patch from
https://github.com/manjaro/packages-extra/commit/ddae91f2 to account
for torvalds/linux@d4edcf0d and the patch
from https://www.virtualbox.org/ticket/15298 to account for
torvalds/linux@09cbfeaf

@mention-bot
Copy link

@jerith666, thanks for your PR! By analyzing the annotation information on this pull request, we identified @edwtjo, @heydojo and @MarcWeber to be potential reviewers

@@ -1,4 +1,4 @@
{ stdenv, fetchurl, kernel ? null, which
{ stdenv, fetchurl, kernel ? null, linux, which
Copy link
Member

Choose a reason for hiding this comment

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

linux? I'm confident one should use the existing kernel parameter instead. That can be null, but that's for case when a kernel module isn't built and such patches shouldn't be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I thought too, but when I was running my build locally, kernel was null. That seemed weird to me, although I don't see kernel being passed in explicitly where this is called (at https://github.com/jerith666/nixpkgs/blob/df9b7c1274551bc1ce0b1e71bb35e4cfbad143ae/pkgs/top-level/all-packages.nix#L11276). But I also don't understand what args callPackage is expected to fill in automatically.

At any rate, here is what I get:

$ git diff
diff --git a/pkgs/os-specific/linux/ati-drivers/default.nix b/pkgs/os-specific/linux/ati-drivers/default.nix
index d54b0ab..621d756 100644
--- a/pkgs/os-specific/linux/ati-drivers/default.nix
+++ b/pkgs/os-specific/linux/ati-drivers/default.nix
@@ -1,4 +1,4 @@
-{ stdenv, fetchurl, kernel ? null, linux, which
+{ stdenv, fetchurl, kernel ? null, which
 , xorg, makeWrapper, glibc, patchelf, unzip
 , fontconfig, freetype, mesa # for fgl_glxgears
 , # Whether to build the libraries only (i.e. not the kernel module or
@@ -72,7 +72,7 @@ stdenv.mkDerivation rec {
     ./patches/15.9-mtrr.patch
     ./patches/15.9-preempt.patch
     ./patches/15.9-sep_printf.patch ]
-  ++ optionals ( (builtins.compareVersions linux.version "4.6") >= 0 )
+  ++ optionals ( (builtins.compareVersions kernel.version "4.6") >= 0 )
                [ ./patches/kernel-4.6-get_user_pages.patch
                  ./patches/kernel-4.6-page_cache_release-put_page.patch ];

$ nixos-rebuild build -I nixpkgs=/home/matt/git/nixos/nixpkgs
building Nix...
building the system configuration...
error: value is null while a set was expected, at /home/matt/git/nixos/nixpkgs/pkgs/os-specific/linux/ati-drivers/default.nix:75:44
(use ‘--show-trace’ to show detailed location information)

Copy link
Member

Choose a reason for hiding this comment

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

I expect you have a 64-bit system with driSupport32Bit = true: there you get 64-bit version build against the kernel and a 32-bit version with libsOnly == true (i.e. without kernel).

@jerith666
Copy link
Contributor Author

The Travis CI failure was in building the OpenJDK docs, which seems spurious to me.

@jerith666
Copy link
Contributor Author

@vcunat you're right. Have a look at 7bc91ff instead.

@domenkozar domenkozar added this to the 16.09 milestone Sep 4, 2016
@jgeerds jgeerds added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Sep 11, 2016
@domenkozar
Copy link
Member

Did someone test this? cc @ocharles

@vcunat vcunat merged commit 7bc91ff into NixOS:master Sep 16, 2016
vcunat added a commit that referenced this pull request Sep 16, 2016
@vcunat
Copy link
Member

vcunat commented Sep 16, 2016

Looks reasonable and builds OK. I assume @jerith666 runs on it.

@vcunat
Copy link
Member

vcunat commented Sep 17, 2016

It still won't build with 4.7, but that's a relatively new branch.

@jerith666
Copy link
Contributor Author

@vcunat I do indeed run on this. I guess I should've said that explicitly in the description? I thought @domenkozar was asking for someone else to test it. Anyway, thanks for the merge!

@jerith666
Copy link
Contributor Author

I haven't looked at 4.7, it probably won't come to the top of my list for a bit yet. :-/

@vcunat
Copy link
Member

vcunat commented Sep 17, 2016

I see you checked the "tested execution" box; that should be enough IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ati-drivers does not build with kernel 4.6
5 participants