Generalize notion of arch-dependent modules #26287

abbradar · 2017-05-31T20:23:46Z

Motivation for this change

Currently we have several places in NixOS which use global architecture-dependent modules:

OpenGL drivers;
Input methods;
sane drivers;
GTK engines;

All of those share common properties:

They are necessarily "impure";
They are architecture-dependent -- that is, 32-bit program must use 32-bit modules;
They are commonly found through hardcoded paths or environment variables.

This patch allows NixOS modules to specify functions that return needed packages given a package set as value for libraries.packages. Those functions are combined and used to build per-arch environments in /run/current-system/${system}-lib where system is stdenv.system for a given arch. Also we add a way to specify PATH-like variables that refer to those directories in libraries.environment.

For now those paths are added in LD_LIBRARY_PATH like OpenGL drivers currently. We want to move from this when libglvnd allows us but meanwhile I sanitize all packages in libraries.packages, removing all non-directories in /lib. This shields people from accidentially polluting their library path by e.g. adding a VDPAU driver. To avoid sanitization one needs to use libraries.unsafePackages, the only current user now is our OpenGL module. When LD_LIBRARY_PATH=/lib is dropped this won't be necessary.

The end goal of this patch is to both streamline arch-dependent modules and add multi-arch support to things that don't have it now, for example IM modules. If implemented correctly all applications, both 32-bit and 64-bit, should have same themes, input modules, drivers and all other arch-dependent modules available in the system as long as libraries.support32Bit is flipped. Also we get rid of /run/opengl-driver{,-32} which I dislike personally (why have it when we already have /run/current-system?) :D

This patchset also cleans up our environment variables situation, some things being prerequisites to the main idea and some (DEs envvar cleanup) because they came along and were straightforward.

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
- 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.

Initial implementation is finished and I'm now trying to build my system with this. Before continuing though I'd like to hear feedback from others.

abbradar · 2017-05-31T20:59:11Z

nbp · 2017-05-31T21:23:03Z

lib/types.nix

+    function = from: to:
+      assert from.getSubModules == null;
+      mkOptionType rec {
+        name = "function";


No, No, No, NO, and NO!

Please elaborate :3

ce6d407#commitcomment-2336710

4ff1ab5#diff-4ef33a7568e82d463e872a450a6c2b7c

Add the tool "nixos-typecheck" that can check an option declaration to: #13607 (comment)

Ah, thanks for the reference!

I'm unsure how to do it the other way (avoid using functions). Without them every added package would need to be effectively doubled, e.g. we'd need to have two lists of identical packages but for different architectures specified by users (we in fact have this now as hardware.opengl.extraPackages{,32} and it looks quite ugly). Also I think a function is just a very natural way to express this idea (abstract away from arch, and focus on "what packages are needed").

Any ideas much appreciated!

Maybe we could have a pkgs-like module argument which corresponds to the recursiveUpdate of both pkgs and pkgsi686Linux packages, with a predicate to stop at derivations. Thus selecting a attribute name in the zipped set of packages would select the same attribute name in both sets.

@nbp Won't this be a heavy computation (zipping two package sets)? Still yes, sans that it sounds like a good idea. I'll try to implement it and see if it would indeed be slow (I think it won't be, after all).

nbp · 2017-05-31T21:25:11Z

nixos/modules/config/system-libs.nix

+
+      packages = mkOption {
+        type = types.function types.attrs (types.listOf types.package);
+        default = pkgs: [];


pkgs is provided as argument of the module, why do you need a function here?

The idea is that this function returns needed packages from passed package set. So when we need 32-bit support we evaluate this function twice, first with pkgs and then with pkgsi686Linux, getting what we want.

EDIT: the same idea is used in buildFHSEnv -- you pass it a function that gets pkgs and returns needed packages, and it evaluates it once or twice with different archs depending on what you need.

nbp · 2017-05-31T21:25:32Z

nixos/modules/config/system-libs.nix

+
+      unsafePackages = mkOption {
+        type = types.function types.attrs (types.listOf types.package);
+        default = pkgs: [];


pkgs is provided as argument of the module, why do you need a function here?

nbp · 2017-05-31T21:29:33Z

nixos/modules/config/system-libs.nix

+  };
+
+  libPath = mkLibPath pkgs;
+  libPath32 = mkLibPath pkgs.pkgsi686Linux;


Having both implies that installing lib32 support would cause the system to compile an x86 toolchain in addition to the existing x86_64 toolchain. This sounds like way too much overhead to me. Couldn't such package already provide a lib32 directory as part of the outputs of the package?

@nbp Not the whole system, only libraries which need to be multiarch. We already do this with video drivers, this just extends the idea to similar packages.

vcunat · 2017-06-11T17:58:53Z

I only skimmed all this, but if we're blocked on the function type, why not use some attrmaps instead of function maps? Nix is lazy, so there's not too much difference. (EDIT: and the memoization we get might actually be a good thing.)

I also don't have any idea about problems caused by cross-compiling NixOS, but I guess we would better leave such questions for some later iteration.

abbradar · 2017-06-13T10:38:44Z

@vcunat Yeah, I plan to do it this way after all (I had some doubts because while Nix is lazy we still need to compute at least the shallow top-level set one more time, and I don't know how long does it take).

I'll push a second revision in several days.

vcunat · 2017-06-13T14:30:52Z

I expect each top-level set to be just a set of platforms, so that should be very cheap for all the unused platforms.

`recursiveZipWithUntil` is a generalization of `recursiveUpdateUntil` for a list of attribute sets and using a custom merge function. `recursiveZipWith` to `recursiveZipWithUntil` is like `recursiveUpdate` to `recursiveUpdateUntil`.

Until now we had several options that accepted environment variables which had different types and copied functionality. Two side-effects from this change: * systemd.service.*.environment.NAME no longer accepts `null`s; * systemd combines global and service-local variables: for example when LD_LIBRARY_PATH is defined both in globalEnvironment and in service's environment as a list they are merged together.

Most of needed variables are already defined system-wide or could be moved there.

This makes it possible to have working input methods in 32-bit programs on a 64-bit system.

Currently we have several places in NixOS which use global architecture-dependent modules: * OpenGL drivers; * Input methods; * sane drivers; * GTK engines; All of those share common properties: * They are necessarily "impure"; * They are architecture-dependent -- that is, 32-bit program must use 32-bit modules; * They are commonly found through hardcoded paths or environment variables. This patch allows NixOS modules to specify functions that return needed packages given a package set as value for `libraries.packages`. Those functions are combined and used to build per-arch environments in /run/current-system/${system}-lib where `system` is `stdenv.system` for a given arch. Also we add a way to specify PATH-like variables that refer to those directories in `libraries.environment`.

FIXME: fglrx is not patched to use new paths yet! References to OpenGL drivers path and other paths are replaced throughout nixpkgs tree. User-facing changes: * `hardware.opengl.driSupport32Bit` is gone, replaced by generic `libraries.support32Bit`; * `hardware.opengl.driSupport` is gone (not used anywhere); * `hardware.opengl.extraPackages{,32}` are replaced by `libraries.packages` which is a function;

We expect now to have libraries handled by libraries.packages.

`i18n.inputMethod.{ibus,fcitx}.engines` are now a function.

`hardware.sane.extraBackends` is now a function.

Instead of placing GTK engines and GIO modules in system path, place then in arch-dependent directories.

It's a zip of package sets for all supported architectures on the system.

abbradar · 2017-06-20T16:32:41Z

I've updated this patchset to use zipped packages instead -- sorry for the delay, life gets into the way :D. To me it looks a step back: zipped packages are much less intuitive and composable. Compare for example abbradar@9bd3dd5 and 388a942. Still, this seems an improvement over what we have now, so if others approve this approach I'll continue the work (see TODO list above).

vcunat · 2017-06-23T06:25:28Z

Oh, I originally thought it would be pkgs_multiarch.<system>.<pkgName> and not the other way around, but I see it would complicate things like unsafePackages...

abbradar · 2017-06-23T10:01:43Z

@vcunat Won't that way also complicate users specifying their packages? For example, how'd I specify that I want libraries from package foo installed? Currently it's libraries.packages = [ pkgs_multiarch.foo ]. I still hope that we can figure out something better than all those map (x : x.${system})...

vcunat · 2017-06-23T11:22:11Z

Yes, that seems similar. I can't immediately see into further consequences of either approach, it was mainly intuition to do it the other way. Zipping the top-level set hopefully won't make a noticeable slow-down (it's memoized).

abbradar · 2017-06-23T11:30:07Z

Hm, but how? If you have `pkgs_multiarch.<system>.foo` then I see no simple way to specify a package for a user because they have to specify either a path (string) or both `arch32.foo` and `arch64.foo` (as we do now). In fact one can think of `pkgs` and `pkgs.pkgsi686Linux` as members of that set, so if we can figure out how can it work nicely we may not even need `pkgs_multiarch` - but I see no immediate solution... -- Nikolay (from phone).

vcunat · 2017-06-23T15:01:30Z

A string wouldn't be much flexible. It would be nice to support .override and similar stuff. Perhaps a utility function pkgs_multi.map (pkgs: pkgs.bash), returning a set indexable by system names, though I haven't given it any deep thought.

Profpatsch · 2018-04-25T11:08:18Z

I think the changes @Ericson2314 is constantly working on should supersede this pull request? Or at least make it easier to implement.

Ericson2314 · 2018-04-26T15:56:57Z

Which changes haha?

Ericson2314 · 2018-04-26T15:57:46Z

If you mean cross in general, I haven't yet got rid of the old multi-lib stuff but yes that would be nice.

mmahut · 2019-08-03T14:41:17Z

What is the status of this pull request?

abbradar · 2019-08-09T09:33:37Z

I want to return to this idea some time as the problem of different arch-dependent modules is still there. We can close this as my next attempt will likely be a complete rewrite.

abbradar added 0.kind: enhancement 1.severity: mass-rebuild 6.topic: nixos 9.needs: community feedback 9.needs: changelog 9.needs: documentation labels May 31, 2017

abbradar force-pushed the libraries branch from 28685ba to 7dea9a0 Compare May 31, 2017 21:04

nbp suggested changes May 31, 2017

View reviewed changes

abbradar force-pushed the libraries branch from 7dea9a0 to 26332db Compare June 1, 2017 09:43

abbradar added 5 commits June 20, 2017 16:19

nixos: cleanup environment variables of desktop environments

28ca7f3

Most of needed variables are already defined system-wide or could be moved there.

gtk{2,3}: support architecture-dependent immodules

a0595c0

This makes it possible to have working input methods in 32-bit programs on a 64-bit system.

unigine-valley: don't prefix with NixOS OpenGL path

64ba817

abbradar mentioned this pull request Jun 20, 2017

Sane config dir saved in environment variable, requiring logout/login on switch #26727

Closed

abbradar added 7 commits June 20, 2017 19:28

system-path module: remove /lib from system path

d5e0c0c

We expect now to have libraries handled by libraries.packages.

input-method module: use libraries.packages

388a942

`i18n.inputMethod.{ibus,fcitx}.engines` are now a function.

sane service: use libraries.packages

4a53e7b

`hardware.sane.extraBackends` is now a function.

desktop manager services: use libraries.packages

8e17ab3

Instead of placing GTK engines and GIO modules in system path, place then in arch-dependent directories.

nixos: add pkgs_multiarch argument

57e0c3a

It's a zip of package sets for all supported architectures on the system.

abbradar force-pushed the libraries branch from 26332db to 57e0c3a Compare June 20, 2017 16:29

abbradar added the 2.status: work-in-progress label Jun 20, 2017

abbradar closed this Aug 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalize notion of arch-dependent modules #26287

Generalize notion of arch-dependent modules #26287

abbradar commented May 31, 2017 •

edited

abbradar commented May 31, 2017 •

edited

nbp May 31, 2017

abbradar May 31, 2017

nbp May 31, 2017

abbradar May 31, 2017 •

edited

nbp Jun 1, 2017

abbradar Jun 3, 2017

nbp May 31, 2017

abbradar May 31, 2017 •

edited

nbp May 31, 2017

nbp May 31, 2017

abbradar May 31, 2017

vcunat commented Jun 11, 2017 •

edited

abbradar commented Jun 13, 2017

vcunat commented Jun 13, 2017

abbradar commented Jun 20, 2017

vcunat commented Jun 23, 2017

abbradar commented Jun 23, 2017 •

edited

vcunat commented Jun 23, 2017

abbradar commented Jun 23, 2017 via email

vcunat commented Jun 23, 2017

Profpatsch commented Apr 25, 2018

Ericson2314 commented Apr 26, 2018

Ericson2314 commented Apr 26, 2018

mmahut commented Aug 3, 2019

abbradar commented Aug 9, 2019

Generalize notion of arch-dependent modules #26287

Generalize notion of arch-dependent modules #26287

Conversation

abbradar commented May 31, 2017 • edited

Motivation for this change

Things done

abbradar commented May 31, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbradar May 31, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbradar May 31, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vcunat commented Jun 11, 2017 • edited

abbradar commented Jun 13, 2017

vcunat commented Jun 13, 2017

abbradar commented Jun 20, 2017

vcunat commented Jun 23, 2017

abbradar commented Jun 23, 2017 • edited

vcunat commented Jun 23, 2017

abbradar commented Jun 23, 2017 via email

vcunat commented Jun 23, 2017

Profpatsch commented Apr 25, 2018

Ericson2314 commented Apr 26, 2018

Ericson2314 commented Apr 26, 2018

mmahut commented Aug 3, 2019

abbradar commented Aug 9, 2019

abbradar commented May 31, 2017 •

edited

abbradar commented May 31, 2017 •

edited

abbradar May 31, 2017 •

edited

abbradar May 31, 2017 •

edited

vcunat commented Jun 11, 2017 •

edited

abbradar commented Jun 23, 2017 •

edited