displaylink: 1.3.52 -> 4.1.9 and refactor #34194

ghost · 2018-01-23T16:24:18Z

While this commit updates displaylink, it also refactors the package to
use a FHS env. This is because the binary of displaylink breaks when
modifying the RPATH with patchelf. So a little wrapper is created around
it, running the binary in the chroot.

This is the first time I use an FHS env, so please look at this accurately.

Motivation for this change

The current version of displaylink doesn't work because patchelf breaks the RPATH.

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
- 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/)
Fits CONTRIBUTING.md.

joachifm · 2018-02-17T10:06:25Z

@GrahamcOfBorg build displaylink

GrahamcOfBorg · 2018-02-17T10:06:43Z

Failure on aarch64-linux (full log)

Partial log (click to expand)

Package ‘displaylink-4.1.9’ in /var/lib/gc-of-borg/nix-test-rs-2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-2/pkgs/os-specific/linux/displaylink/default.nix:62 has an unfree license (‘unfree’), refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-02-17T10:06:46Z

Failure on x86_64-linux (full log)

Partial log (click to expand)

Package ‘displaylink-4.1.9’ in /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/os-specific/linux/displaylink/default.nix:62 has an unfree license (‘unfree’), refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-02-17T10:06:47Z

Failure on x86_64-darwin (full log)

Partial log (click to expand)

Package ‘chrootenv’ in /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/lnl7-mac/pkgs/build-support/build-fhs-userenv/chrootenv/default.nix:14 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

joachifm · 2018-02-17T10:08:48Z

Ah, it's unfree, ignore the build errors :)

abbradar

This doesn't build for me; probably needs hardeningDisable = [ "format" ]; because of kernel module errors.

Before you fix my other comments -- are you sure FHS environment is really necessary? Binary corruption can be possibly fixed with just dontStrip = true;.

P.S.: I don't have hardware to test this, so I rely on you saying that it works.

abbradar · 2018-02-17T12:18:54Z

nixos/modules/hardware/video/displaylink.nix

@@ -48,9 +48,10 @@ in
      after = [ "display-manager.service" ];
      conflicts = [ "getty@tty7.service" ];
      path = [ pkgs.kmod ];
+      wantedBy = [ "multi-user.target" ];


My memory is hazy but I think this service is started automatically by udev rules, so no need for that.

abbradar · 2018-02-17T12:19:43Z

pkgs/os-specific/linux/displaylink/default.nix

+  executable = true;
+  text = ''
+    #!${bash}/bin/bash
+    exec ${fhsEnv}/bin/displaylink-fhs-env "${displaylink}/bin/DisplayLinkManager"


Better use runScript in fhsEnv.

abbradar · 2018-02-17T12:20:18Z

pkgs/os-specific/linux/displaylink/default.nix

  };
+in writeTextFile {


Won't other files, like udev rules, be hidden in package because of that indirection?

abbradar · 2018-02-17T12:21:12Z

pkgs/os-specific/linux/displaylink/default.nix


 let
+  version = "4.1.9";
+  libPath = lib.makeLibraryPath [ stdenv.cc.cc utillinux libusb1 evdi ];


It's not used in code anymore.

ghost · 2018-02-21T15:22:49Z

@abbradar Thank you for your review! As you can see, I'm a Nix(OS) newbie so your hints are highly appreciated.
With help of @Lassulus I entirely rebuilt this PR from scratch.

This doesn't build for me; probably needs hardeningDisable = [ "format" ]; because of kernel module errors.

Um, it works here without hardeningDisable. Have you enabled any special hardening options that I can enable as well to see if it still works?

Before you fix my other comments -- are you sure FHS environment is really necessary? Binary corruption can be possibly fixed with just dontStrip = true;.

Sadly, yes. When creating the PR from scratch, I tried again with dontStrip. However, the binary is already stripped so I doubt that strip would do anything. Also, it only works when removing --set-rpath from patchelf, so I think that patchelf causes the binary corruption. When using ldd, it cannot read the rpath when it's patched.
So yes, the FHS env seems to be required.

P.S.: I don't have hardware to test this, so I rely on you saying that it works.

It works here, but of course, it's better to have more testers. I'll ask a friend of mine who also runs NixOS to test it, too, but that's all I can do. EDIT: It works for him

My memory is hazy but I think this service is started automatically by udev rules, so no need for that.

Your memory is correct ;) I removed the RequiredBy

Won't other files, like udev rules, be hidden in package because of that indirection?

Yes, but that should be fixed now with the symlinkJoin.

While this commit updates displaylink, it also refactors the package to use a FHS env. This is because the binary of displaylink breaks when modifying the RPATH with patchelf. So a little wrapper is created around it, running the binary in the chroot

abbradar · 2018-02-22T09:57:53Z

@Scarmiglione Very strange, I just now tried to bump DisplayLink's version in our existing package and the binary (DisplayLinkManager) works. About hardening: I guess that's because I try to build the package on master which has newer something. I've fixed it with disabling fortify hardening for evdi (DisplayLink's kernel driver).

Can you try building https://github.com/abbradar/nixpkgs/commits/displaylink and verifying if resulting binary fails for you?

ghost · 2018-02-23T15:52:55Z

@abbradar It seems to run with your branch, but my two monitors are not detected. So what are we going to do with this? Do you know any other troubleshooting methods?

abbradar · 2018-02-23T16:20:49Z

Does daemon run and is evdi module loaded?

ghost · 2018-02-23T16:26:22Z

Yes, the daemon is running and the evdi module is loaded

abbradar · 2018-02-23T16:31:52Z

Maybe /tmp/DisplayLinkManager.log says something?

If we won't be able to solve this in a reasonable amount of time let's use your FHS implementation instead.

ghost · 2018-02-23T21:22:16Z

Probably you mean /var/log/displaylink/DisplaylinkManager.log? It just contains base64-encoded lines. If I decode one, it's binary gibberish.

abbradar · 2018-02-24T01:40:35Z

@schuppentier I think I fixed that, can you try if it works now?

ghost · 2018-02-26T13:23:56Z

@abbradar Yes! This really fixes it! This message is typed on a browser on my DisplayLink monitor, running your branch. Do you want to create a new PR that replaces this one?

abbradar · 2018-02-26T13:25:20Z

Awesome ;) I'll just push to master then after one more evaluation test (I'll close this one when I push). Thank you!

ghost · 2018-02-26T13:25:53Z

Thank you very much for your help :) Will this be part of the next NixOS release btw?

abbradar · 2018-02-26T13:26:35Z

Yeah, I still have plenty of time to push that. I'll backport this to 17.09 too.

ghost · 2018-02-26T13:27:59Z

Alright, thank you for your time and effort 👍

ghost · 2018-02-26T14:12:28Z

Do you also backport evdi? It's required for DisplayLink and broken on the current kernel.

abbradar · 2018-02-26T14:17:30Z

@schuppentier Yeah, plan to do so. Generally our policy is that it's okay to backport version changes to stable releases if it's needed to fix something.

Closes #34194. (cherry picked from commit 4db787b)

azdle · 2018-04-01T18:16:48Z

@schuppentier is there anything required to use this other than adding it to displayDrivers? I've been trying to get this to work for a few months now and your fix has solved the segfault issue, but am still not getting anything on my external displays.

My config: https://github.com/azdle/my-nixos-config/blob/master/desktop-configuration.nix#L31

ghost · 2018-04-02T16:31:10Z

@azdle Try something like xrandr --listproviders and xrandr --setprovideroutputsource 0 1

azdle · 2018-04-02T22:45:38Z

Dang, that did it. You rock.

I ran:

xrandr --setprovideroutputsource 2 0
xrandr --setprovideroutputsource 3 0

and both of my external displays then showed up in gnome's "displays" settings pane as turned off. Turned each one and they're both working now.

It's somehow making firefox really laggy (multiple seconds of lag) and the whole UI goes laggy when I close the lid, but this is a whole lot closer than I've gotten before and this is something that should be much more googleable.

Again, thanks, so much.

Edit: resetting layers.acceleration.force-enabled back to false fixed the lag issue in firefox.

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Jan 23, 2018

joachifm requested a review from abbradar February 17, 2018 10:06

abbradar requested changes Feb 17, 2018

View reviewed changes

ghost requested review from edolstra, edwtjo, Ericson2314, FRidh, nbp, orivej, peti, ttuegel and zimbatm as code owners February 21, 2018 15:22

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 8.has: clean-up labels Feb 21, 2018

peti removed their request for review February 21, 2018 16:04

Ericson2314 removed their request for review February 21, 2018 22:03

ttuegel removed their request for review February 24, 2018 14:20

abbradar closed this in 4db787b Feb 27, 2018

abbradar added a commit that referenced this pull request Feb 27, 2018

displaylink: 1.3.52 -> 4.1.9

4030f92

Closes #34194. (cherry picked from commit 4db787b)

ghost deleted the displaylink branch April 2, 2018 16:30

FRidh removed their request for review April 8, 2018 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

displaylink: 1.3.52 -> 4.1.9 and refactor #34194

displaylink: 1.3.52 -> 4.1.9 and refactor #34194

ghost commented Jan 23, 2018

joachifm commented Feb 17, 2018

GrahamcOfBorg commented Feb 17, 2018

GrahamcOfBorg commented Feb 17, 2018

GrahamcOfBorg commented Feb 17, 2018

joachifm commented Feb 17, 2018

abbradar left a comment •

edited

abbradar Feb 17, 2018

abbradar Feb 17, 2018

abbradar Feb 17, 2018

abbradar Feb 17, 2018

ghost commented Feb 21, 2018 •

edited by ghost

abbradar commented Feb 22, 2018

ghost commented Feb 23, 2018 •

edited by ghost

abbradar commented Feb 23, 2018

ghost commented Feb 23, 2018

abbradar commented Feb 23, 2018

ghost commented Feb 23, 2018

abbradar commented Feb 24, 2018

ghost commented Feb 26, 2018

abbradar commented Feb 26, 2018

ghost commented Feb 26, 2018

abbradar commented Feb 26, 2018

ghost commented Feb 26, 2018 •

edited by ghost

ghost commented Feb 26, 2018

abbradar commented Feb 26, 2018

azdle commented Apr 1, 2018

ghost commented Apr 2, 2018

azdle commented Apr 2, 2018 •

edited

displaylink: 1.3.52 -> 4.1.9 and refactor #34194

displaylink: 1.3.52 -> 4.1.9 and refactor #34194

Conversation

ghost commented Jan 23, 2018

Motivation for this change

Things done

joachifm commented Feb 17, 2018

GrahamcOfBorg commented Feb 17, 2018

GrahamcOfBorg commented Feb 17, 2018

GrahamcOfBorg commented Feb 17, 2018

joachifm commented Feb 17, 2018

abbradar left a comment • edited

Choose a reason for hiding this comment

abbradar Feb 17, 2018

Choose a reason for hiding this comment

abbradar Feb 17, 2018

Choose a reason for hiding this comment

abbradar Feb 17, 2018

Choose a reason for hiding this comment

abbradar Feb 17, 2018

Choose a reason for hiding this comment

ghost commented Feb 21, 2018 • edited by ghost

abbradar commented Feb 22, 2018

ghost commented Feb 23, 2018 • edited by ghost

abbradar commented Feb 23, 2018

ghost commented Feb 23, 2018

abbradar commented Feb 23, 2018

ghost commented Feb 23, 2018

abbradar commented Feb 24, 2018

ghost commented Feb 26, 2018

abbradar commented Feb 26, 2018

ghost commented Feb 26, 2018

abbradar commented Feb 26, 2018

ghost commented Feb 26, 2018 • edited by ghost

ghost commented Feb 26, 2018

abbradar commented Feb 26, 2018

azdle commented Apr 1, 2018

ghost commented Apr 2, 2018

azdle commented Apr 2, 2018 • edited

abbradar left a comment •

edited

ghost commented Feb 21, 2018 •

edited by ghost

ghost commented Feb 23, 2018 •

edited by ghost

ghost commented Feb 26, 2018 •

edited by ghost

azdle commented Apr 2, 2018 •

edited