-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
General problems with environment variable wrappers #60260
Comments
I have an alternative design in mind, where we get rid of wrappers, instead packages declare in But it does have the problem that when a profile is switched ( |
Remember we also need to support users outside of NixOS. They could be running the binary with any environment. |
Yeah, that is indeed one aspect of the problem. Wrappers do directly address this, but at the expense of creating a mess with environment variables, including when running in NixOS where this may be unneeded if suitable stuff is included in the profile. I'm wondering if we could, in the long term, drop support for running GUI apps "directly", outside of an environment, and provide a practical solution to encapsulate them in an environment. Like, the environment could be the one that generates correct wrappers, not the package itself. |
This would solve "3. Wrappers obscure the process name". It is also more flexible because all packages in the environment (such as whether you are running a specific desktop) could contribute to the variables set in other wrappers, e.g. including the KDE desktop in the environment could ensure that all other apps get support for SVG icons. Or special configuration options (e.g. support these GVFS plugins) could adjust some variable in all wrappers. |
By the way, "Wrappers obscure the process name" can already be solved relatively easily, by putting the executable into a subdirectory, e.g. |
Might be worth trying, but there could be some issues with relative paths, symlinks, etc. To be clear, I 100% agree with the problems you are describing, I just know from experience that there's not a good alternative. I think we could look into using a C-based wrapper instead to prevent the issues with script. This is a little bit less readable, but could solve 3 & 5. This is also an issue on macOS with python shebangs. |
Not sure about this. How does it help with (3)? And it also does not solve (5) in the sense that the program opening the executable will open the wrong executable (the wrapper) not the one it wanted to. However, whatever we do with wrappers, we do not address "(1) Environment variables are propagated to child processes". We could look into more invasive approaches. For example, the wrapper could pass all such "variables" via a single specific environment variable (e.g. |
Ah you probably mean the ability to specify The idea for a C-based wrapper seems good. It could also help solving (2) (not adding to list if already present) more elegantly. I will now try to implement this, probably with C++, to be used as an |
#60229 is a particular recent manifestation of some of the problems outlined here - when the program restarts itself it calls the wrapper so the wrapper does all the adjustments after they were already done. In that specific case the problem is actually adding already-added command line arguments, but it must also be adding to environment variables I see that the current shell wrappers already set Maybe fiddling with |
Another problem with the current wrappers is that since it uses bash it cannot work with setuid. This would be solved by the use of a C-based wrapper. I've made some progress making a C++ wrapper, but it cannot be a drop-in replacement because it's not bash. For example:
In general it seems like like can be done, but some cases would need to be handled differently, possibly by patching the application code or by sticking with the bash wrapper. |
Here's a prototype of a C++ based wrapper. There is one executable which is both the interpreter for wrappers and a tool to generate the wrapper file. The latter is so because escaping strings correctly in bash would be a pain. It works for a few test case packages. Prefix and suffix functionality work such that any previous occurrences of added elements are removed. I don't think it makes sense to go replacing the current wrappers with this. It would be some work and not much benefit (which is the check for existing occurrences of elements, and perhaps a small performance improvement). Many use cases could not be handled because it doesn't let you run bash code. On the other hand I like this limitation because it enforces some sanity. This wrapper could be a good starting point for a system where |
About 5. I have encountered issues with wine due to this on several occasions. The more severe of them is that when you create a 64-bit wine prefix with the package wineWowPackages.full it will create a prefix with ONLY 64-bit dlls. The syswow64 folder is completely empty. This may cause alot of issues and needs to be dealt with. If you want to see this yourself I have made a nix-shell command that reproduces and logs only 64-bit dlls being copied and the error where wine can't be loaded as an ELF executable: |
I would like to have a binary that reads a Sample:
the problem, as usual, is finding the path to the file, although that one could be given as argument to the wrapper binary. |
This comment has been minimized.
This comment has been minimized.
This issue has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/include-custom-environment-variable-into-wrapper/8229/2 |
I would like to share my point of view which is a bit different than those presented here. What if instead of focusing on improving wrappers we try to get rid of the problem they are solving. For example |
@majkrzak I personally would agree, environmental pollution as a means to fix those goes against hermetic package management. See #44047 for a past attempt at this for Qt. But note that maintainers are, it seems, generally not interested in patching upstream software to deal with this. (I personally think it's wrong, since it ends up that every leaf package has to know and deal with intricacies of the packaging of the dependency.) |
Clearly, that is preferable. Except it then becomes a language/framework/package specific task, and still something is needed to put that together. |
I was bitten by this recently (Emacs propagated an incompatible |
Another fresh issue for me: This is causing issues with at the very least KWin, but it is likely other KDE or Plasma components assume the path will be the actual binary. This is a situation where even tampering with argv0 won't help. See #116549 for a fix for KWin. I wouldn't be surprised if there were other similar issues, in KDE frameworks, Plasma, and elsewhere. |
@timor most likely by injecting it to: https://gitlab.gnome.org/GNOME/gdk-pixbuf/-/blob/master/gdk-pixbuf/queryloaders.c#L307 and https://gitlab.gnome.org/GNOME/gdk-pixbuf/-/blob/master/gdk-pixbuf/gdk-pixbuf-io.c#L376 or by playing with |
Small question: concerning problem 1, can we always know when the environment variable should be propagated to the child process or not? I guess sometimes we do want to propagate them to the child process (for example in a shell). I'm also hijacking into this issue because of this bug, which adds a 6th issue with wrappers:
This is a problem because some programs (carla and NSM in my case) relies on the fact that If we can get rid of wrappers, it would be really great (not sure how it is realistic). If it's not possible, I'm thinking (but I'm not an expert at all, so it may be complete non-sense), that there may be another solution: instead of setting the environment variables in a wrapper script, one may try to associate to any executable
Then, the idea would be to change slightly GLIBC in order to take care of that file: when a new file (say Advantages:
Drawbacks: On the other side, maintaining wrappers is also quite a nightmare, so maybe spending a bit more time in GLIBC, but less time in the wrappers could make sense. And if we are ready to maintain a different GLIBC, maybe we could even avoid the need to patch proprietary executables (except for changing the interpreter path), by directly asking to GLIBC to find the libraries in the good place depending on our Mitigate the drawbacks: Do you think it can work? Is it realistic, or just a nightmare to maintain? (either the fork, or the custom -- EDIT -- |
Actually, to wrap scripts (and solve problem 6, and 3 for scripts, we still need to consider scripts and binaries differently), I found a trick to avoid the need to patch glibc and is not too hard to implement I guess: instead of externalizing the script, I externalize the wrapper by defining a custom (script) interpreter whose role is to first configure the environment variable (the language of this interpreter can be arbitrary, I just use bash), and then call the original interpreter (it could be any interpreter: bash, python...). This solution is interesting because it does not depend on the script used, and it does not change the value of
#!./my_python_wrapper
# The above line can be replaced with any file like /nix/store/xxx-mysoftware/wrappers/my_python_wrapper
import sys
import os
print("Here is my name {}, the variable MYENV is {}".format(sys.argv[0], os.environ["MYENV"]))
#!/usr/bin/env bash
# Setup environment variables...
export MYENV="WIN :-)"
# Call the original wrapper, and forward all arguments, /usr/bin/env will be replaced in practice with the actual /nix/store/xxx-python3/bin/python interpreter.
exec /usr/bin/env python3 "$@" Demo:
PS: note that you do need to put an absolute path for the wrapper if you want to be able to call the executable from multiples folders... So it means that it will be hard in practice to change the location of the executable. However, in practice I guess it is not very important since anyway many path are hardcoded in the |
I marked this as stale due to inactivity. → More info |
Still interested. |
Thanks. But will it only fix a MacOs-specific issue, or will this script actually think it is not wrapped? Like what happens if a wrapped script writes |
I checked and the recent PR does not solve the issue I mentioned. |
This issue has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/terminal-emulator-leaks-environment-variables-to-shell/33673/11 |
This has the primary benefit of preventing the name of the wrapper from leaking into the UI and process info (see #60260 for more info).
This has the primary benefit of preventing the name of the wrapper from leaking into the UI and process info (see NixOS#60260 for more info).
The use of wrapper scripts to set or change environment variables before running the real executable has become very widespread in nixpkgs. While they are important for making programs run out-of-the-box, wrappers have some general problems. Some of these could be fixed but some are perhaps unfixable. I'm writing this to generate some discussion to see if we could solve some of these issues or come up with an alternative solution.
Environment variables are propagated to child processes. I believe this is the most problematic thing. The intent of the wrapper is to make the wrapped program run correctly, and the fact that child processes see the adjustment is unintended leakage of information/configuration. Any child process started by the wrapped program that also needs environment changes will generally have its own wrapper. This leakage could cause another application to run incorrectly or differently than if it was started "cleanly".
Wrappers which add to list-type variables (typically colon-separated) will add the value unconditionally without checking if it is already in the list.
To observe (1) and (2) in action, open Konsole and run
echo "$XDG_DATA_DIRS"
. Then typekonsole
in the same terminal and run the echo in the new terminal. You will see that the variable contains two entries for<konsole-store-path>/share
. You can keep repeating this to getXDG_DATA_DIRS
that grows without bounds.Wrappers obscure the process name. Just look at a process list and find many
.something-wrapped
processes.People forget to make wrappers and/or don't know which variables need to be set. For example, probably most GTK apps need to have
GDK_PIXBUF_MODULE_FILE
set to point to some file in thelibrsvg
package. It is far from obvious to a random person packaging an app that this is necessary (see gdk-pixbuf: Use a different GDK_PIXBUF_MODULE_FILE environment variable on 32-bit and 64-bit systems. #60254 and Use a NixOS module for generating the gdk-pixbuf loaders cache. #42562).Wrappers break software that opens the executable and expects it to be an ELF binary. I have encountered this with Wine but I have not yet filed an issue (it results in very suble but widespread breakage). But this is uncommon.
The text was updated successfully, but these errors were encountered: