chromium: Do not rely on import-from-derivation

This has been introduced by me in 690a845 and discovered by @vcunat in
his comment over at:

https://github.com/NixOS/nixpkgs/commit/690a845de93a49e85480ad5fae492fa14658ce85#commitcomment-14209868

It's really a bit ugly to have builds running during evaluation, but
back when I made that commit the reason was to avoid having to shell
quote the hell out of it (see the comment in mkPluginInfo for the
reason).

Now we propagate plugin flags and environment variables as a list of
arguments in a plain file that's appended verbatim to makeWrapper, so
it shouldn't do any builds anymore during instantiation.

I have tested this with both just WideVine and just Flash enabled as
well as both in combination and none of the plugins and the output seems
correct. However I didn't test to run Chromium with the new
implementation.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Reported-by: Vladimír Čunát <vcunat@gmail.com>

2 files changed, 48 insertions, 34 deletions

diff --git a/pkgs/applications/networking/browsers/chromium/default.nix b/pkgs/applications/networking/browsers/chromium/default.nix
index bf870e144e56..c0c5e485e59c 100644
--- a/pkgs/applications/networking/browsers/chromium/default.nix
+++ b/pkgs/applications/networking/browsers/chromium/default.nix
@@ -64,20 +64,17 @@ let
 in stdenv.mkDerivation {
   name = "chromium${suffix}-${chromium.browser.version}";
 
-  buildInputs = [ makeWrapper ] ++ chromium.plugins.enabledPlugins;
+  buildInputs = [ makeWrapper ];
 
   buildCommand = let
     browserBinary = "${chromium.browser}/libexec/chromium/chromium";
-    mkEnvVar = key: val: "--set '${key}' '${val}'";
-    envVars = chromium.plugins.settings.envVars or {};
-    flags = chromium.plugins.settings.flags or [];
+    getWrapperFlags = plugin: "$(< \"${plugin}/nix-support/wrapper-flags\")";
   in with stdenv.lib; ''
     mkdir -p "$out/bin" "$out/share/applications"
 
     ln -s "${chromium.browser}/share" "$out/share"
-    makeWrapper "${browserBinary}" "$out/bin/chromium" \
-      ${concatStrings (mapAttrsToList mkEnvVar envVars)} \
-      --add-flags "${concatStringsSep " " flags}"
+    eval makeWrapper "${browserBinary}" "$out/bin/chromium" \
+      ${concatMapStringsSep " " getWrapperFlags chromium.plugins.enabled}
 
     ln -s "$out/bin/chromium" "$out/bin/chromium-browser"
     ln -s "${chromium.browser}/share/icons" "$out/share/icons"
diff --git a/pkgs/applications/networking/browsers/chromium/plugins.nix b/pkgs/applications/networking/browsers/chromium/plugins.nix
index 0b0e5bd08382..dda976905610 100644
--- a/pkgs/applications/networking/browsers/chromium/plugins.nix
+++ b/pkgs/applications/networking/browsers/chromium/plugins.nix
@@ -8,6 +8,34 @@
 with stdenv.lib;
 
 let
+  # Generate a shell fragment that emits flags appended to the
+  # final makeWrapper call for wrapping the browser's main binary.
+  #
+  # Note that this is shell-escaped so that only the variable specified
+  # by the "output" attribute is substituted.
+  mkPluginInfo = { output ? "out", allowedVars ? [ output ]
+                 , flags ? [], envVars ? {}
+                 }: let
+    shSearch = ["'"] ++ map (var: "\$${var}") allowedVars;
+    shReplace = ["'\\''"] ++ map (var: "'\"\${${var}}\"'") allowedVars;
+    # We need to triple-escape "val":
+    #  * First because makeWrapper doesn't do any quoting of its arguments by
+    #    itself.
+    #  * Second because it's passed to the makeWrapper call separated by IFS but
+    #    not by the _real_ arguments, for example the Widevine plugin flags
+    #    contain spaces, so they would end up as separate arguments.
+    #  * Third in order to be correctly quoted for the "echo" call below.
+    shEsc = val: "'${replaceStrings ["'"] ["'\\''"] val}'";
+    mkSh = val: "'${replaceStrings shSearch shReplace (shEsc val)}'";
+    mkFlag = flag: ["--add-flags" (shEsc flag)];
+    mkEnvVar = key: val: ["--set" (shEsc key) (shEsc val)];
+    envList = mapAttrsToList mkEnvVar envVars;
+    quoted = map mkSh (flatten ((map mkFlag flags) ++ envList));
+  in ''
+    mkdir -p "''$${output}/nix-support"
+    echo ${toString quoted} > "''$${output}/nix-support/wrapper-flags"
+  '';
+
   plugins = stdenv.mkDerivation {
     name = "chromium-binary-plugins";
 
@@ -61,40 +89,29 @@ let
 
       install -vD PepperFlash/libpepflashplayer.so \
         "$flash/lib/libpepflashplayer.so"
-      mkdir -p "$flash/nix-support"
-      cat > "$flash/nix-support/chromium-plugin.nix" <<NIXOUT
-        { flags = [
-            "--ppapi-flash-path='$flash/lib/libpepflashplayer.so'"
-            "--ppapi-flash-version=$flashVersion"
-          ];
-        }
-      NIXOUT
+
+      ${mkPluginInfo {
+        output = "flash";
+        allowedVars = [ "flash" "flashVersion" ];
+        flags = [
+          "--ppapi-flash-path=$flash/lib/libpepflashplayer.so"
+          "--ppapi-flash-version=$flashVersion"
+        ];
+      }}
 
       install -vD libwidevinecdm.so \
         "$widevine/lib/libwidevinecdm.so"
       install -vD libwidevinecdmadapter.so \
         "$widevine/lib/libwidevinecdmadapter.so"
-      mkdir -p "$widevine/nix-support"
-      cat > "$widevine/nix-support/chromium-plugin.nix" <<NIXOUT
-        { flags = [ "--register-pepper-plugins='${wvModule}${wvInfo}'" ];
-          envVars.NIX_CHROMIUM_PLUGIN_PATH_WIDEVINE = "$widevine/lib";
-        }
-      NIXOUT
+
+      ${mkPluginInfo {
+        output = "widevine";
+        flags = [ "--register-pepper-plugins=${wvModule}${wvInfo}" ];
+        envVars.NIX_CHROMIUM_PLUGIN_PATH_WIDEVINE = "$widevine/lib";
+      }}
     '';
 
-    passthru = let
-      enabledPlugins = optional enablePepperFlash plugins.flash
+    passthru.enabled = optional enablePepperFlash plugins.flash
                     ++ optional enableWideVine    plugins.widevine;
-      getNix = plugin: import "${plugin}/nix-support/chromium-plugin.nix";
-      mergeAttrsets = let
-        f = v: if all isAttrs v then mergeAttrsets v
-          else if all isList  v then concatLists   v
-          else if tail v == []  then head          v
-          else head (tail v);
-      in fold (l: r: zipAttrsWith (_: f) [ l r ]) {};
-    in {
-      inherit enabledPlugins;
-      settings = mergeAttrsets (map getNix enabledPlugins);
-    };
   };
 in plugins