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

[RFC] Declarative wrappers #85103

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
242a7da
Write a prototype derivation that knows allInputs
doronbehar Apr 10, 2020
30f26fe
Now %out% is string replaced
doronbehar Apr 12, 2020
112882f
Make more stuff work
doronbehar Apr 12, 2020
9893944
Add more passthrus and make basic functionality work
doronbehar Apr 12, 2020
1e1464e
Create a very much working :) wrapGApps function
doronbehar Apr 12, 2020
78cbe77
fix gdk pixbuf module file env
doronbehar Apr 12, 2020
74a01b3
Replace _all_ occurences of %outname% in env's strings
doronbehar Apr 12, 2020
a18f521
prepare neowrap for XDG_DATA_DIRS=$out/share
doronbehar Apr 12, 2020
72d6478
teach it to deal with encyclopedia.wrapOut
doronbehar Apr 12, 2020
d0ed3a5
Prepare for a PR
doronbehar Apr 12, 2020
c0dab18
dumbly add passthrus to some qt packages
doronbehar Apr 12, 2020
6283f15
Make picard work perfectly with new wrapping method
doronbehar Apr 12, 2020
b4d3633
fix missing semicolon pkgs/development/libraries/gvfs/default.nix
ryantm Apr 12, 2020
ea9640a
Fix another ofborg error
doronbehar Apr 13, 2020
8074b9c
Stop using builtins.trace to avoid ofborg complaints
doronbehar Apr 13, 2020
f173b80
use @out@ and not %out%
doronbehar Apr 13, 2020
dda939a
Support single value env keys - for GDK_PIXBUF_MODULE_FILE
doronbehar Apr 13, 2020
8999768
Enable to refer to the unwrapped (real) drv of the wrapped drv
doronbehar Apr 13, 2020
2300445
some comments improvements
doronbehar Apr 14, 2020
32dd553
Use envInfo as was once envInfoFolded - for readability
doronbehar Apr 14, 2020
277ac54
Instead of using many values in env vars, enable to use a link argume…
doronbehar Apr 14, 2020
0ed98f4
Make it possible to lndir whole packages by env
doronbehar Apr 14, 2020
270ffc2
Test and make picard work as well with new features
doronbehar Apr 14, 2020
aea2b5a
Prepare support for python wrappings
doronbehar Apr 15, 2020
e6f9600
Improve getAllInputs recursiveness
doronbehar Apr 15, 2020
f50caf8
Make sure picard and arandr dont wrap python scripts, letting wrapGen…
doronbehar Apr 15, 2020
2ca4dc7
Add propagateEnv for more python deps of picard and arandr
doronbehar Apr 15, 2020
8c74d6a
Move wrapOut to wrapper arguments, out of encyclopedia
doronbehar Apr 15, 2020
3a8e032
Improve comments for debug statements / variables
doronbehar Apr 15, 2020
73bc870
sed 's/PYTHONPATH/NIX_PYTHONPATH/g' as @FRidh suggested
doronbehar Apr 16, 2020
2aacb90
Improve logic of linkPkg and encyclopedia vs wrapOut
doronbehar Apr 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions pkgs/applications/audio/picard/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ in pythonPackages.buildPythonApplication rec {
sha256 = "0xalg4dvaqb396h4s6gzxnplgv1lcvsczmmrlhyrj0kfj10amhsj";
};

nativeBuildInputs = [ gettext qt5.wrapQtAppsHook qt5.qtbase ]
nativeBuildInputs = [ gettext qt5.wrapQtAppsHook ];

propagatedBuildInputs = with pythonPackages; [
qt5.qtbase
pyqt5
mutagen
chromaprint
discid
]
++ stdenv.lib.optionals (pyqt5.multimediaEnabled) [
qt5.qtmultimedia.bin
gst_all_1.gstreamer
Expand All @@ -32,18 +40,12 @@ in pythonPackages.buildPythonApplication rec {
]
;

propagatedBuildInputs = with pythonPackages; [
pyqt5
mutagen
chromaprint
discid
];

prePatch = ''
# Pesky unicode punctuation.
substituteInPlace setup.cfg --replace "‘" "'"
'';

dontWrapPythonPrograms = (qt5.wrapQtAppsHook == null);
# In order to spare double wrapping, we use:
preFixup = ''
makeWrapperArgs+=("''${qtWrapperArgs[@]}")
Expand Down
294 changes: 294 additions & 0 deletions pkgs/build-support/neowrap.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,294 @@
{ lib
, symlinkJoin
, makeWrapper
, lndir
, pkgs
, extraPkgsByOverride ? []
}:

# A single pkg or a list of pkgs mainly packaged
pkgList:

let
wrapper = {
extraPkgs ? [],
extraMakeWrapperArgs ? "",
# An attribute set that tells the builder how to handle links per file /
# directory found in every key of propagateEnv. E.g:
/*** example value ***
linkByEnv = {
# Tells the builder to link the files in every directory that propagates
# XDG_DATA_DIRS.
XDG_DATA_DIRS = "link";
# Tells the builder to simply link all the files of a package that
# propagets this env var. Note that this may affect closure size of the
# result as every pkg's all outputs are used unconditionally. Useful for
# python / other languages wrappings
GI_TYPELIB_PATH = "linkPkg";
},
/*** Another example value ***
linkByEnv ? {
QT_PLUGIN_PATH = "link";
QML2_IMPORT_PATH = "link";
},
*/
# TODO: Decide what to do when `linkPkg` is used here and the file
# nix-support/propagated-build-inputs has different values per package
linkByEnv ? {},
# If we want the wrapping to also include an environmental variable in
# out, we list here for every env var what path to add to the wrapper's
# values.
/*** example value ***
wrapOut ? {
XDG_DATA_DIRS = "$out/share";
PYTHONPATH = "$out/${pkgs.python3.sitePackages}";
},
*/
wrapOut ? {},
}:
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if you consider targetting wrappers in-scope? For example, fwupd needs to exclude EFI files from being wrapped; gjs only wants to wrap installed tests…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Am I to understand you are concerned that wrapGeneric can't be instructed to wrap only some executables and not others?

Copy link
Member

@jtojnar jtojnar Aug 17, 2020

Choose a reason for hiding this comment

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

Yes. Ideally, it should be able to handle multiple disjoint wrappers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely we need to have that ability and it shouldn't be that hard to implement. I still don't have an idea though as to how this interface should look like. I'll update this PR or the RFC once I'll think about something, suggestions are welcome.

let
# Where we keep general knowledge about known to be used environmental variables
encyclopedia = {
# Here we write the separator string between values of every env var, the
# default is ":"
separators = {
# XDG_DATA_DIRS = ":";
};
# If the `link` argument was supplied, we use the keys provided here as a
# dictionary that defines where to symlink the files that the caller
# wants to be symlinked as well, per env var. If a key shows up here and
# in wrapOut as well, it is expected they will have the same value.
linkPaths = {
XDG_DATA_DIRS = "$out/share";
GI_TYPELIB_PATH = "$out/lib/girepository-1.0";
QT_PLUGIN_PATH = "$out/${pkgs.qt5.qtbase.qtPluginPrefix}";
QML2_IMPORT_PATH = "$out/${pkgs.qt5.qtbase.qtQmlPrefix}";
# Not using a strict version here as it may confuse someone digging
# into the results of this wrapper
PYTHONPATH = "$out/python-sitePackages";
};
# If you want an environment variable to have a single value and that's it,
# put it here:
singleValue = [
"GDK_PIXBUF_MODULE_FILE"
];
# To prevent a stack overflow when inspecting a derivation's inputs
# recursively, we use this list to tell the function that dives into the
# dependency graph, what packages should be skipped instantly.
skipDivingInto = with pkgs; [
gnu-efi iptables kexectools libapparmor libcap libidn2 libmicrohttpd
libnetfilter_conntrack libnftnl libseccomp pciutils systemd acl autogen
gnutls guile libevent p11-kit unbound xorg.libXext freetype xorg.libICE
libpng_apng kmod libgcrypt curl libkrb5 libssh2 nghttp2 libtool nettle
glib libselinux utillinux xorg.libX11 xorg.libxcb xorg.libXdmcp libxslt cracklib
linux-pam xorg.libXau libxml2 sqlite readline bison zlib libffi openssl ncurses
# Note we add perl itself here, not any library of it
perl xorg.xorgproto expat xz fontconfig xorg.libXfixes llvm
];
};
# recursive function that goes deep through the dependency graph of a given
# list of packages and creates a list of all buildInputs they all depend
# on. The 2nd argument pkgsFound is used internally and it's expected to be
# [] at the first call.
getAllInputs =
pkgs:
pkgsFound:
map (
pkg:
if builtins.isAttrs pkg then
let
inEncyclopedia =
inputPkg:
lib.lists.any (
knownPkg:
if builtins.isAttrs inputPkg then
# builtins.trace "testing ${inputPkg.name} if in encyclopedia: ${builtins.toJSON (inputPkg.name == knownPkg.name)}" inputPkg.name == knownPkg.name
inputPkg.name == knownPkg.name
else
false
) encyclopedia.skipDivingInto
;
notInEncyclopedia =
inputPkg:
!(inEncyclopedia inputPkg)
;
hasInputs = pkg.buildInputs != [] || pkg.propagatedBuildInputs != [];
in
if !(inEncyclopedia pkg) && hasInputs then
let
# From some reason, pkg == knownPkg doesn't work :/ don't know
# why... Never the less this should be good enough
deeperPkgs = builtins.filter notInEncyclopedia (pkg.buildInputs ++ pkg.propagatedBuildInputs);
Copy link
Member

Choose a reason for hiding this comment

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

How does the neowrapper handle propagation boundaries when buildInputs and propagatedBuildInputs are the same?
For example, libinput depends on pyudev for its utilities but we do not want everything that links against libinput to have pyudev as dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libinput not only has a certain python environment in it's buildInputs, it also references this python env at ${libinput.bin}/libexec/libinput/libinput-measure-fuzz. Note it's the bin output, not the more widely linked against out output.

we do not want everything that links against libinput to have pyudev as dependency.

Agreed. The current design suffers from the inability to distinguish between libinput specified in a drv's buildInputs, vs libinput.out (and not libinput.bin) referenced by a drv. With the ability to distinguish between the two, wrapGenric should be capable of knowing not to add python related env vars to a wrapper, if it's only libinput.out that's referenced.

This issue naturally relates to the unresolved questions of the RFC.

deeperPkgs_ = builtins.trace "deeper is ${builtins.toJSON deeperPkgs}" deeperPkgs;
currentPkgs = pkgsFound ++ pkgs;
in
(getAllInputs deeperPkgs currentPkgs)
else
pkgsFound ++ [ pkg ]
else
[]
) pkgs
;
allPkgs = (lib.lists.flatten pkgList) ++ extraPkgsByOverride ++ extraPkgs;
# Useful for debugging, not evaluated if not used.
allPkgs_ = builtins.trace "allPkgs is: ${builtins.toJSON allPkgs}" allPkgs;
allInputs = lib.lists.unique (lib.lists.flatten (getAllInputs allPkgs []));
# Useful for debugging, not evaluated if not used.
allInputs_ = builtins.trace "allInputs is: ${builtins.toJSON allInputs}" allInputs;
# filter out of all the inputs the packages with the propagateEnv attribute
envPkgs = builtins.filter (
pkg:
if builtins.isAttrs pkg then
(builtins.hasAttr "propagateEnv" pkg)
else
false
) allInputs;
# Useful for debugging, not evaluated if not used.
envPkgs_ = builtins.trace "envPkgs is: ${builtins.toJSON envPkgs}" envPkgs;
# Given a package, it's outputs and an envStr such as found in the values
# of passthru's `propagateEnv`, it replaces all occurences of %<outname>%
# from envStr according to the pkg.outputs
replaceAllOutputs = {
pkg,
envStr,
outputs,
outname ? "out",
}:
let
outname = builtins.elemAt outputs 0;
in
if (builtins.length outputs) == 0 then
envStr
else
replaceAllOutputs {
inherit pkg;
outputs = lib.lists.subtractLists [outname] outputs;
envStr = builtins.replaceStrings
[ "@${outname}@" ]
[ "${pkg.${outname}}" ]
envStr;
}
;
# Where we calculate every env var's values. This also considers the `link`
# argument's value - if it was requested to `link` directories of certain
# env vars or paths, that's taken care of later, at `linkInfo`. The
# encyclopedia's `linkPaths` set is used if needed.
envInfo = lib.attrsets.foldAttrs (n: a: lib.lists.unique ([n] ++ a)) [] (map (
pkg:
# for every package in envPkgs, do the following for every env key and value
(lib.attrsets.mapAttrs (
# env var name
name:
# env var value
value:
let
real_value = replaceAllOutputs {
inherit pkg;
outputs = pkg.outputs;
envStr = value;
};
in
if builtins.hasAttr name linkByEnv then
if linkByEnv.${name} == "link" then
if builtins.hasAttr name encyclopedia.linkPaths then
{
linkType = "normal";
linkTo = encyclopedia.linkPaths.${name};
linkFrom = real_value;
}
else
abort "neowrap.nix: I was requested to symlink paths of propagated environment for env var `${name}` but I don't know where to put these files as they are not in my encyclopedia"
else
if linkByEnv.${name} != "linkPkg" then
abort "neowrap.nix: got an unknown value for a linkByenv env key: ${name}"
else
{
linkType = "pkg";
linkTo = "$out";
# Should this be `pkg.dev` or simply `pkg` ?
linkFrom = pkg.out;
}
else
real_value
) pkg.propagateEnv)
) envPkgs);
# Useful for debugging, not evaluated if not used.
envInfo_ = builtins.trace "envInfo is ${(builtins.toJSON envInfo)}" envInfo;
makeWrapperArgs = lib.lists.flatten (lib.attrsets.mapAttrsToList (
key:
value:
(let
sep = encyclopedia.separators.${key} or ":"; # default separator used for most wrappings
in
# To filter out envInfo changed via the `linkByEnv` argument
if builtins.isString (builtins.elemAt value 0) then
if builtins.elem key encyclopedia.singleValue then
if (builtins.length value) > 1 then
abort "neowrap.nix: There is more then 1 derivation in all inputs of: ${builtins.toJSON allPkgs} that set ${key} to a value via a passthru. This env key, is defined in my encyclopedia as a single value key. Hence I don't know what value to use in wrapping."
else
# Should this be "--set" ?
"--set-default ${key} ${builtins.elemAt value 0}"
else
"--prefix ${key} ${sep} ${builtins.concatStringsSep sep value}"
else
# duplicates are removed by lib.lists.flatten at the beginning
if (builtins.elemAt value 0).linkType == "normal" then
"--prefix ${key} ${sep} ${(builtins.elemAt value 0).linkTo}"
else # (builtins.elemAt value 0).linkType == "pkg" then
if builtins.hasAttr key wrapOut || builtins.hasAttr key encyclopedia.linkPaths then
"--prefix ${key} ${sep} ${wrapOut.${key} or encyclopedia.linkPaths.${key}}"
else
abort "neowrap.nix: When requested to link packages of env var ${key}, I couldn't decide where to link them _to_, either set wrapOut.${key} to e.g $out/${key}, and that directory will be added to the values of ${key}, or add an entry to the encyclopedia.linkPaths attribute set"
)
)
# Before calculating makeWrapperArgs, we need to add values to env vars
# according to the `wrapOut` argument:
(lib.attrsets.mapAttrs (
name:
values:
if builtins.hasAttr name wrapOut && builtins.isList values then
if builtins.elem wrapOut.${name} values then
values
else
values ++ [wrapOut.${name}]
else
values
) envInfo)
);
makeWrapperArgs_ = builtins.trace "makeWrapperArgs is ${builtins.toJSON makeWrapperArgs}" makeWrapperArgs;
linkCmds = lib.lists.flatten (lib.attrsets.mapAttrsToList (
key:
values:
(let
sep = encyclopedia.separators.${key} or ":"; # default separator used for most wrappings
in
if builtins.isAttrs (builtins.elemAt values 0) then
(map (
v:
"mkdir -p ${v.linkTo} && lndir -silent ${v.linkFrom} ${v.linkTo}"
) values)
else
# removed by lib.lists.flatten at the beginning
[]
)
)
envInfo
);
# Useful for debugging, not evaluated if not used.
linkCmds_ = builtins.trace "linkCmds is ${builtins.toJSON linkCmds}" linkCmds;
in
symlinkJoin {
Copy link
Member

@jtojnar jtojnar Aug 16, 2020

Choose a reason for hiding this comment

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

One disadvantage of symlinkJoin is that when absolute path is used in an entry point (e.g. desktop files, systemd services…), it will launch the unwrapped program.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right.. Extra commands should be at postBuild for that to work. I wonder whether it'd be worth the effort to write something generic, or make wrapGeneric accept a extraBuildCommands string.

name = "runtime-env";
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this should inherit the name of the parent derivation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not always there's a single parent derivation, but certainly for most purposes, this would be nicer.

paths = pkgList;
passthru.unwrapped = pkgList;

buildInputs = [ makeWrapper lndir ];
postBuild = ''
${builtins.concatStringsSep "\n" linkCmds}
for i in $out/bin/*; do
wrapProgram "$i" ${builtins.concatStringsSep " " makeWrapperArgs}
done
'';
};
in
lib.makeOverridable wrapper
3 changes: 3 additions & 0 deletions pkgs/development/libraries/atk/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ stdenv.mkDerivation rec {
updateScript = gnome3.updateScript {
packageName = pname;
};
propagateEnv = {
GI_TYPELIB_PATH = "@out@/lib/girepository-1.0";
};
};

meta = {
Expand Down
3 changes: 3 additions & 0 deletions pkgs/development/libraries/dconf/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ stdenv.mkDerivation rec {
updateScript = gnome3.updateScript {
packageName = pname;
};
propagateEnv = {
GIO_EXTRA_MODULES = "@lib@/lib/gio/modules";
};
};

meta = with stdenv.lib; {
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/libraries/gdk-pixbuf/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ in stdenv.mkDerivation rec {

# gdk_pixbuf_moduledir variable from gdk-pixbuf-2.0.pc
moduleDir = "lib/gdk-pixbuf-2.0/2.10.0/loaders";
cacheFile = "lib/gdk-pixbuf-2.0/2.10.0/loaders.cache";
propagateEnv = {
GI_TYPELIB_PATH = "@out@/lib/girepository-1.0";
};
};

meta = with stdenv.lib; {
Expand Down
3 changes: 3 additions & 0 deletions pkgs/development/libraries/gobject-introspection/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ stdenv.mkDerivation rec {
updateScript = gnome3.updateScript {
packageName = pname;
};
propagateEnv = {
GI_TYPELIB_PATH = "@out@/lib/girepository-1.0";
};
};

meta = with stdenv.lib; {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ stdenv.mkDerivation rec {

passthru = {
updateScript = gnome3.updateScript { packageName = "gsettings-desktop-schemas"; };
propagateEnv = {
XDG_DATA_DIRS = "@out@/share/gsettings-schemas/${pname}-${version}";
GI_TYPELIB_PATH = "@out@/lib/girepository-1.0";
};
};

# meson installs the schemas to share/glib-2.0/schemas
Expand Down
6 changes: 6 additions & 0 deletions pkgs/development/libraries/gstreamer/bad/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@ in stdenv.mkDerivation rec {
# that trip up clang with format security enabled.
hardeningDisable = [ "format" ];

passthru = {
propagateEnv = {
GST_PLUGIN_SYSTEM_PATH_1_0 = "@out@/lib/gstreamer-1.0";
};
};

doCheck = false; # fails 20 out of 58 tests, expensive

meta = with stdenv.lib; {
Expand Down
Loading