Short version: environment variables cannot be set in a multi-threaded environment. It is ridiculous, and it has been a problem since the environment was invented, and no one has a solution for it, and computers are the absolute worst.
There is one environment variable that was cannot get around having to set. Python's gettext module, to determine the language to return, searches $LANGUAGE, $LC_ALL, $LC_MESSAGES, and $LANG. This is completely different from gettext() and friends in glibc, which, if there is nothing in $LANGUAGE, uses the locale data set by setlocale, which is thread-safe. This means that if we want translations to work in any python module that we do not control, we need to modify the environment.
So, possible solutions: 1) write thread-safe versions of get/set/put/unsetenv, shoehorn it in with $LD_PRELOAD 2) don't set any environment variables at all after starting threads, monkey-patch the gettext module to do the right thing 3) don't set most environment variables, cross fingers and burn some sage before setting environment variables
These patches implement #3.
The last one is just a bonus. Did you know that nm-connection-editor wasn't run in the language the user selected, but it totally could be? Now it is.
All this needs to be ported to RHEL 7.2 with Related: rhbz#1188287
David Shea (9): Add a pylint module to look for modifications to the environment Set $DISPLAY before threads are started. Add a method to set environment variables for child processes Add a parameter to iutil.startProgram to extend the environment Avoid setting $LANG and $LANGUAGE, except where we can't Assume that a bunch of digits in a version number is a timestamp Do not set $TZ Set $LIBUSER_CONF early Add an option to startProgram to not reset the locale
anaconda | 61 +++++++++------- pyanaconda/constants.py | 3 + pyanaconda/ihelp.py | 2 +- pyanaconda/isys/__init__.py | 17 +++-- pyanaconda/iutil.py | 34 +++++++-- pyanaconda/kickstart.py | 11 ++- pyanaconda/localization.py | 11 ++- pyanaconda/packaging/__init__.py | 4 +- pyanaconda/rescue.py | 6 +- pyanaconda/timezone.py | 9 +++ pyanaconda/ui/gui/__init__.py | 2 +- pyanaconda/ui/gui/spokes/datetime_spoke.py | 10 +-- pyanaconda/ui/gui/spokes/network.py | 4 +- pyanaconda/ui/gui/spokes/welcome.py | 36 ++++++++-- pyanaconda/users.py | 25 +++---- pyanaconda/vnc.py | 19 +++-- scripts/anaconda-yum | 1 + tests/pylint/environ.py | 107 +++++++++++++++++++++++++++++ tests/pylint/pylint-one.sh | 2 +- utils/handle-sshpw | 2 - widgets/configure.ac | 2 +- widgets/src/BaseWindow.c | 6 +- widgets/src/BaseWindow.h | 2 +- widgets/src/StandaloneWindow.c | 5 +- widgets/src/StandaloneWindow.h | 2 +- 25 files changed, 284 insertions(+), 99 deletions(-) create mode 100644 tests/pylint/environ.py
The environment cannot be safely modified in a multi-threaded program, because it has the potential to crash getenv calls, and we can't control calls to getenv because basically everything uses getenv.
For the easy fixes, mark setupEnvironment as safe since it's run before threads are started, mark the environment cleanup in anaconda-yum as safe since it's run in its own, single-threaded process, and remove the environment cleanup at the end of handle-sshpw because it doesn't do anything. --- anaconda | 4 ++ scripts/anaconda-yum | 1 + tests/pylint/environ.py | 107 +++++++++++++++++++++++++++++++++++++++++++++ tests/pylint/pylint-one.sh | 2 +- utils/handle-sshpw | 2 - 5 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 tests/pylint/environ.py
diff --git a/anaconda b/anaconda index cd28667..ae8415e 100755 --- a/anaconda +++ b/anaconda @@ -388,6 +388,10 @@ def setupPythonPath(): sys.path.extend(ADDON_PATHS)
def setupEnvironment(): + # This method is run before any threads are started, so this is the one + # point where it's ok to modify the environment. + # pylint: disable=environment-modify + # Silly GNOME stuff if 'HOME' in os.environ and not "XAUTHORITY" in os.environ: os.environ['XAUTHORITY'] = os.environ['HOME'] + '/.Xauthority' diff --git a/scripts/anaconda-yum b/scripts/anaconda-yum index 82d7203..cf58d1a 100755 --- a/scripts/anaconda-yum +++ b/scripts/anaconda-yum @@ -82,6 +82,7 @@ def run_yum_transaction(release, arch, yum_conf, install_root, ts_file, script_l from yum.Errors import PackageSackError, RepoError, YumBaseError, YumRPMTransError
# remove some environmental variables that can cause problems with package scripts + # pylint: disable=environment-modify env_remove = ('DISPLAY', 'DBUS_SESSION_BUS_ADDRESS') for k in env_remove: if k in os.environ: diff --git a/tests/pylint/environ.py b/tests/pylint/environ.py new file mode 100644 index 0000000..ffbed49 --- /dev/null +++ b/tests/pylint/environ.py @@ -0,0 +1,107 @@ +# setenv pylint module +# +# Copyright (C) 2015 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): David Shea dshea@redhat.com +# + +import astroid + +from pylint.checkers import BaseChecker +from pylint.checkers.utils import check_messages, safe_infer +from pylint.interfaces import IAstroidChecker + +import os + +class EnvironChecker(BaseChecker): + __implements__ = (IAstroidChecker,) + name = "environ" + msgs = {"W9940" : ("Found potentially unsafe modification of environment", + "environment-modify", + "Potentially thread-unsafe modification of environment")} + + def _is_environ(self, node): + # Guess whether a node being modified is os.environ + + if isinstance(node, astroid.Getattr): + if node.attrname == "environ": + expr_node = safe_infer(node.expr) + if isinstance(expr_node, astroid.Module) and expr_node.name == "os": + return True + + # If the node being modified is just "environ" assume that it's os.environ + if isinstance(node, astroid.Name): + if node.name == "environ": + return True + + return False + + @check_messages("environment-modify") + def visit_assign(self, node): + if not isinstance(node, astroid.Assign): + return + + # Look for os.environ["WHATEVER"] = something + for target in node.targets: + if not isinstance(target, astroid.Subscript): + continue + + if self._is_environ(target.value): + self.add_message("environment-modify", node=node) + + @check_messages("environment-modify") + def visit_callfunc(self, node): + # Check both for uses of os.putenv and os.setenv and modifying calls + # to the os.environ object, such as os.environ.update + + if not isinstance(node, astroid.CallFunc): + return + + function_node = safe_infer(node.func) + if not isinstance(function_node, (astroid.Function, astroid.BoundMethod)): + return + + # If the function is from the os or posix modules, look for calls that + # modify the environment + if function_node.root().name in ("os", os.name) and \ + function_node.name in ("putenv", "unsetenv"): + self.add_message("environment-modify", node=node) + + # Look for methods bound to the environ dict + if isinstance(function_node, astroid.BoundMethod) and \ + isinstance(function_node.bound, astroid.Dict) and \ + function_node.bound.root().name in ("os", os.name) and \ + function_node.bound.name == "environ" and \ + function_node.name in ("clear", "pop", "popitem", "setdefault", "update"): + self.add_message("environment-modify", node=node) + + @check_messages("environment-modify") + def visit_delete(self, node): + if not isinstance(node, astroid.Delete): + return + + # Look for del os.environ["WHATEVER"] + for target in node.targets: + if not isinstance(target, astroid.Subscript): + continue + + if self._is_environ(target.value): + self.add_message("environment-modify", node=node) + +def register(linter): + """required method to auto register this checker """ + linter.register_checker(EnvironChecker(linter)) diff --git a/tests/pylint/pylint-one.sh b/tests/pylint/pylint-one.sh index 7c7b8c2..d21df61 100755 --- a/tests/pylint/pylint-one.sh +++ b/tests/pylint/pylint-one.sh @@ -18,7 +18,7 @@ pylint_output="$(pylint \ --dummy-variables-rgx=_ \ --ignored-classes=DefaultInstall,Popen,QueueFactory,TransactionSet \ --defining-attr-methods=__init__,_grabObjects,initialize,reset,start,setUp \ - --load-plugins=intl,preconf,markup,eintr,pointless-override \ + --load-plugins=intl,preconf,markup,eintr,pointless-override,environ \ --init-hook=\ 'import gi.overrides, os; gi.overrides.__path__[0:0] = (os.environ["ANACONDA_WIDGETS_OVERRIDES"].split(":") if "ANACONDA_WIDGETS_OVERRIDES" in os.environ else [])' \ diff --git a/utils/handle-sshpw b/utils/handle-sshpw index 8965572..0616acf 100755 --- a/utils/handle-sshpw +++ b/utils/handle-sshpw @@ -52,5 +52,3 @@ for ud in userdata: kwargs = ud.__dict__ kwargs.update({"root": "/", "mkmailspool": False}) u.createUser(ud.username, **kwargs) - -del(os.environ["LIBUSER_CONF"])
$DISPLAY is effectively a constant, so set it early, before the network thread is started, using a value in pyanaconda.constants. --- anaconda | 22 +++++++++++----------- pyanaconda/constants.py | 3 +++ pyanaconda/vnc.py | 19 +++++++++---------- 3 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/anaconda b/anaconda index ae8415e..d2cb178 100755 --- a/anaconda +++ b/anaconda @@ -120,13 +120,10 @@ def startX11():
# Start Xorg and wait for it become ready iutil.startX(["Xorg", "-br", "-logfile", "/tmp/X.log", - ":1", "vt6", "-s", "1440", "-ac", + ":%s" % constants.X_DISPLAY_NUMBER, "vt6", "-s", "1440", "-ac", "-nolisten", "tcp", "-dpi", "96", "-noreset"], output_redirect=xfd)
- # Now that X is started, make $DISPLAY available - os.environ["DISPLAY"] = ":1" - # function to handle X startup special issues for anaconda def doStartupX11Actions(): """Start window manager""" @@ -410,6 +407,12 @@ def setupEnvironment(): if "LD_PRELOAD" in os.environ: del os.environ["LD_PRELOAD"]
+ # Go ahead and set $DISPLAY whether we're going to use X or not + if 'DISPLAY' in os.environ: + flags.preexisting_x11 = True + else: + os.environ["DISPLAY"] = ":%s" % constants.X_DISPLAY_NUMBER + def setupLoggingFromOpts(options): if (options.debug or options.updateSrc) and not options.loglevel: # debugging means debug logging if an explicit level hasn't been st @@ -618,9 +621,6 @@ def setupDisplay(anaconda, options, addons=None): stdoutLog.warning("Not asking for VNC because we don't have Xvnc") flags.vncquestion = False
- if 'DISPLAY' in os.environ: - flags.preexisting_x11 = True - # Should we try to start Xorg? want_x = anaconda.displayMode == 'g' and \ not (flags.preexisting_x11 or flags.usevnc) @@ -966,10 +966,10 @@ if __name__ == "__main__":
log.error("%s already exists, exiting", pidfile_path)
- # If we have a $DISPLAY and zenity is available, we may be running in - # a live environment and we can display an error dialog. Otherwise - # just print an error. - if "DISPLAY" in os.environ and os.access("/usr/bin/zenity", os.X_OK): + # If we had a $DISPLAY at start and zenity is available, we may be + # running in a live environment and we can display an error dialog. + # Otherwise just print an error. + if flags.preexisting_x11 and os.access("/usr/bin/zenity", os.X_OK): # The module-level _() calls are ok here because the language may # be set from the live environment in this case, and anaconda's # language setup hasn't happened yet. diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py index 46e00f2..26ed16b 100644 --- a/pyanaconda/constants.py +++ b/pyanaconda/constants.py @@ -174,3 +174,6 @@ IPMI_FAILED = 0xA # installation hit an exception # for how long (in seconds) we try to wait for enough entropy for LUKS # keep this a multiple of 60 (minutes) MAX_ENTROPY_WAIT = 10 * 60 + +# X display number to use +X_DISPLAY_NUMBER = 1 diff --git a/pyanaconda/vnc.py b/pyanaconda/vnc.py index 9f7d1c8..473c16c 100644 --- a/pyanaconda/vnc.py +++ b/pyanaconda/vnc.py @@ -55,11 +55,10 @@ def shutdownServer():
class VncServer:
- def __init__(self, display="1", root="/", ip=None, name=None, + def __init__(self, root="/", ip=None, name=None, password="", vncconnecthost="", vncconnectport="", log_file="/tmp/vncserver.log", pw_file="/tmp/vncpassword"): - self.display = display self.root = root self.ip = ip self.name = name @@ -123,9 +122,11 @@ class VncServer: name_ips = [i[4][0] for i in socket.getaddrinfo(self.name, 0)] if self.name is not None and not self.name.startswith('localhost') \ and ipstr is not None and self.ip in name_ips: - self.connxinfo = "%s:%s (%s:%s)" % (socket.getfqdn(name=self.name), self.display, ipstr, self.display) + self.connxinfo = "%s:%s (%s:%s)" % \ + (socket.getfqdn(name=self.name), constants.X_DISPLAY_NUMBER, + ipstr, constants.X_DISPLAY_NUMBER) elif ipstr is not None: - self.connxinfo = "%s:%s" % (ipstr, self.display,) + self.connxinfo = "%s:%s" % (ipstr, constants.X_DISPLAY_NUMBER) else: self.connxinfo = None
@@ -157,7 +158,7 @@ class VncServer: else: hostarg = self.vncconnecthost
- vncconfigcommand = [self.root+"/usr/bin/vncconfig", "-display", ":%s"%self.display, "-connect", hostarg] + vncconfigcommand = [self.root+"/usr/bin/vncconfig", "-display", ":%s" % constants.X_DISPLAY_NUMBER, "-connect", hostarg]
for _i in range(maxTries): vncconfp = iutil.startProgram(vncconfigcommand, stdout=subprocess.PIPE, stderr=subprocess.PIPE) # vncconfig process @@ -184,7 +185,7 @@ class VncServer:
self.log.info(_("Attempting to start vncconfig"))
- vncconfigcommand = [self.root+"/usr/bin/vncconfig", "-nowin", "-display", ":%s" % self.display] + vncconfigcommand = [self.root+"/usr/bin/vncconfig", "-nowin", "-display", ":%s" % constants.X_DISPLAY_NUMBER]
# Use startProgram to run vncconfig in the background iutil.startProgram(vncconfigcommand, stdout=self.openlogfile(), stderr=subprocess.STDOUT) @@ -199,7 +200,7 @@ class VncServer: else: self.log.info(_("Please manually connect your vnc client to <IP ADDRESS>:%s " "to begin the install. Switch to the shell (Ctrl-B 2) and " - "run 'ip addr' to find the <IP ADDRESS>."), self.display) + "run 'ip addr' to find the <IP ADDRESS>."), constants.X_DISPLAY_NUMBER)
def startServer(self): self.log.info(_("Starting VNC...")) @@ -226,7 +227,7 @@ class VncServer: self.setVNCPassword()
# Lets start the xvnc. - xvnccommand = [ XVNC_BINARY_NAME, ":%s" % self.display, + xvnccommand = [ XVNC_BINARY_NAME, ":%s" % constants.X_DISPLAY_NUMBER, "-depth", "16", "-br", "IdleTimeout=0", "-auth", "/dev/null", "-once", "DisconnectClients=false", "desktop=%s" % (self.desktop,), @@ -266,8 +267,6 @@ class VncServer: else: self.VNCListen()
- os.environ["DISPLAY"]=":%s" % self.display - # Start vncconfig for copy/paste self.startVncConfig()
iutil.setenv will manipulate a dictionary that is added to a copy of os.environ that is passed to Popen. This way environment variables can be made available to child processes without modifying the environment of the anaconda process, which is not thread safe.
Use iutil.setenv in place of os.environment for a variety of environment variables not needed by the anaconda process. --- anaconda | 12 ++++++------ pyanaconda/iutil.py | 16 ++++++++++++++++ pyanaconda/rescue.py | 6 +++--- pyanaconda/ui/gui/__init__.py | 2 +- 4 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/anaconda b/anaconda index d2cb178..82af7e7 100755 --- a/anaconda +++ b/anaconda @@ -1146,14 +1146,14 @@ if __name__ == "__main__": log.info("Failed to parse proxy "%s": %s", anaconda.proxy, e) else: # Set environmental variables to be used by pre/post scripts - os.environ["PROXY"] = proxy.noauth_url - os.environ["PROXY_USER"] = proxy.username or "" - os.environ["PROXY_PASSWORD"] = proxy.password or "" + iutil.setenv("PROXY", proxy.noauth_url) + iutil.setenv("PROXY_USER", proxy.username or "") + iutil.setenv("PROXY_PASSWORD", proxy.password or "")
# Variables used by curl, libreport, etc. - os.environ["http_proxy"] = proxy.url - os.environ["ftp_proxy"] = proxy.url - os.environ["HTTPS_PROXY"] = proxy.url + iutil.setenv("http_proxy", proxy.url) + iutil.setenv("ftp_proxy", proxy.url) + iutil.setenv("HTTPS_PROXY", proxy.url)
if flags.noverifyssl: ksdata.method.noverifyssl = flags.noverifyssl diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index 87378f6..d82ef4f 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -50,11 +50,27 @@ program_log = logging.getLogger("program")
from pyanaconda.anaconda_log import program_log_lock
+_child_env = {} + +def setenv(name, value): + """ Set an environment variable to be used by child processes. + + This method does not modify os.environ for the running process, which + is not thread-safe. If setenv has already been called for a particular + variable name, the old value is overwritten. + + :param str name: The name of the environment variable + :param str value: The value of the environment variable + """ + + _child_env[name] = value + def augmentEnv(): env = os.environ.copy() env.update({"LC_ALL": "C", "ANA_INSTALL_PATH": getSysroot() }) + env.update(_child_env) return env
_root_path = "/mnt/sysimage" diff --git a/pyanaconda/rescue.py b/pyanaconda/rescue.py index 6702e2b..c808558 100644 --- a/pyanaconda/rescue.py +++ b/pyanaconda/rescue.py @@ -383,7 +383,7 @@ def doRescue(intf, rescue_mount, ksdata): # set a library path to use mounted fs libdirs = os.environ.get("LD_LIBRARY_PATH", "").split(":") mounted = map(lambda dir: "/mnt/sysimage%s" % dir, libdirs) - os.environ["LD_LIBRARY_PATH"] = ":".join(libdirs + mounted) + iutil.setenv("LD_LIBRARY_PATH", ":".join(libdirs + mounted))
# find groff data dir gversion = None @@ -401,8 +401,8 @@ def doRescue(intf, rescue_mount, ksdata):
if gversion is not None: gpath = "/mnt/sysimage/usr/share/groff/"+gversion - os.environ["GROFF_FONT_PATH"] = gpath + '/font' - os.environ["GROFF_TMAC_PATH"] = "%s:/mnt/sysimage/usr/share/groff/site-tmac" % (gpath + '/tmac',) + iutil.setenv("GROFF_FONT_PATH", gpath + '/font') + iutil.setenv("GROFF_TMAC_PATH", "%s:/mnt/sysimage/usr/share/groff/site-tmac" % (gpath + '/tmac',))
# do we have bash? try: diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py index b775a54..35d2c78 100644 --- a/pyanaconda/ui/gui/__init__.py +++ b/pyanaconda/ui/gui/__init__.py @@ -536,7 +536,7 @@ class GraphicalUserInterface(UserInterface): if monitor_height_px >= 1200 and monitor_dpi_x > 192 and monitor_dpi_y > 192: display.set_window_scale(2) # Export the scale so that Gtk programs launched by anaconda are also scaled - os.environ["GDK_SCALE"] = "2" + iutil.setenv("GDK_SCALE", 2)
def _convertSignals(self): # What tends to happen when we receive a signal is that the signal will
Use the parameter when starting metacity so that the global environment does not need to be modified. --- anaconda | 8 ++++---- pyanaconda/iutil.py | 8 ++++++-- 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/anaconda b/anaconda index 82af7e7..b62a909 100755 --- a/anaconda +++ b/anaconda @@ -138,12 +138,12 @@ def doStartupX11Actions(): # files. env_bak = copy(os.environ) if 'XDG_DATA_DIRS' in os.environ: - os.environ['XDG_DATA_DIRS'] = '/usr/share/anaconda/window-manager:' + \ - os.environ['XDG_DATA_DIRS'] + xdg_data_dirs = '/usr/share/anaconda/window-manager:' + os.environ['XDG_DATA_DIRS'] else: - os.environ['XDG_DATA_DIRS'] = '/usr/share/anaconda/window-manager:/usr/share' + xdg_data_dirs = '/usr/share/anaconda/window-manager:/usr/share'
- childproc = iutil.startProgram(["metacity", "--display", ":1", "--sm-disable"]) + childproc = iutil.startProgram(["metacity", "--display", ":1", "--sm-disable"], + env_add={'XDG_DATA_DIRS': xdg_data_dirs}) os.environ = env_bak iutil.watchProcess(childproc, "metacity")
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index d82ef4f..f15c558 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -118,7 +118,7 @@ def setSysroot(path): _sysroot = path
def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - env_prune=None, reset_handlers=True, **kwargs): + env_prune=None, env_add=None, reset_handlers=True, **kwargs): """ Start an external program and return the Popen object.
The root and reset_handlers arguments are handled by passing a @@ -131,7 +131,8 @@ def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subp :param stdin: The file object to read stdin from. :param stdout: The file object to write stdout to. :param stderr: The file object to write stderr to. - :param env_prune: environment variable to remove before execution + :param env_prune: environment variables to remove before execution + :param env_add: environment variables to add before execution :param reset_handlers: whether to reset to SIG_DFL any signal handlers set to SIG_IGN :param kwargs: Additional parameters to pass to subprocess.Popen :return: A Popen object for the running command. @@ -173,6 +174,9 @@ def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subp for var in env_prune: env.pop(var, None)
+ if env_add: + env.update(env_add) + return subprocess.Popen(argv, stdin=stdin, stdout=stdout,
Remove the language argument from AnacondaBaseWindow.retranslate. Setting the locale and language environment is handled by the caller, so retranslate should only refresh translations to the current locale.
Setting $LANG is needed by Python's gettext module, so we can't get rid of all of it. Handle the UEFI firmware language by doing it earlier, and handle GUI changes by setting the envrionment variable in a way that kinda maybe sort of looks thread-safe. --- anaconda | 9 ++++++--- pyanaconda/localization.py | 11 +++++++++-- pyanaconda/ui/gui/spokes/welcome.py | 36 ++++++++++++++++++++++++++++++++---- widgets/configure.ac | 2 +- widgets/src/BaseWindow.c | 6 +----- widgets/src/BaseWindow.h | 2 +- widgets/src/StandaloneWindow.c | 5 ++--- widgets/src/StandaloneWindow.h | 2 +- 8 files changed, 53 insertions(+), 20 deletions(-)
diff --git a/anaconda b/anaconda index b62a909..1700595 100755 --- a/anaconda +++ b/anaconda @@ -907,6 +907,7 @@ if __name__ == "__main__": isys.installSyncSignalHandlers()
setupEnvironment() + # make sure we have /var/log soon, some programs fail to start without it iutil.mkdirChain("/var/log")
@@ -1212,6 +1213,11 @@ if __name__ == "__main__": from pyanaconda import localization # Set the language before loading an interface, when it may be too late.
+ # first, try to load firmware language if nothing is already set in + # the environment + if "LANG" not in os.environ: + localization.load_firmware_language(ksdata.lang) + # check if the LANG environmental variable is set env_lang = os.environ.get("LANG") if env_lang is not None: @@ -1346,9 +1352,6 @@ if __name__ == "__main__": if not anaconda.ksdata.timezone.nontp: iutil.start_service("chronyd")
- # try to load firmware language - localization.load_firmware_language(ksdata.lang) - # FIXME: This will need to be made cleaner once this file starts to take # shape with the new UI code. anaconda._intf.setup(ksdata) diff --git a/pyanaconda/localization.py b/pyanaconda/localization.py index a7cc908..30112f9 100644 --- a/pyanaconda/localization.py +++ b/pyanaconda/localization.py @@ -29,7 +29,7 @@ import glob from collections import namedtuple
from pyanaconda import constants -from pyanaconda.iutil import upcase_first_letter +from pyanaconda.iutil import upcase_first_letter, setenv
import logging log = logging.getLogger("anaconda") @@ -187,6 +187,9 @@ def setup_locale(locale, lang=None): ksdata.lang object (if given). DOES NOT PERFORM ANY CHECKS OF THE GIVEN LOCALE.
+ $LANG must be set by the caller in order to set the language used by gettext. + Doing this in a thread-safe way is up to the caller. + :param locale: locale to setup :type locale: str :param lang: ksdata.lang object or None @@ -198,7 +201,7 @@ def setup_locale(locale, lang=None): if lang: lang.lang = locale
- os.environ["LANG"] = locale + setenv("LANG", locale) locale_mod.setlocale(locale_mod.LC_ALL, locale)
def get_english_name(locale): @@ -420,6 +423,8 @@ def load_firmware_language(lang): information in the given ksdata.lang object and sets the $LANG environment variable.
+ This method must be run before any other threads are started. + :param lang: ksdata.lang object :return: None :rtype: None @@ -471,6 +476,8 @@ def load_firmware_language(lang): log.debug("Using UEFI PlatformLang '%s' ('%s') as our language.", d, locales[0]) setup_locale(locales[0], lang)
+ os.environ["LANG"] = locales[0] # pylint: disable=environment-modify + _DateFieldSpec = namedtuple("DateFieldSpec", ["format", "suffix"])
def resolve_date_format(year, month, day, fail_safe=True): diff --git a/pyanaconda/ui/gui/spokes/welcome.py b/pyanaconda/ui/gui/spokes/welcome.py index bf4fbbd..1dbe527 100644 --- a/pyanaconda/ui/gui/spokes/welcome.py +++ b/pyanaconda/ui/gui/spokes/welcome.py @@ -23,10 +23,11 @@ import sys import re import langtable +import os
from pyanaconda.ui.gui.hubs.summary import SummaryHub from pyanaconda.ui.gui.spokes import StandaloneSpoke -from pyanaconda.ui.gui.utils import setup_gtk_direction, escape_markup +from pyanaconda.ui.gui.utils import setup_gtk_direction, escape_markup, gtk_action_wait from pyanaconda.ui.gui.xkl_wrapper import XklWrapper from pyanaconda.ui.gui.spokes.lib.lang_locale_handler import LangLocaleHandler
@@ -65,6 +66,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): (store, itr) = self._localeSelection.get_selected()
locale = store[itr][1] + self._set_lang(locale) localization.setup_locale(locale, self.data.lang)
# Skip timezone and keyboard default setting for kickstart installs. @@ -194,6 +196,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): # if there are no translations for the given locales, # use default if not langs_with_translations: + self._set_lang(DEFAULT_LANG) localization.setup_locale(DEFAULT_LANG, self.data.lang) lang_itr, _locale_itr = self._select_locale(self.data.lang.lang) langs_with_translations[DEFAULT_LANG] = lang_itr @@ -218,6 +221,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): store.set(newItr, 0, "", 1, "", 2, "", 3, True)
# setup the "best" locale + self._set_lang(locales[0]) localization.setup_locale(locales[0], self.data.lang) self._select_locale(self.data.lang.lang)
@@ -235,7 +239,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): else: widget.set_label(_(before))
- def retranslate(self, lang): + def retranslate(self): # Change the translations on labels and buttons that do not have # substitution text. for name in ["pickLanguageLabel", "betaWarnTitle", "betaWarnDesc"]: @@ -263,7 +267,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
# And of course, don't forget the underlying window. self.window.set_property("distribution", distributionText().upper()) - self.window.retranslate(lang) + self.window.retranslate()
def refresh(self): self._select_locale(self.data.lang.lang) @@ -297,8 +301,9 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
if selected: lang = store[selected[0]][1] + self._set_lang(lang) localization.setup_locale(lang) - self.retranslate(lang) + self.retranslate()
# Reset the text direction setup_gtk_direction() @@ -330,3 +335,26 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): sys.exit(0)
StandaloneSpoke._on_continue_clicked(self, window, user_data) + + @gtk_action_wait + def _set_lang(self, lang): + # This is *hopefully* safe. The only threads that might be running + # outside of the GIL are those doing file operations, the Gio dbus + # proxy thread, and calls from the Gtk main loop. The file operations + # won't be doing things that may access the environment, fingers + # crossed, the GDbus thread shouldn't be doing anything weird since all + # of our dbus calls are from python and synchronous. Using + # gtk_action_wait ensures that this is Gtk main loop thread, and it's + # holding the GIL. + # + # There is a lot of uncertainty and weasliness in those statements. + # This is not good code. + # + # We cannot get around setting $LANG. Python's gettext implementation + # differs from C in that consults only the environment for the current + # language and not the data set via setlocale. If we want translations + # from python modules to work, something needs to be set in the + # environment when the language changes. + + # pylint: disable=environment-modify + os.environ["LANG"] = lang diff --git a/widgets/configure.ac b/widgets/configure.ac index 6456db9..e86e6cf 100644 --- a/widgets/configure.ac +++ b/widgets/configure.ac @@ -66,7 +66,7 @@ AC_CHECK_HEADERS([libintl.h stdlib.h string.h unistd.h locale.h], [ANACONDA_SOFT_FAILURE([Header file $ac_header not found.])], [])
-AC_CHECK_FUNCS([setenv setlocale], +AC_CHECK_FUNCS([setlocale], [], [ANACONDA_SOFT_FAILURE([Function $ac_func not found.])])
diff --git a/widgets/src/BaseWindow.c b/widgets/src/BaseWindow.c index a9ce108..1074e56 100644 --- a/widgets/src/BaseWindow.c +++ b/widgets/src/BaseWindow.c @@ -681,7 +681,6 @@ void anaconda_base_window_clear_info(AnacondaBaseWindow *win) { /** * anaconda_base_window_retranslate: * @win: a #AnacondaBaseWindow - * @lang: target language * * Reload translations for this widget as needed. Generally, this is not * needed. However when changing the language during installation, we need @@ -690,12 +689,9 @@ void anaconda_base_window_clear_info(AnacondaBaseWindow *win) { * * Since: 1.0 */ -void anaconda_base_window_retranslate(AnacondaBaseWindow *win, const char *lang) { +void anaconda_base_window_retranslate(AnacondaBaseWindow *win) { GValue distro = G_VALUE_INIT;
- setenv("LANGUAGE", lang, 1); - setlocale(LC_ALL, ""); - /* This bit is internal gettext magic. */ { extern int _nl_msg_cat_cntr; diff --git a/widgets/src/BaseWindow.h b/widgets/src/BaseWindow.h index 0ffca12..0cc0876 100644 --- a/widgets/src/BaseWindow.h +++ b/widgets/src/BaseWindow.h @@ -69,7 +69,7 @@ struct _AnacondaBaseWindowClass { GType anaconda_base_window_get_type (void); GtkWidget *anaconda_base_window_new ();
-void anaconda_base_window_retranslate (AnacondaBaseWindow *win, const char *lang); +void anaconda_base_window_retranslate (AnacondaBaseWindow *win);
gboolean anaconda_base_window_get_beta (AnacondaBaseWindow *win); void anaconda_base_window_set_beta (AnacondaBaseWindow *win, gboolean is_beta); diff --git a/widgets/src/StandaloneWindow.c b/widgets/src/StandaloneWindow.c index f0477c0..c952a1f 100644 --- a/widgets/src/StandaloneWindow.c +++ b/widgets/src/StandaloneWindow.c @@ -175,7 +175,6 @@ static void anaconda_standalone_window_init(AnacondaStandaloneWindow *win) { /** * anaconda_standalone_window_retranslate: * @win: a #AnacondaStandaloneWindow - * @lang: target language * * Reload translations for this widget as needed. Generally, this is not * needed. However when changing the language during installation, we need @@ -184,8 +183,8 @@ static void anaconda_standalone_window_init(AnacondaStandaloneWindow *win) { * * Since: 1.0 */ -void anaconda_standalone_window_retranslate(AnacondaStandaloneWindow *win, const char *lang) { - anaconda_base_window_retranslate(ANACONDA_BASE_WINDOW(win), lang); +void anaconda_standalone_window_retranslate(AnacondaStandaloneWindow *win) { + anaconda_base_window_retranslate(ANACONDA_BASE_WINDOW(win));
gtk_button_set_label(GTK_BUTTON(win->priv->quit_button), _(QUIT_TEXT)); gtk_button_set_label(GTK_BUTTON(win->priv->continue_button), _(CONTINUE_TEXT)); diff --git a/widgets/src/StandaloneWindow.h b/widgets/src/StandaloneWindow.h index 63a9e9e..85671f2 100644 --- a/widgets/src/StandaloneWindow.h +++ b/widgets/src/StandaloneWindow.h @@ -63,7 +63,7 @@ struct _AnacondaStandaloneWindowClass {
GType anaconda_standalone_window_get_type (void); GtkWidget *anaconda_standalone_window_new (); -void anaconda_standalone_window_retranslate (AnacondaStandaloneWindow *win, const char *lang); +void anaconda_standalone_window_retranslate (AnacondaStandaloneWindow *win);
G_END_DECLS
This ensures that the behavior of the timestamp test doesn't change on January 1st and doesn't depend on the time zone. --- pyanaconda/packaging/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py index 91343db..21c6da1 100644 --- a/pyanaconda/packaging/__init__.py +++ b/pyanaconda/packaging/__init__.py @@ -395,7 +395,9 @@ class Payload(object): except ConfigParser.Error: pass
- if version.startswith(time.strftime("%Y")): + # If the version looks like a timestamp, assume it's rawhide + # plz change before Fedora version 9999999 + if version[:8].isdigit(): version = "rawhide"
log.debug("got a release version of %s", version)
For datetime.now(), use explicit tzinfo objects to set the time zone. Avoid uses of mktime by calculating datetime.timedelta objects from a given datetime to timestamp 0. Instead of logging a time with ctime, print out the UTC version of a timestamp and slap "UTC" on the end. --- pyanaconda/isys/__init__.py | 17 +++++++++++------ pyanaconda/timezone.py | 9 +++++++++ pyanaconda/ui/gui/spokes/datetime_spoke.py | 10 +++++----- 3 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/pyanaconda/isys/__init__.py b/pyanaconda/isys/__init__.py index b099355..02e7d85 100644 --- a/pyanaconda/isys/__init__.py +++ b/pyanaconda/isys/__init__.py @@ -32,6 +32,7 @@ except ImportError: import blivet.arch import time import datetime +import pytz
import logging log = logging.getLogger("anaconda") @@ -108,10 +109,10 @@ def set_system_time(secs): """
_isys.set_system_time(secs) - log.info("System time set to %s", time.ctime(secs)) + log.info("System time set to %s UTC", time.asctime(time.gmtime(secs)))
def set_system_date_time(year=None, month=None, day=None, hour=None, minute=None, - second=None): + second=None, tz=None): """ Set system date and time given by the parameters as numbers. If some parameter is missing or None, the current system date/time field is used @@ -122,7 +123,7 @@ def set_system_date_time(year=None, month=None, day=None, hour=None, minute=None """
# get the right values - now = datetime.datetime.now() + now = datetime.datetime.now(tz) year = year if year is not None else now.year month = month if month is not None else now.month day = day if day is not None else now.day @@ -130,9 +131,13 @@ def set_system_date_time(year=None, month=None, day=None, hour=None, minute=None minute = minute if minute is not None else now.minute second = second if second is not None else now.second
- # struct fields -> year, month, day, hour, minute, second, week_day, year_day, dst - time_struct = time.struct_time((year, month, day, hour, minute, second, 0, 0, -1)) - set_system_time(int(time.mktime(time_struct))) + set_date = datetime.datetime(year, month, day, hour, minute, second, tzinfo=tz) + + # Calculate the number of seconds between this time and timestamp 0 + epoch = datetime.datetime.fromtimestamp(0, pytz.UTC) + timestamp = (set_date - epoch).total_seconds() + + set_system_time(timestamp)
def total_memory(): """Returns total system memory in kB (given to us by /proc/meminfo)""" diff --git a/pyanaconda/timezone.py b/pyanaconda/timezone.py index 9cc2c53..956c33e 100644 --- a/pyanaconda/timezone.py +++ b/pyanaconda/timezone.py @@ -215,3 +215,12 @@ def is_valid_timezone(timezone):
return timezone in pytz.common_timezones + etc_zones
+def get_timezone(timezone): + """ + Return a tzinfo object for a given timezone name. + + :param str timezone: the timezone name + :rtype: datetime.tzinfo + """ + + return pytz.timezone(timezone) diff --git a/pyanaconda/ui/gui/spokes/datetime_spoke.py b/pyanaconda/ui/gui/spokes/datetime_spoke.py index 1648c35..65f3130 100644 --- a/pyanaconda/ui/gui/spokes/datetime_spoke.py +++ b/pyanaconda/ui/gui/spokes/datetime_spoke.py @@ -34,7 +34,7 @@ from pyanaconda.ui.gui.helpers import GUIDialogInputCheckHandler from pyanaconda.ui.helpers import InputCheck
from pyanaconda.i18n import _, CN_ -from pyanaconda.timezone import NTP_SERVICE, get_all_regions_and_timezones, is_valid_timezone +from pyanaconda.timezone import NTP_SERVICE, get_all_regions_and_timezones, get_timezone, is_valid_timezone from pyanaconda.localization import get_xlated_timezone, resolve_date_format from pyanaconda import iutil from pyanaconda import isys @@ -46,7 +46,6 @@ from pyanaconda import constants from pyanaconda.threads import threadMgr, AnacondaThread
import datetime -import os import re import threading import locale as locale_mod @@ -405,6 +404,7 @@ class DatetimeSpoke(FirstbootSpokeMixIn, NormalSpoke): self._update_datetime_timer_id = None self._start_updating_timer_id = None self._shown = False + self._tz = None
def initialize(self): NormalSpoke.initialize(self) @@ -718,7 +718,7 @@ class DatetimeSpoke(FirstbootSpokeMixIn, NormalSpoke): return (hours + correction) % 24
def _update_datetime(self): - now = datetime.datetime.now() + now = datetime.datetime.now(self._tz) if self._radioButton24h.get_active(): self._hoursLabel.set_text("%0.2d" % now.hour) else: @@ -766,7 +766,7 @@ class DatetimeSpoke(FirstbootSpokeMixIn, NormalSpoke): day = self._get_combo_selection(self._dayCombo)[0] #day may be None if there is no such in the selected year and month if day: - isys.set_system_date_time(year, month, day, hours, minutes) + isys.set_system_date_time(year, month, day, hours, minutes, tz=self._tz)
#start the timer only when the spoke is shown if self._shown and not self._update_datetime_timer_id: @@ -1022,7 +1022,7 @@ class DatetimeSpoke(FirstbootSpokeMixIn, NormalSpoke): timezone = location.get_property('zone') if self._set_timezone(timezone): # timezone successfully set - os.environ["TZ"] = timezone + self._tz = get_timezone(timezone) self._update_datetime()
def on_timeformat_changed(self, button24h, *args):
The contents of the config file may change after the payload is setup and packages are installed, but the environment variable needs to be set early. For that reason, treat any failure to access an existing libuser.conf as an error in createLuserConf. --- anaconda | 6 ++++++ pyanaconda/kickstart.py | 11 +++++------ pyanaconda/users.py | 25 +++++++++++-------------- 3 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/anaconda b/anaconda index 1700595..9e614bd 100755 --- a/anaconda +++ b/anaconda @@ -385,6 +385,8 @@ def setupPythonPath(): sys.path.extend(ADDON_PATHS)
def setupEnvironment(): + from pyanaconda.users import createLuserConf + # This method is run before any threads are started, so this is the one # point where it's ok to modify the environment. # pylint: disable=environment-modify @@ -413,6 +415,10 @@ def setupEnvironment(): else: os.environ["DISPLAY"] = ":%s" % constants.X_DISPLAY_NUMBER
+ # Call createLuserConf now to setup $LIBUSER_CONF + # the config file can change later but the environment variable cannot + createLuserConf(iutil.getSysroot()) + def setupLoggingFromOpts(options): if (options.debug or options.updateSrc) and not options.loglevel: # debugging means debug logging if an explicit level hasn't been st diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index 5a31299..75fe829 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -94,6 +94,9 @@ class AnacondaKSScript(KSScript): else: scriptRoot = "/"
+ # Environment variables that cause problems for %post scripts + env_prune = ["LIBUSER_CONF"] + (fd, path) = tempfile.mkstemp("", "ks-script-", scriptRoot + "/tmp")
iutil.eintr_retry_call(os.write, fd, self.script) @@ -120,7 +123,8 @@ class AnacondaKSScript(KSScript): with open(messages, "w") as fp: rc = iutil.execWithRedirect(self.interp, ["/tmp/%s" % os.path.basename(path)], stdout=fp, - root = scriptRoot) + root = scriptRoot, + env_prune = env_prune)
if rc != 0: log.error("Error code %s running the kickstart script at line %s", rc, self.lineno) @@ -1977,11 +1981,6 @@ def runPostScripts(scripts): if len(postScripts) == 0: return
- # Remove environment variables that cause problems for %post scripts. - for var in ["LIBUSER_CONF"]: - if var in os.environ: - del(os.environ[var]) - log.info("Running kickstart %%post script(s)") map (lambda s: s.run(iutil.getSysroot()), postScripts) log.info("All kickstart %%post script(s) have been run") diff --git a/pyanaconda/users.py b/pyanaconda/users.py index cedf2db..465e38a 100644 --- a/pyanaconda/users.py +++ b/pyanaconda/users.py @@ -42,21 +42,18 @@ def createLuserConf(instPath, algoname='sha512'): This must be called before User() is instantiated the first time so that libuser.admin will use the temporary config file. """ - createTmp = False - try: - fn = os.environ["LIBUSER_CONF"] - if os.access(fn, os.F_OK): - log.info("removing libuser.conf at %s", os.getenv("LIBUSER_CONF")) - os.unlink(fn) - log.info("created new libuser.conf at %s with instPath="%s"", fn, instPath) - fd = open(fn, 'w') - except (OSError, IOError, KeyError): - createTmp = True
- if createTmp: + # If LIBUSER_CONF is not set, create a new temporary file + if "LIBUSER_CONF" not in os.environ: (fp, fn) = tempfile.mkstemp(prefix="libuser.") log.info("created new libuser.conf at %s with instPath="%s"", fn, instPath) - fd = os.fdopen(fp, 'w') + fd = os.fdopen(fp, "w") + # This is only ok if createLuserConf is first called before threads are started + os.environ["LIBUSER_CONF"] = fn # pylint: disable=environment-modify + else: + fn = os.environ["LIBUSER_CONF"] + log.info("Clearing libuser.conf at %s", fn) + fd = open(fn, "w")
buf = """ [defaults] @@ -81,7 +78,6 @@ login_defs = %(instPath)s/etc/login.defs
fd.write(buf) fd.close() - os.environ["LIBUSER_CONF"] = fn
return fn
@@ -210,7 +206,8 @@ class Users: if not root in ["","/"]: os.chroot(root) os.chdir("/") - del(os.environ["LIBUSER_CONF"]) + # This is ok because it's after a fork + del(os.environ["LIBUSER_CONF"]) # pylint: disable=environment-modify
self.admin = libuser.admin()
For the interactive programs started by anaconda, keep the locale set by the user. --- pyanaconda/ihelp.py | 2 +- pyanaconda/iutil.py | 12 +++++++----- pyanaconda/ui/gui/spokes/network.py | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/pyanaconda/ihelp.py b/pyanaconda/ihelp.py index 5326048..470f300 100644 --- a/pyanaconda/ihelp.py +++ b/pyanaconda/ihelp.py @@ -124,7 +124,7 @@ def start_yelp(help_path): # under some extreme circumstances (placeholders missing) # the help path can be None and we need to prevent Popen # receiving None as an argument instead of a string - yelp_process = startProgram(["yelp", help_path or ""]) + yelp_process = startProgram(["yelp", help_path or ""], reset_lang=False)
def kill_yelp(): """Try to kill any existing yelp processes""" diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index f15c558..b163091 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -67,9 +67,7 @@ def setenv(name, value):
def augmentEnv(): env = os.environ.copy() - env.update({"LC_ALL": "C", - "ANA_INSTALL_PATH": getSysroot() - }) + env.update({"ANA_INSTALL_PATH": getSysroot()}) env.update(_child_env) return env
@@ -118,7 +116,7 @@ def setSysroot(path): _sysroot = path
def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - env_prune=None, env_add=None, reset_handlers=True, **kwargs): + env_prune=None, env_add=None, reset_handlers=True, reset_lang=True, **kwargs): """ Start an external program and return the Popen object.
The root and reset_handlers arguments are handled by passing a @@ -134,6 +132,7 @@ def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subp :param env_prune: environment variables to remove before execution :param env_add: environment variables to add before execution :param reset_handlers: whether to reset to SIG_DFL any signal handlers set to SIG_IGN + :param reset_lang: whether to set the locale of the child process to C :param kwargs: Additional parameters to pass to subprocess.Popen :return: A Popen object for the running command. """ @@ -174,6 +173,9 @@ def startProgram(argv, root='/', stdin=None, stdout=subprocess.PIPE, stderr=subp for var in env_prune: env.pop(var, None)
+ if reset_lang: + env.update({"LC_ALL": "C"}) + if env_add: env.update(env_add)
@@ -417,7 +419,7 @@ def execReadlines(command, argv, stdin=None, root='/', env_prune=None): ## Run a shell. def execConsole(): try: - proc = startProgram(["/bin/sh"], stdout=None, stderr=None) + proc = startProgram(["/bin/sh"], stdout=None, stderr=None, reset_lang=False) proc.wait() except OSError as e: raise RuntimeError("Error running /bin/sh: " + e.strerror) diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py index 5718768..052871b 100644 --- a/pyanaconda/ui/gui/spokes/network.py +++ b/pyanaconda/ui/gui/spokes/network.py @@ -560,7 +560,7 @@ class NetworkControlBox(GObject.GObject): log.info("network: configuring connection %s device %s ssid %s", uuid, devname, self.selected_ssid) self.kill_nmce(msg="Configure button clicked") - proc = startProgram(["nm-connection-editor", "--edit", "%s" % uuid]) + proc = startProgram(["nm-connection-editor", "--edit", "%s" % uuid], reset_lang=False) self._running_nmce = proc
GLib.child_watch_add(proc.pid, self.on_nmce_exited, activate) @@ -650,7 +650,7 @@ class NetworkControlBox(GObject.GObject): def add_device(self, ty): log.info("network: adding device of type %s", ty) self.kill_nmce(msg="Add device button clicked") - proc = startProgram(["nm-connection-editor", "--create", "--type=%s" % ty]) + proc = startProgram(["nm-connection-editor", "--create", "--type=%s" % ty], reset_lang=False) self._running_nmce = proc
GLib.child_watch_add(proc.pid, self.on_nmce_exited)
Since things seem to be going horribly with both incoming and going email for me I've copied these patches into a pull request. https://github.com/rhinstaller/anaconda/pull/1 The email gateway isn't set up yet so I guess if you comment there try to comment here too? I don't know, I should get back to figuring that out.
On Thu, 2015-02-05 at 15:20 +0100, David Shea wrote:
Short version: environment variables cannot be set in a multi-threaded environment. It is ridiculous, and it has been a problem since the environment was invented, and no one has a solution for it, and computers are the absolute worst.
There is one environment variable that was cannot get around having to set. Python's gettext module, to determine the language to return, searches $LANGUAGE, $LC_ALL, $LC_MESSAGES, and $LANG. This is completely different from gettext() and friends in glibc, which, if there is nothing in $LANGUAGE, uses the locale data set by setlocale, which is thread-safe. This means that if we want translations to work in any python module that we do not control, we need to modify the environment.
So, possible solutions:
- write thread-safe versions of get/set/put/unsetenv, shoehorn it in with $LD_PRELOAD
- don't set any environment variables at all after starting threads, monkey-patch the gettext module to do the right thing
- don't set most environment variables, cross fingers and burn some sage before setting environment variables
These patches implement #3.
The last one is just a bonus. Did you know that nm-connection-editor wasn't run in the language the user selected, but it totally could be? Now it is.
All this needs to be ported to RHEL 7.2 with Related: rhbz#1188287
David Shea (9): Add a pylint module to look for modifications to the environment Set $DISPLAY before threads are started. Add a method to set environment variables for child processes
Cannot see this commit/patch. Is it only me or nobody has it?
On Tue, 2015-02-10 at 09:52 +0100, Vratislav Podzimek wrote:
On Thu, 2015-02-05 at 15:20 +0100, David Shea wrote:
Short version: environment variables cannot be set in a multi-threaded environment. It is ridiculous, and it has been a problem since the environment was invented, and no one has a solution for it, and computers are the absolute worst.
There is one environment variable that was cannot get around having to set. Python's gettext module, to determine the language to return, searches $LANGUAGE, $LC_ALL, $LC_MESSAGES, and $LANG. This is completely different from gettext() and friends in glibc, which, if there is nothing in $LANGUAGE, uses the locale data set by setlocale, which is thread-safe. This means that if we want translations to work in any python module that we do not control, we need to modify the environment.
So, possible solutions:
- write thread-safe versions of get/set/put/unsetenv, shoehorn it in with $LD_PRELOAD
- don't set any environment variables at all after starting threads, monkey-patch the gettext module to do the right thing
- don't set most environment variables, cross fingers and burn some sage before setting environment variables
These patches implement #3.
The last one is just a bonus. Did you know that nm-connection-editor wasn't run in the language the user selected, but it totally could be? Now it is.
All this needs to be ported to RHEL 7.2 with Related: rhbz#1188287
David Shea (9): Add a pylint module to look for modifications to the environment Set $DISPLAY before threads are started. Add a method to set environment variables for child processes
Cannot see this commit/patch. Is it only me or nobody has it?
Reviewed PATCH 3/9 on GH and these all look good to me. They are ugly, but I don't think we can do anything nicer in this case.
On 02/10/2015 09:52 AM, Vratislav Podzimek wrote:
On Thu, 2015-02-05 at 15:20 +0100, David Shea wrote:
Short version: environment variables cannot be set in a multi-threaded environment. It is ridiculous, and it has been a problem since the environment was invented, and no one has a solution for it, and computers are the absolute worst.
There is one environment variable that was cannot get around having to set. Python's gettext module, to determine the language to return, searches $LANGUAGE, $LC_ALL, $LC_MESSAGES, and $LANG. This is completely different from gettext() and friends in glibc, which, if there is nothing in $LANGUAGE, uses the locale data set by setlocale, which is thread-safe. This means that if we want translations to work in any python module that we do not control, we need to modify the environment.
So, possible solutions:
- write thread-safe versions of get/set/put/unsetenv, shoehorn it in with $LD_PRELOAD
- don't set any environment variables at all after starting threads, monkey-patch the gettext module to do the right thing
- don't set most environment variables, cross fingers and burn some sage before setting environment variables
These patches implement #3.
The last one is just a bonus. Did you know that nm-connection-editor wasn't run in the language the user selected, but it totally could be? Now it is.
All this needs to be ported to RHEL 7.2 with Related: rhbz#1188287
David Shea (9): Add a pylint module to look for modifications to the environment Set $DISPLAY before threads are started. Add a method to set environment variables for child processes
Cannot see this commit/patch. Is it only me or nobody has it?
I'll try resending it. It looks like the rest made it to the list, but I got a failure message about timeouts connecting to lists.fedorahosted.org for #3 for some reason.
iutil.setenv will manipulate a dictionary that is added to a copy of os.environ that is passed to Popen. This way environment variables can be made available to child processes without modifying the environment of the anaconda process, which is not thread safe.
Use iutil.setenv in place of os.environment for a variety of environment variables not needed by the anaconda process. --- anaconda | 12 ++++++------ pyanaconda/iutil.py | 16 ++++++++++++++++ pyanaconda/rescue.py | 6 +++--- pyanaconda/ui/gui/__init__.py | 2 +- 4 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/anaconda b/anaconda index d2cb178..82af7e7 100755 --- a/anaconda +++ b/anaconda @@ -1146,14 +1146,14 @@ if __name__ == "__main__": log.info("Failed to parse proxy "%s": %s", anaconda.proxy, e) else: # Set environmental variables to be used by pre/post scripts - os.environ["PROXY"] = proxy.noauth_url - os.environ["PROXY_USER"] = proxy.username or "" - os.environ["PROXY_PASSWORD"] = proxy.password or "" + iutil.setenv("PROXY", proxy.noauth_url) + iutil.setenv("PROXY_USER", proxy.username or "") + iutil.setenv("PROXY_PASSWORD", proxy.password or "")
# Variables used by curl, libreport, etc. - os.environ["http_proxy"] = proxy.url - os.environ["ftp_proxy"] = proxy.url - os.environ["HTTPS_PROXY"] = proxy.url + iutil.setenv("http_proxy", proxy.url) + iutil.setenv("ftp_proxy", proxy.url) + iutil.setenv("HTTPS_PROXY", proxy.url)
if flags.noverifyssl: ksdata.method.noverifyssl = flags.noverifyssl diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index 87378f6..d82ef4f 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -50,11 +50,27 @@ program_log = logging.getLogger("program")
from pyanaconda.anaconda_log import program_log_lock
+_child_env = {} + +def setenv(name, value): + """ Set an environment variable to be used by child processes. + + This method does not modify os.environ for the running process, which + is not thread-safe. If setenv has already been called for a particular + variable name, the old value is overwritten. + + :param str name: The name of the environment variable + :param str value: The value of the environment variable + """ + + _child_env[name] = value + def augmentEnv(): env = os.environ.copy() env.update({"LC_ALL": "C", "ANA_INSTALL_PATH": getSysroot() }) + env.update(_child_env) return env
_root_path = "/mnt/sysimage" diff --git a/pyanaconda/rescue.py b/pyanaconda/rescue.py index 6702e2b..c808558 100644 --- a/pyanaconda/rescue.py +++ b/pyanaconda/rescue.py @@ -383,7 +383,7 @@ def doRescue(intf, rescue_mount, ksdata): # set a library path to use mounted fs libdirs = os.environ.get("LD_LIBRARY_PATH", "").split(":") mounted = map(lambda dir: "/mnt/sysimage%s" % dir, libdirs) - os.environ["LD_LIBRARY_PATH"] = ":".join(libdirs + mounted) + iutil.setenv("LD_LIBRARY_PATH", ":".join(libdirs + mounted))
# find groff data dir gversion = None @@ -401,8 +401,8 @@ def doRescue(intf, rescue_mount, ksdata):
if gversion is not None: gpath = "/mnt/sysimage/usr/share/groff/"+gversion - os.environ["GROFF_FONT_PATH"] = gpath + '/font' - os.environ["GROFF_TMAC_PATH"] = "%s:/mnt/sysimage/usr/share/groff/site-tmac" % (gpath + '/tmac',) + iutil.setenv("GROFF_FONT_PATH", gpath + '/font') + iutil.setenv("GROFF_TMAC_PATH", "%s:/mnt/sysimage/usr/share/groff/site-tmac" % (gpath + '/tmac',))
# do we have bash? try: diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py index b775a54..35d2c78 100644 --- a/pyanaconda/ui/gui/__init__.py +++ b/pyanaconda/ui/gui/__init__.py @@ -536,7 +536,7 @@ class GraphicalUserInterface(UserInterface): if monitor_height_px >= 1200 and monitor_dpi_x > 192 and monitor_dpi_y > 192: display.set_window_scale(2) # Export the scale so that Gtk programs launched by anaconda are also scaled - os.environ["GDK_SCALE"] = "2" + iutil.setenv("GDK_SCALE", 2)
def _convertSignals(self): # What tends to happen when we receive a signal is that the signal will
On Thu, Feb 05, 2015 at 03:20:46PM +0100, David Shea wrote:
Short version: environment variables cannot be set in a multi-threaded environment. It is ridiculous, and it has been a problem since the environment was invented, and no one has a solution for it, and computers are the absolute worst.
These all look ok to me. I continue to be surprised things ever work...
anaconda-patches@lists.fedorahosted.org