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
wrapGAppsHook: don't add empty variables (see also #75443) #75450
wrapGAppsHook: don't add empty variables (see also #75443) #75450
Conversation
Adding empty variables can lead to this problem: ```diff wrapProgram \ ./pye_menu_shell \ --prefix PATH : /nix/store/4c3z5r6yxsf2cxwwyazhdn92xixn4j5b-python3-3.7.5/bin:/nix/store/b3l3niilvqcxcsbxmd6sgqk1dy1rk81c-pye-menu-1.0/bin:/nix/store/y8j1cfj8d9r5rbbxc22w7hnfjw5f4fd3-cairo-1.16.0-dev/bin:/nix/store/6mg7lfbdh9pgx7pbxr3544qqbrigdl1q-freetype-2.10.1-dev/bin:/nix/store/gpszqcy0xi0lavbbjdq82zkkjp3jbp2a-bzip2-1.0.6.0.1-bin/bin:/nix/store/031c5pk5lzabgmpqpyd46hzi625as6bp-libpng-apng-1.6.37-dev/bin:/nix/store/f8kl7kmpv130aw9zm542p74a3hg0yc13-fontconfig-2.12.6-bin/bin:/nix/store/bqp30vkncmm222mjvwggz0s7p318sflj-expat-2.2.7-dev/bin:/nix/store/w57xa8g4s4aviwmqwgra7m5hwj2b005m-glib-2.60.7-dev/bin:/nix/store/v5d4966ahvfir2hwpv003022f3pb7vik-gettext-0.19.8.1/bin:/nix/store/qpvxhl1jr0fxnrx9idnpdagqs00m5m2z-glib-2.60.7/bin \ --set PYTHONNOUSERSITE true \ --set GDK_PIXBUF_MODULE_FILE /nix/store/7ddlakx6xjczqbfs80xjd14f30fzadws-gdk-pixbuf-2.38.1/lib/gdk-pixbuf-2.0/2.10.0/loaders.cache \ --prefix XDG_DATA_DIRS : /nix/store/0snjc1qg89zqn3v35l9d55xrykh9nj5c-gtk+3-3.24.10/share/gsettings-schemas/gtk+3-3.24.10:/nix/store/b41z51vdv11n6df8ki5vj8dynxw98f9l-gsettings-desktop-schemas-3.32.0/share/gsettings-schemas/gsettings-desktop-schemas-3.32.0:/nix/store/0snjc1qg89zqn3v35l9d55xrykh9nj5c-gtk+3-3.24.10/share/gsettings-schemas/gtk+3-3.24.10 \ - --prefix GST_PLUGIN_SYSTEM_PATH_1_0 : \ + --prefix GST_PLUGIN_SYSTEM_PATH_1_0 : "" \ --prefix GI_TYPELIB_PATH : /nix/store/0snjc1qg89zqn3v35l9d55xrykh9nj5c-gtk+3-3.24.10/lib/girepository-1.0:/nix/store/z29l5xaaxh1s0697mcldj71ab0zshry1-librsvg-2.44.15/lib/girepository-1.0:/nix/store/pija1xzm7izxfb5m2hvhvlwp1l38ffxa-gobject-introspection-1.60.2/lib/girepository-1.0 \ - --prefix GRL_PLUGIN_PATH : + --prefix GRL_PLUGIN_PATH : "" ``` Where the diff is to highlight the problem: we don't have a valid value for GST_PLUGIN_SYSTEM_PATH_1_0 or GRL_PLUGIN_PATH, and instead of passing the empy string, the empty string gets unquoted somewhere, so we end up passing no arguments, thus the parser in wrapProgram takes --prefix as the argument of GST_PLUGIN_SYSTEM_PATH_1_0, and then GI_TYPELIB_PATH is missing it's --prefix so wrapProgram complains/dies. The easiest change is to not add empty arguments to the wrapper
Draft pull-request while I figure out how to test this properly, I feel like this might impact a lot of packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is a good idea.
While makeWrapper
will ignore empty values to prefix, passing them does not make much sense. Also, we guard the other env vars, so we should do it here for consistency.
Though the issue still occurred from improper use due to bad documentation. We should fix that as well.
Verified that it fixes your project build, even with the broken diff --git a/default.nix b/default.nix
index e0b2c15..a168ea8 100644
--- a/default.nix
+++ b/default.nix
@@ -1,11 +1,17 @@
let nixpkgs = import <nixpkgs> {};
in
-{ python3 ? nixpkgs.python3,
- gtk3 ? nixpkgs.gtk3,
+{ python3 ? nixpkgs.python3, librsvg ? nixpkgs.librsvg, makeSetupHook ? nixpkgs.makeSetupHook, makeWrapper ? nixpkgs.makeWrapper
+ ,gtk3 ? nixpkgs.gtk3,
gobject-introspection ? nixpkgs.gobject-introspection,
wrapGAppsHook ? nixpkgs.wrapGAppsHook,
lib ? nixpkgs.lib
}:
+
+let
+ wrapGAppsHook = makeSetupHook {
+ deps = [ gtk3 librsvg makeWrapper ];
+ } /home/jtojnar/Projects/nixpkgs/pkgs/build-support/setup-hooks/wrap-gapps-hook2.sh;
+in
python3.pkgs.buildPythonPackage rec {
pname = "pye-menu";
version = "1.0";
|
Done in #75465 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I actually had the same diff in a stash when I noticed it weeks ago.
The build failure isn't valid, I can build this hook locally no problem. (rebased) |
Adding empty variables can lead to this problem:
Where the diff is to highlight the problem: we don't have a valid value
for GST_PLUGIN_SYSTEM_PATH_1_0 or GRL_PLUGIN_PATH, and instead of
passing the empy string, the empty string gets unquoted somewhere, so we
end up passing no arguments, thus the parser in wrapProgram takes
--prefix as the argument of GST_PLUGIN_SYSTEM_PATH_1_0, and then
GI_TYPELIB_PATH is missing it's --prefix so wrapProgram complains/dies.
The easiest change is to not add empty arguments to the wrapper
See also #75443
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @