From 2c791e4d7f4ab304fda9a03eabc3916056b0e36f Mon Sep 17 00:00:00 2001 From: William Deegan Date: Wed, 2 Nov 2022 23:21:57 -0400 Subject: Move execution environment sanitation from Action._suproc to SCons.Util.sanitize_shell_env(ENV). Have ninja's spawning logic use this same function to clean it's execution environment --- CHANGES.txt | 5 +++++ RELEASE.txt | 3 +++ SCons/Action.py | 19 +------------------ SCons/Platform/darwin.py | 1 + SCons/Tool/ninja/Utils.py | 16 ++-------------- SCons/Tool/ninja/__init__.py | 8 +++++++- SCons/Util.py | 27 +++++++++++++++++++++++++++ test/ninja/shell_command.py | 6 ++++++ 8 files changed, 52 insertions(+), 33 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0a9f697..146a861 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -22,6 +22,11 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER NOTE: If you hook into SCons.Jobs, you'll have to change that to use SCons.Taskmaster.Jobs - Changed the Taskmaster trace logic to use python's logging module. The output formatting should remain (mostly) the same. Minor update to unittest for this to adjust for 1 less newline. + - Ninja: Fix execution environment sanitation for launching ninja. Previously if you set an + execution environment variable set to a python list it would crash. Now it + will create a string joining the list with os.pathsep + - Move execution environment sanitation from Action._subproc() to + SCons.Util.sanitize_shell_env(ENV) diff --git a/RELEASE.txt b/RELEASE.txt index 94ba72e..b93c449 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -50,6 +50,9 @@ FIXES use: env["JAVACLASSPATH"] = env.Split("foo bar baz") There is no change in how JAVACLASSPATH gets turned into the -classpath argument passed to the JDK tools. +- Ninja: Fix execution environment sanitation for launching ninja. Previously if you set an + execution environment variable set to a python list it would crash. Now it + will create a string joining the list with os.pathsep IMPROVEMENTS ------------ diff --git a/SCons/Action.py b/SCons/Action.py index ccb3a2c..8151b2a 100644 --- a/SCons/Action.py +++ b/SCons/Action.py @@ -806,24 +806,7 @@ def _subproc(scons_env, cmd, error='ignore', **kw): ENV = kw.get('env', None) if ENV is None: ENV = get_default_ENV(scons_env) - # Ensure that the ENV values are all strings: - new_env = {} - for key, value in ENV.items(): - if is_List(value): - # If the value is a list, then we assume it is a path list, - # because that's a pretty common list-like value to stick - # in an environment variable: - value = SCons.Util.flatten_sequence(value) - new_env[key] = os.pathsep.join(map(str, value)) - else: - # It's either a string or something else. If it's a string, - # we still want to call str() because it might be a *Unicode* - # string, which makes subprocess.Popen() gag. If it isn't a - # string or a list, then we just coerce it to a string, which - # is the proper way to handle Dir and File instances and will - # produce something reasonable for just about everything else: - new_env[key] = str(value) - kw['env'] = new_env + kw['env'] = SCons.Util.sanitize_shell_env(ENV) try: pobj = subprocess.Popen(cmd, **kw) diff --git a/SCons/Platform/darwin.py b/SCons/Platform/darwin.py index f997a7d..d57f810 100644 --- a/SCons/Platform/darwin.py +++ b/SCons/Platform/darwin.py @@ -40,6 +40,7 @@ def generate(env): # env['ENV']['PATH'] = '/opt/local/bin:/opt/local/sbin:' + env['ENV']['PATH'] + ':/sw/bin' # Store extra system paths in env['ENV']['PATHOSX'] + env['ENV']['PATHOSX'] = '/opt/local/bin' filelist = ['/etc/paths',] # make sure this works on Macs with Tiger or earlier diff --git a/SCons/Tool/ninja/Utils.py b/SCons/Tool/ninja/Utils.py index 583301e..7269fb2 100644 --- a/SCons/Tool/ninja/Utils.py +++ b/SCons/Tool/ninja/Utils.py @@ -285,6 +285,7 @@ def ninja_sorted_build(ninja, **build): sorted_dict = ninja_recursive_sorted_dict(build) ninja.build(**sorted_dict) + def get_command_env(env, target, source): """ Return a string that sets the environment for any environment variables that @@ -311,21 +312,8 @@ def get_command_env(env, target, source): windows = env["PLATFORM"] == "win32" command_env = "" + scons_specified_env = SCons.Util.sanitize_shell_env(scons_specified_env) for key, value in scons_specified_env.items(): - # Ensure that the ENV values are all strings: - if is_List(value): - # If the value is a list, then we assume it is a - # path list, because that's a pretty common list-like - # value to stick in an environment variable: - value = flatten_sequence(value) - value = joinpath(map(str, value)) - else: - # If it isn't a string or a list, then we just coerce - # it to a string, which is the proper way to handle - # Dir and File instances and will produce something - # reasonable for just about everything else: - value = str(value) - if windows: command_env += "set '{}={}' && ".format(key, value) else: diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py index c4023c3..2622641 100644 --- a/SCons/Tool/ninja/__init__.py +++ b/SCons/Tool/ninja/__init__.py @@ -34,6 +34,7 @@ import SCons import SCons.Script import SCons.Tool.ninja.Globals from SCons.Script import GetOption +from SCons.Util import sanitize_shell_env from .Globals import NINJA_RULES, NINJA_POOLS, NINJA_CUSTOM_HANDLERS, NINJA_DEFAULT_TARGETS, NINJA_CMDLINE_TARGETS from .Methods import register_custom_handler, register_custom_rule_mapping, register_custom_rule, register_custom_pool, \ @@ -100,11 +101,16 @@ def ninja_builder(env, target, source): # reproduce the output like a ninja build would def execute_ninja(): + if env['PLATFORM'] == 'win32': + spawn_env = os.environ + else: + spawn_env = sanitize_shell_env(env['ENV']) + proc = subprocess.Popen(cmd, stderr=sys.stderr, stdout=subprocess.PIPE, universal_newlines=True, - env=os.environ if env["PLATFORM"] == "win32" else env['ENV'] + env=spawn_env ) for stdout_line in iter(proc.stdout.readline, ""): yield stdout_line diff --git a/SCons/Util.py b/SCons/Util.py index 7eb2005..c914af0 100644 --- a/SCons/Util.py +++ b/SCons/Util.py @@ -2138,6 +2138,33 @@ class DispatchingFormatter(Formatter): formatter = self._formatters.get(record.name, self._default_formatter) return formatter.format(record) + +def sanitize_shell_env(env): + """ + Sanitize all values from env['ENV'] which will be propagated to the shell + :param env: The shell environment variables to be propagated to spawned shell + :return: sanitize dictionary of env variables (similar to what you'd get from os.environ) + """ + + # Ensure that the ENV values are all strings: + new_env = {} + for key, value in env.items(): + if is_List(value): + # If the value is a list, then we assume it is a path list, + # because that's a pretty common list-like value to stick + # in an environment variable: + value = flatten_sequence(value) + new_env[key] = os.pathsep.join(map(str, value)) + else: + # It's either a string or something else. If it's a string, + # we still want to call str() because it might be a *Unicode* + # string, which makes subprocess.Popen() gag. If it isn't a + # string or a list, then we just coerce it to a string, which + # is the proper way to handle Dir and File instances and will + # produce something reasonable for just about everything else: + new_env[key] = str(value) + return new_env + # Local Variables: # tab-width:4 # indent-tabs-mode:nil diff --git a/test/ninja/shell_command.py b/test/ninja/shell_command.py index a6926c7..a6c48c4 100644 --- a/test/ninja/shell_command.py +++ b/test/ninja/shell_command.py @@ -53,6 +53,12 @@ SetOption('experimental','ninja') DefaultEnvironment(tools=[]) env = Environment() + +# Added to verify that SCons Ninja tool is sanitizing the shell environment +# before it spawns a new shell +env['ENV']['ZPATH']=['/a/b/c','/c/d/e'] + + env.Tool('ninja') prog = env.Program(target = 'foo', source = 'foo.c') env.Command('foo.out', prog, '%(shell)sfoo%(_exe)s > foo.out') -- cgit v0.12 From ef9879e9d8de55631e07a7e05510ca33aaf61185 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Thu, 3 Nov 2022 10:35:17 -0400 Subject: Changed argument name for sanitize_shell_env from env -> execution_env per feedback from mwichmann --- SCons/Util.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/SCons/Util.py b/SCons/Util.py index c914af0..a7a6307 100644 --- a/SCons/Util.py +++ b/SCons/Util.py @@ -2139,16 +2139,16 @@ class DispatchingFormatter(Formatter): return formatter.format(record) -def sanitize_shell_env(env): +def sanitize_shell_env(execution_env): """ - Sanitize all values from env['ENV'] which will be propagated to the shell - :param env: The shell environment variables to be propagated to spawned shell + Sanitize all values in execution_env (typically this is env['ENV']) which will be propagated to the shell + :param execution_env: The shell environment variables to be propagated to spawned shell :return: sanitize dictionary of env variables (similar to what you'd get from os.environ) """ # Ensure that the ENV values are all strings: new_env = {} - for key, value in env.items(): + for key, value in execution_env.items(): if is_List(value): # If the value is a list, then we assume it is a path list, # because that's a pretty common list-like value to stick -- cgit v0.12 From f3dc3495cfa86524f39fb1b86115febbe40383e5 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Sun, 13 Nov 2022 08:55:43 -0500 Subject: Remove macports path, update comment to remove python2 related issues --- SCons/Platform/darwin.py | 4 ++-- SCons/Util.py | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/SCons/Platform/darwin.py b/SCons/Platform/darwin.py index d57f810..dcaf5c8 100644 --- a/SCons/Platform/darwin.py +++ b/SCons/Platform/darwin.py @@ -31,6 +31,7 @@ selection method. from . import posix import os + def generate(env): posix.generate(env) env['SHLIBSUFFIX'] = '.dylib' @@ -40,8 +41,7 @@ def generate(env): # env['ENV']['PATH'] = '/opt/local/bin:/opt/local/sbin:' + env['ENV']['PATH'] + ':/sw/bin' # Store extra system paths in env['ENV']['PATHOSX'] - env['ENV']['PATHOSX'] = '/opt/local/bin' - + filelist = ['/etc/paths',] # make sure this works on Macs with Tiger or earlier try: diff --git a/SCons/Util.py b/SCons/Util.py index a7a6307..33dc0d3 100644 --- a/SCons/Util.py +++ b/SCons/Util.py @@ -2156,9 +2156,7 @@ def sanitize_shell_env(execution_env): value = flatten_sequence(value) new_env[key] = os.pathsep.join(map(str, value)) else: - # It's either a string or something else. If it's a string, - # we still want to call str() because it might be a *Unicode* - # string, which makes subprocess.Popen() gag. If it isn't a + # It's either a string or something else. If it isn't a # string or a list, then we just coerce it to a string, which # is the proper way to handle Dir and File instances and will # produce something reasonable for just about everything else: -- cgit v0.12