xorg: allow overriding via overrideScope' #45328

thefloweringash · 2018-08-19T12:27:37Z

Motivation for this change

Wanted to modify some xorg.* packages in an overlay, ran into #11944. I did a fairly straight forward conversion into makeScope / overrideScope, but I don't have a good idea of the state of the art in this area.

Additionally discovered that while pkgs/servers/x11/xorg/default.nix is auto-generated, it has been manually updated for a few packages. I updated the source lists so that regenerating default.nix doesn't change any packages. The exception to this is xeyes, which gained a new buildInput. The source lists should be converted to https, but I'd prefer to do that in a separate PR.

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

thefloweringash

The generator script attempts to use a caching feature of nix-prefetch-url which no longer exists. Every time it's run it downloads everything from scratch.

thefloweringash · 2018-08-19T14:23:28Z

pkgs/servers/x11/xorg/default.nix


-  libxcb = (mkDerivation "libxcb" {
+  libxcb = callPackage ({ stdenv, libxslt, libpthreadstubs, python, libXau, xcbproto, libXdmcp }: stdenv.mkDerivation {
    name = "libxcb-1.12";
    builder = ./builder.sh;
    src = fetchurl {
      url = http://xcb.freedesktop.org/dist/libxcb-1.12.tar.bz2;
      sha256 = 


Moved this to overrides.nix. It got lost in the regeneration.

(Github is displaying incorrectly for me, this comment should be attached to xkeyboardconfig's configureFlags).

thefloweringash · 2018-08-19T14:25:15Z

pkgs/servers/x11/xorg/overrides.nix

    configureFlags = attrs.configureFlags or []
      ++ malloc0ReturnsNullCrossFlag;
    # the include files need ft2build.h, and Requires.private isn't enough for us
    postInstall = ''
      sed "/^Requires:/s/$/, freetype2/" -i "$dev/lib/pkgconfig/xft.pc"
    '';
-  };
+    passthru = {


The generated file no longer applies // { inherit $buildInputs; } to each package. Qt3 relies on being able to access libXft.freetype and libXft.fontconfig. Updated to passthru.

thefloweringash · 2018-08-19T14:28:44Z

pkgs/top-level/all-packages.nix

@@ -13464,7 +13464,7 @@ with pkgs;
    libdrm = if stdenv.isLinux then libdrm else null;
    abiCompat = config.xorg.abiCompat # `config` because we have no `xorg.override`
      or (if stdenv.isDarwin then "1.18" else null); # 1.19 needs fixing on Darwin
-  } // { inherit xlibsWrapper; } );
+  } // { inherit xlibsWrapper; } ) [ "packages" ]);


Need to remove the "packages" attribute from xorg's scope. Otherwise it gets merged into the main attribute set and makes lambdabot fail. I don't yet know why it's not a problem to have "callPackage", etc from makeScope

nixpkgs/pkgs/top-level/splice.nix

Lines 27 to 28 in 8014c7f

defaultBuildBuildScope = pkgs.buildPackages.buildPackages // pkgs.buildPackages.buildPackages.xorg;

defaultBuildHostScope = pkgs.buildPackages // pkgs.buildPackages.xorg;

nixpkgs/lib/customisation.nix

Lines 194 to 203 in 8014c7f

makeScope = newScope: f:

let self = f self // {

newScope = scope: newScope (self // scope);

callPackage = self.newScope {};

overrideScope = g:

makeScope newScope

(self_: let super = f self_; in super // g super self_);

packages = f;

};

in self;

They should probably also be removed. Should this be done in all-packages.nix or splice.nix?

thefloweringash · 2018-08-19T14:31:23Z

pkgs/servers/x11/xorg/default.nix


-  libxcb = (mkDerivation "libxcb" {
+  libxcb = callPackage ({ stdenv, libxslt, libpthreadstubs, python, libXau, xcbproto, libXdmcp }: stdenv.mkDerivation {
    name = "libxcb-1.12";
    builder = ./builder.sh;
    src = fetchurl {
      url = http://xcb.freedesktop.org/dist/libxcb-1.12.tar.bz2;
      sha256 = 


I believe this (xeyes) is the only rebuild in this PR.

thefloweringash · 2018-08-20T16:21:40Z

This is at the point that I need some feedback. Some questions:

why did the old version import some dependencies for xorg via inherit (buildPlatform) ...;? Does this break cross?
the "has clean-up" tag suggests a removed package, but there shouldn't be any. How do I list changes?

Ericson2314 · 2018-09-10T20:24:59Z

pkgs/top-level/all-packages.nix

-      cairo epoxy;
-    inherit (buildPackages) pkgconfig xmlto asciidoc flex bison;
-    inherit (darwin) apple_sdk cf-private libobjc;
+  xorg = recurseIntoAttrs ((lib.callPackagesWith pkgs ../servers/x11/xorg {


You actually want callPackageWith. callPackagesWith will push the override into the inner attributes which is no longer correct.

Ericson2314 · 2018-09-10T20:30:06Z

pkgs/top-level/all-packages.nix

-    inherit (buildPackages) pkgconfig xmlto asciidoc flex bison;
-    inherit (darwin) apple_sdk cf-private libobjc;
+  xorg = recurseIntoAttrs ((lib.callPackagesWith pkgs ../servers/x11/xorg {
+  }).overrideScope (lib.callPackageWith pkgs ../servers/x11/xorg/overrides.nix {


Can we do inline

nixpkgs/lib/customisation.nix

Line 201 in f305b9b

(lib.fixedPoints.extends (lib.flip g) f);

to get something like

(lib.fixedPoints.extends (import ../servers/x11/xorg/overrides.nix) (import ../servers/x11/xorg)

(This ignores the flip. That would be for the odd order of self: super:, when you already did idiomatic self: super:.)

Ericson2314 · 2018-09-10T20:39:50Z

Why did the old version import some dependencies for xorg via inherit (buildPlatform) ...;? Does this break cross?

So first of all, I think you meant buildPackages. And to answer your question, yes, it is needed because callPackagesWith pkgs is being used directly. See the unfortunate "splicing' stuff that goes into normal callPcakges. callPackagesWith pkgs doesn't get that, and so is missing packages.

Now that splicePackages is exposed, however, you can let-bind a callPackageWithoutXorg which will obviate the need to sort specific packages by hand.

Ericson2314 · 2018-09-10T20:40:26Z

Overall, great work! It would be nice to get this into 18.09 as it's a good cleanup and will make any xorg backports easier if we do so.

thefloweringash · 2018-09-15T03:18:28Z

Rebased to resolve the conflict, fixed callPackagesWith vs callPackageWith.

It sounds like you've got a clear idea of how to make this nicer, but I'm not familiar enough with the splicing to grasp it. Can you expand a bit on how implementing callPackageWithoutXorg would look?

As for usinglib.fixedPoints.extends, would that require changing default.nix and overrides.nix to take pkgs explicitly instead of being called with callPackage?

Thanks for looking at this!

Ericson2314 · 2018-09-23T18:39:16Z

I'm going to make a few fixes here, then merge soon.

Ericson2314

N.B. to other reviews I will fix cross

Ericson2314 · 2018-10-18T16:21:30Z

Well, it's all good now! Finally!!

dtzWill · 2018-10-19T02:19:51Z

Can this (master, really) be merged over into staging? Otherwise various X updates (gathered in #48549) will make it a pain when wanting to merge staging back over. It will probably be a pain to rebase that on top of this but not too bad and better to do that in a PR, under review, than down the line by some poor soul trying to merge and hoping they don't break everything :P (and hopefully the updates can be auto-generated to fit this anyway :)).

Ericson2314 · 2018-10-20T17:19:03Z

@dtzWill Sorry just saw. Sure that makes sense, will do!

thefloweringash force-pushed the xorg-override branch from 3c63091 to 01bb7e7 Compare August 19, 2018 13:31

GrahamcOfBorg added 8.has: clean-up 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Aug 19, 2018

thefloweringash force-pushed the xorg-override branch from 7bc7688 to e723094 Compare August 19, 2018 14:07

thefloweringash commented Aug 19, 2018

View reviewed changes

thefloweringash changed the title ~~xorg: allow overriding via overrideScope~~ [WIP] xorg: allow overriding via overrideScope Aug 19, 2018

thefloweringash force-pushed the xorg-override branch 2 times, most recently from af84145 to e53faef Compare August 20, 2018 15:45

matthewbauer requested review from abbradar and Ericson2314 August 22, 2018 20:03

Ericson2314 reviewed Sep 10, 2018

View reviewed changes

Ericson2314 added this to the 18.09 milestone Sep 10, 2018

thefloweringash force-pushed the xorg-override branch from e53faef to 1b748ec Compare September 15, 2018 03:10

Ericson2314 requested changes Sep 23, 2018

View reviewed changes

samueldr modified the milestones: 18.09, 19.03 Oct 6, 2018

thefloweringash added 5 commits October 11, 2018 13:41

xorg: sync source lists with default.nix

0804902

xorg: allow overriding via overrideScope

9454f71

xorg: add caching to generate-expr-from-tarballs.pl

0e12683

xorg: simplify argument passing into xorg and overrides

fb9604c

xorg: don't splice scope attributes into main package set

47bf3f3

Ericson2314 force-pushed the xorg-override branch from 1b748ec to 47bf3f3 Compare October 11, 2018 19:03

GrahamcOfBorg added the 2.status: merge conflict label Oct 11, 2018

xorg: Don't use deprecated item

88cd633

Ericson2314 force-pushed the xorg-override branch from 1e89ca8 to 638b41c Compare October 18, 2018 15:57

Merge remote-tracking branch 'upstream/master' into xorg-override

1147d2e

Ericson2314 force-pushed the xorg-override branch from 638b41c to 1147d2e Compare October 18, 2018 16:00

GrahamcOfBorg removed 10.rebuild-darwin-stdenv 10.rebuild-linux-stdenv labels Oct 18, 2018

Ericson2314 merged commit d408063 into NixOS:master Oct 18, 2018

Ericson2314 changed the title ~~[WIP] xorg: allow overriding via overrideScope~~ xorg: allow overriding via overrideScope Oct 18, 2018

Ericson2314 changed the title ~~xorg: allow overriding via overrideScope~~ xorg: allow overriding via overrideScope' Oct 18, 2018

Ericson2314 mentioned this pull request Oct 18, 2018

xorg: allow overriding via overrideScope' for 18.09 #48673

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xorg: allow overriding via overrideScope' #45328

xorg: allow overriding via overrideScope' #45328

thefloweringash commented Aug 19, 2018

thefloweringash left a comment

thefloweringash Aug 19, 2018 •

edited

thefloweringash Aug 19, 2018

thefloweringash Aug 19, 2018

thefloweringash Aug 19, 2018 •

edited

thefloweringash commented Aug 20, 2018

Ericson2314 Sep 10, 2018

Ericson2314 Sep 10, 2018

Ericson2314 commented Sep 10, 2018

Ericson2314 commented Sep 10, 2018

thefloweringash commented Sep 15, 2018

Ericson2314 commented Sep 23, 2018

Ericson2314 left a comment

Ericson2314 commented Oct 18, 2018 •

edited

dtzWill commented Oct 19, 2018

Ericson2314 commented Oct 20, 2018

	defaultBuildBuildScope = pkgs.buildPackages.buildPackages // pkgs.buildPackages.buildPackages.xorg;
	defaultBuildHostScope = pkgs.buildPackages // pkgs.buildPackages.xorg;

	makeScope = newScope: f:
	let self = f self // {
	newScope = scope: newScope (self // scope);
	callPackage = self.newScope {};
	overrideScope = g:
	makeScope newScope
	(self_: let super = f self_; in super // g super self_);
	packages = f;
	};
	in self;

xorg: allow overriding via overrideScope' #45328

xorg: allow overriding via overrideScope' #45328

Conversation

thefloweringash commented Aug 19, 2018

Motivation for this change

Things done

thefloweringash left a comment

Choose a reason for hiding this comment

thefloweringash Aug 19, 2018 • edited

Choose a reason for hiding this comment

thefloweringash Aug 19, 2018

Choose a reason for hiding this comment

thefloweringash Aug 19, 2018

Choose a reason for hiding this comment

thefloweringash Aug 19, 2018 • edited

Choose a reason for hiding this comment

thefloweringash commented Aug 20, 2018

Ericson2314 Sep 10, 2018

Choose a reason for hiding this comment

Ericson2314 Sep 10, 2018

Choose a reason for hiding this comment

Ericson2314 commented Sep 10, 2018

Ericson2314 commented Sep 10, 2018

thefloweringash commented Sep 15, 2018

Ericson2314 commented Sep 23, 2018

Ericson2314 left a comment

Choose a reason for hiding this comment

Ericson2314 commented Oct 18, 2018 • edited

dtzWill commented Oct 19, 2018

Ericson2314 commented Oct 20, 2018

thefloweringash Aug 19, 2018 •

edited

thefloweringash Aug 19, 2018 •

edited

Ericson2314 commented Oct 18, 2018 •

edited