From 02c414bc3af7e29e9c75bc3b23b6e7123fbbf9a1 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 27 Sep 2023 07:59:16 -0600 Subject: Fix some problems in new scons_subproc_run The logic to handle the legacy 'error' kwarg supported by the predecssor _subproc function had some problems. Reworked. A few minor lint tweaks also. Signed-off-by: Mats Wichmann --- SCons/Action.py | 132 +++++++++++++++++++++++++------------------------------- 1 file changed, 58 insertions(+), 74 deletions(-) diff --git a/SCons/Action.py b/SCons/Action.py index e85e5e1..4d13595 100644 --- a/SCons/Action.py +++ b/SCons/Action.py @@ -109,15 +109,15 @@ import sys from abc import ABC, abstractmethod from collections import OrderedDict from subprocess import DEVNULL -from typing import Union import SCons.Debug import SCons.Errors import SCons.Subst import SCons.Util -from SCons.Debug import logInstanceCreation # we use these a lot, so try to optimize them +from SCons.Debug import logInstanceCreation +from SCons.Subst import SUBST_SIG, SUBST_RAW from SCons.Util import is_String, is_List class _null: @@ -387,9 +387,10 @@ def _object_instance_content(obj): # print("Inst Methods :\n%s"%pp.pformat(methods)) def _actionAppend(act1, act2): - # This function knows how to slap two actions together. - # Mainly, it handles ListActions by concatenating into - # a single ListAction. + """Joins two actions together. + + Mainly, it handles ListActions by concatenating into a single ListAction. + """ a1 = Action(act1) a2 = Action(act2) if a1 is None: @@ -399,13 +400,12 @@ def _actionAppend(act1, act2): if isinstance(a1, ListAction): if isinstance(a2, ListAction): return ListAction(a1.list + a2.list) - else: - return ListAction(a1.list + [ a2 ]) - else: - if isinstance(a2, ListAction): - return ListAction([ a1 ] + a2.list) - else: - return ListAction([ a1, a2 ]) + return ListAction(a1.list + [ a2 ]) + + if isinstance(a2, ListAction): + return ListAction([ a1 ] + a2.list) + + return ListAction([ a1, a2 ]) def _do_create_keywords(args, kw): @@ -468,11 +468,7 @@ def _do_create_action(act, kw): return CommandAction(act, **kw) if callable(act): - try: - gen = kw['generator'] - del kw['generator'] - except KeyError: - gen = 0 + gen = kw.pop('generator', 0) if gen: action_type = CommandGeneratorAction else: @@ -480,7 +476,7 @@ def _do_create_action(act, kw): return action_type(act, kw) # Catch a common error case with a nice message: - if isinstance(act, int) or isinstance(act, float): + if isinstance(act, (int, float)): raise TypeError("Don't know how to create an Action from a number (%s)"%act) # Else fail silently (???) return None @@ -504,10 +500,9 @@ def _do_create_list_action(act, kw) -> "ListAction": acts.append(aa) if not acts: return ListAction([]) - elif len(acts) == 1: + if len(acts) == 1: return acts[0] - else: - return ListAction(acts) + return ListAction(acts) def Action(act, *args, **kw): @@ -547,7 +542,7 @@ class ActionBase(ABC): batch_key = no_batch_key - def genstring(self, target, source, env): + def genstring(self, target, source, env, executor=None) -> str: return str(self) @abstractmethod @@ -561,7 +556,7 @@ class ActionBase(ABC): def get_contents(self, target, source, env): result = self.get_presig(target, source, env) - if not isinstance(result,(bytes, bytearray)): + if not isinstance(result, (bytes, bytearray)): result = bytearray(result, 'utf-8') else: # Make a copy and put in bytearray, without this the contents returned by get_presig @@ -579,17 +574,15 @@ class ActionBase(ABC): for v in vl: # do the subst this way to ignore $(...$) parts: if isinstance(result, bytearray): - result.extend(SCons.Util.to_bytes(env.subst_target_source('${'+v+'}', SCons.Subst.SUBST_SIG, target, source))) + result.extend(SCons.Util.to_bytes(env.subst_target_source('${'+v+'}', SUBST_SIG, target, source))) else: raise Exception("WE SHOULD NEVER GET HERE result should be bytearray not:%s"%type(result)) - # result.append(SCons.Util.to_bytes(env.subst_target_source('${'+v+'}', SCons.Subst.SUBST_SIG, target, source))) + # result.append(SCons.Util.to_bytes(env.subst_target_source('${'+v+'}', SUBST_SIG, target, source))) - - if isinstance(result, (bytes,bytearray)): + if isinstance(result, (bytes, bytearray)): return result - else: - raise Exception("WE SHOULD NEVER GET HERE - #2 result should be bytearray not:%s" % type(result)) - # return b''.join(result) + + raise Exception("WE SHOULD NEVER GET HERE - #2 result should be bytearray not:%s" % type(result)) def __add__(self, other): return _actionAppend(self, other) @@ -788,7 +781,7 @@ def get_default_ENV(env): return env['ENV'] except KeyError: if not default_ENV: - import SCons.Environment + import SCons.Environment # pylint: disable=import-outside-toplevel,redefined-outer-name # This is a hideously expensive way to get a default execution # environment. What it really should do is run the platform # setup to get the default ENV. Fortunately, it's incredibly @@ -815,18 +808,17 @@ def _resolve_shell_env(env, target, source): shell_gens = iter(shell_gen) except TypeError: raise SCons.Errors.UserError("SHELL_ENV_GENERATORS must be iteratable.") - else: - ENV = ENV.copy() - for generator in shell_gens: - ENV = generator(env, target, source, ENV) - if not isinstance(ENV, dict): - raise SCons.Errors.UserError(f"SHELL_ENV_GENERATORS function: {generator} must return a dict.") + + ENV = ENV.copy() + for generator in shell_gens: + ENV = generator(env, target, source, ENV) + if not isinstance(ENV, dict): + raise SCons.Errors.UserError(f"SHELL_ENV_GENERATORS function: {generator} must return a dict.") + return ENV -def scons_subproc_run( - scons_env, *args, error: str = None, **kwargs -) -> subprocess.CompletedProcess: +def scons_subproc_run(scons_env, *args, **kwargs) -> subprocess.CompletedProcess: """Run a command with arguments using an SCons execution environment. Does an underlyng call to :func:`subprocess.run` to run a command and @@ -841,8 +833,8 @@ def scons_subproc_run( execution environment to process into appropriate form before it is supplied to :mod:`subprocess`; if omitted, *scons_env* is used to derive a suitable default. The other keyword arguments are passed through, - except that SCons legacy ``error` keyword is remapped to - the subprocess ``check` keyword. The caller is responsible for + except that SCons legacy ``error`` keyword is remapped to + the subprocess ``check`` keyword. The caller is responsible for setting up the desired arguments for :func:`subprocess.run`. This function retains the legacy behavior of returning something @@ -882,13 +874,13 @@ def scons_subproc_run( ENV = get_default_ENV(scons_env) kwargs['env'] = SCons.Util.sanitize_shell_env(ENV) - # backwards-compat with _subproc: accept 'error', map it to - # ``check`` and remove, since it would not be recognized by run() - check = kwargs.get('check') - if check and error: + # Backwards-compat with _subproc: accept 'error', map to 'check', + # and remove, since subprocess.run does not recognize. + # 'error' isn't True/False, it takes a string value (see _subproc) + error = kwargs.get('error') + if error and 'check' in kwargs: raise ValueError('error and check arguments may not both be used.') - if check is None: - check = False # always supply some value, to pacify checkers (pylint). + check = kwargs.get('check', False) # always set a value for 'check' if error is not None: if error == 'raise': check = True @@ -915,11 +907,12 @@ def scons_subproc_run( # Some tools/tests expect no failures for things like missing files # unless raise/check is set. If set, let subprocess.run go ahead and # kill things, else catch and construct a dummy CompletedProcess instance. + # Note pylint can't see we always include 'check', so quiet it. if check: - cp = subprocess.run(*args, **kwargs) + cp = subprocess.run(*args, **kwargs) # pylint: disable=subprocess-run-check else: try: - cp = subprocess.run(*args, **kwargs) + cp = subprocess.run(*args, **kwargs) # pylint: disable=subprocess-run-check except OSError as exc: argline = ' '.join(*args) cp = subprocess.CompletedProcess(argline, 1) @@ -1044,7 +1037,6 @@ class CommandAction(_ActionAction): if self.cmdstr is None: return None if self.cmdstr is not _null: - from SCons.Subst import SUBST_RAW if executor: c = env.subst(self.cmdstr, SUBST_RAW, executor=executor, overrides=overrides) else: @@ -1077,12 +1069,11 @@ class CommandAction(_ActionAction): spawn = env['SPAWN'] except KeyError: raise SCons.Errors.UserError('Missing SPAWN construction variable.') - else: - if is_String(spawn): - spawn = env.subst(spawn, raw=1, conv=lambda x: x) - escape = env.get('ESCAPE', lambda x: x) + if is_String(spawn): + spawn = env.subst(spawn, raw=1, conv=lambda x: x) + escape = env.get('ESCAPE', lambda x: x) ENV = _resolve_shell_env(env, target, source) # Ensure that the ENV values are all strings: @@ -1125,7 +1116,6 @@ class CommandAction(_ActionAction): This strips $(-$) and everything in between the string, since those parts don't affect signatures. """ - from SCons.Subst import SUBST_SIG cmd = self.cmd_list if is_List(cmd): cmd = ' '.join(map(str, cmd)) @@ -1133,8 +1123,7 @@ class CommandAction(_ActionAction): cmd = str(cmd) if executor: return env.subst_target_source(cmd, SUBST_SIG, executor=executor) - else: - return env.subst_target_source(cmd, SUBST_SIG, target, source) + return env.subst_target_source(cmd, SUBST_SIG, target, source) def get_implicit_deps(self, target, source, env, executor=None): """Return the implicit dependencies of this action's command line.""" @@ -1154,17 +1143,15 @@ class CommandAction(_ActionAction): # An integer value greater than 1 specifies the number of entries # to scan. "all" means to scan all. return self._get_implicit_deps_heavyweight(target, source, env, executor, icd_int) - else: - # Everything else (usually 1 or True) means that we want - # lightweight dependency scanning. - return self._get_implicit_deps_lightweight(target, source, env, executor) + # Everything else (usually 1 or True) means that we want + # lightweight dependency scanning. + return self._get_implicit_deps_lightweight(target, source, env, executor) def _get_implicit_deps_lightweight(self, target, source, env, executor): """ Lightweight dependency scanning involves only scanning the first entry in an action string, even if it contains &&. """ - from SCons.Subst import SUBST_SIG if executor: cmd_list = env.subst_list(self.cmd_list, SUBST_SIG, executor=executor) else: @@ -1202,7 +1189,6 @@ class CommandAction(_ActionAction): # Avoid circular and duplicate dependencies by not providing source, # target, or executor to subst_list. This causes references to # $SOURCES, $TARGETS, and all related variables to disappear. - from SCons.Subst import SUBST_SIG cmd_list = env.subst_list(self.cmd_list, SUBST_SIG, conv=lambda x: x) res = [] @@ -1281,7 +1267,7 @@ class CommandGeneratorAction(ActionBase): def batch_key(self, env, target, source): return self._generate(target, source, env, 1).batch_key(env, target, source) - def genstring(self, target, source, env, executor=None): + def genstring(self, target, source, env, executor=None) -> str: return self._generate(target, source, env, 1, executor).genstring(target, source, env) def __call__(self, target, source, env, exitstatfunc=_null, presub=_null, @@ -1409,7 +1395,6 @@ class FunctionAction(_ActionAction): if self.cmdstr is None: return None if self.cmdstr is not _null: - from SCons.Subst import SUBST_RAW if executor: c = env.subst(self.cmdstr, SUBST_RAW, executor=executor) else: @@ -1456,9 +1441,7 @@ class FunctionAction(_ActionAction): rsources = list(map(rfile, source)) try: result = self.execfunction(target=target, source=rsources, env=env) - except KeyboardInterrupt as e: - raise - except SystemExit as e: + except (KeyboardInterrupt, SystemExit): raise except Exception as e: result = e @@ -1466,8 +1449,8 @@ class FunctionAction(_ActionAction): if result: result = SCons.Errors.convert_to_BuildError(result, exc_info) - result.node=target - result.action=self + result.node = target + result.action = self try: result.command=self.strfunction(target, source, env, executor) except TypeError: @@ -1480,7 +1463,7 @@ class FunctionAction(_ActionAction): # some codes do not check the return value of Actions and I do # not have the time to modify them at this point. if (exc_info[1] and - not isinstance(exc_info[1],EnvironmentError)): + not isinstance(exc_info[1], EnvironmentError)): raise result return result @@ -1514,7 +1497,7 @@ class ListAction(ActionBase): self.varlist = () self.targets = '$TARGETS' - def genstring(self, target, source, env): + def genstring(self, target, source, env, executor=None) -> str: return '\n'.join([a.genstring(target, source, env) for a in self.list]) def __str__(self) -> str: @@ -1601,8 +1584,9 @@ class ActionCaller: # was called by using this hard-coded value as a special return. if s == '$__env__': return env - elif is_String(s): + if is_String(s): return env.subst(s, 1, target, source) + return self.parent.convert(s) def subst_args(self, target, source, env): -- cgit v0.12 From 964d74af5b943062c3305109f1a94fe03c25b9b3 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Tue, 3 Oct 2023 15:55:17 -0600 Subject: scons_subproc_run: update docstrings, add tests Documentation reworded (docstring/comment only, this is not a public API). Removed duplicated cleaning of execution env - previous iteration both called the Util function and then did the same work manually. Added some unit tests for the argument fiddling. Signed-off-by: Mats Wichmann --- SCons/Action.py | 104 +++++++++++++++++++++++++-------------------------- SCons/ActionTests.py | 51 ++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 54 deletions(-) diff --git a/SCons/Action.py b/SCons/Action.py index 4d13595..0dd02cc 100644 --- a/SCons/Action.py +++ b/SCons/Action.py @@ -108,7 +108,7 @@ import subprocess import sys from abc import ABC, abstractmethod from collections import OrderedDict -from subprocess import DEVNULL +from subprocess import DEVNULL, PIPE import SCons.Debug import SCons.Errors @@ -468,7 +468,7 @@ def _do_create_action(act, kw): return CommandAction(act, **kw) if callable(act): - gen = kw.pop('generator', 0) + gen = kw.pop('generator', False) if gen: action_type = CommandGeneratorAction else: @@ -819,30 +819,36 @@ def _resolve_shell_env(env, target, source): def scons_subproc_run(scons_env, *args, **kwargs) -> subprocess.CompletedProcess: - """Run a command with arguments using an SCons execution environment. - - Does an underlyng call to :func:`subprocess.run` to run a command and - returns a :class:`subprocess.CompletedProcess` instance with the results. - Use when an external command needs to run in an "SCons context" - - that is, with a crafted execution environment, rather than the user's - existing environment, particularly when you need to collect output - back. Typical case: run a tool's "version" option to find out which - version you have installed. - - If supplied, the ``env`` keyword argument passes an - execution environment to process into appropriate form before it is - supplied to :mod:`subprocess`; if omitted, *scons_env* is used to derive - a suitable default. The other keyword arguments are passed through, - except that SCons legacy ``error`` keyword is remapped to - the subprocess ``check`` keyword. The caller is responsible for - setting up the desired arguments for :func:`subprocess.run`. + """Run an external command using an SCons execution environment. + + SCons normally runs external build commands using :mod:`subprocess`, + but does not harvest any output from such commands. This function + is a thin wrapper around :func:`subprocess.run` allowing running + a command in an SCons context (i.e. uses an "execution environment" + rather than the user's existing environment), and provides the ability + to return any output in a :class:`subprocess.CompletedProcess` + instance (this must be selected by setting ``stdout`` and/or + ``stderr`` to ``PIPE``, or setting ``capture_output=True`` - see + Keyword Arguments). Typical use case is to run a tool's "version" + option to find out the installed version. + + If supplied, the ``env`` keyword argument provides an execution + environment to process into appropriate form before it is supplied + to :mod:`subprocess`; if omitted, *scons_env* is used to derive a + suitable default. The other keyword arguments are passed through, + except that the SCons legacy ``error`` keyword is remapped to the + subprocess ``check`` keyword; if both are omitted ``check=False`` + will be passed. The caller is responsible for setting up the desired + arguments for :func:`subprocess.run`. This function retains the legacy behavior of returning something - vaguely usable even in the face of complete failure: it synthesizes - a :class:`~subprocess.CompletedProcess` instance in this case. + vaguely usable even in the face of complete failure, unless + ``check=True`` (in which case an error is allowed to be raised): + it synthesizes a :class:`~subprocess.CompletedProcess` instance in + this case. A subset of interesting keyword arguments follows; see the Python - documentation of :mod:`subprocess` for a complete list. + documentation of :mod:`subprocess` for the complete list. Keyword Arguments: stdout: (and *stderr*, *stdin*) if set to :const:`subprocess.PIPE`. @@ -850,12 +856,11 @@ def scons_subproc_run(scons_env, *args, **kwargs) -> subprocess.CompletedProcess the subprocess; the default ``None`` does no redirection (i.e. output or errors may go to the console or log file, but is not captured); if set to :const:`subprocess.DEVNULL` - they are explicitly thrown away. ``capture_output`` is a + they are explicitly thrown away. ``capture_output=True`` is a synonym for setting both ``stdout`` and ``stderr`` to :const:`~subprocess.PIPE`. text: open *stdin*, *stdout*, *stderr* in text mode. Default - is binary mode. ``universal_newlines`` is a synonym - - note ``text`` is not understood before Python 3.7. + is binary mode. ``universal_newlines`` is a synonym. encoding: specifies an encoding. Changes to text mode. errors: specified error handling. Changes to text mode. input: a byte sequence to be passed to *stdin*, unless text @@ -869,10 +874,10 @@ def scons_subproc_run(scons_env, *args, **kwargs) -> subprocess.CompletedProcess .. versionadded:: 4.6 """ # Figure out the execution environment to use - ENV = kwargs.get('env', None) - if ENV is None: - ENV = get_default_ENV(scons_env) - kwargs['env'] = SCons.Util.sanitize_shell_env(ENV) + env = kwargs.get('env', None) + if env is None: + env = get_default_ENV(scons_env) + kwargs['env'] = SCons.Util.sanitize_shell_env(env) # Backwards-compat with _subproc: accept 'error', map to 'check', # and remove, since subprocess.run does not recognize. @@ -887,27 +892,20 @@ def scons_subproc_run(scons_env, *args, **kwargs) -> subprocess.CompletedProcess del kwargs['error'] kwargs['check'] = check - # 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(str(v) for v in value) - else: - # If it's not a list type, "convert" it to str. This is - # harmless if it's already a string, but gets us the right - # thing for Dir and File instances and will produce something - # reasonable for just about everything else: - new_env[key] = str(value) - kwargs['env'] = new_env - - # Some tools/tests expect no failures for things like missing files - # unless raise/check is set. If set, let subprocess.run go ahead and - # kill things, else catch and construct a dummy CompletedProcess instance. - # Note pylint can't see we always include 'check', so quiet it. + # TODO: Python version-compat stuff: remap/remove too-new args if needed + if 'text' in kwargs and sys.version_info[:3] < (3, 7): + kwargs['universal_newlines'] = kwargs.pop('text') + + if 'capture_output' in kwargs and sys.version_info[:3] < (3, 7): + capture_output = kwargs.pop('capture_output') + if capture_output: + kwargs['stdout'] = kwargs['stderr'] = PIPE + + # Most SCons tools/tests expect not to fail on things like missing files. + # check=True (or error="raise") means we're okay to take an exception; + # else we catch the likely exception and construct a dummy + # CompletedProcess instance. + # Note pylint can't see we always include 'check' in kwargs: suppress. if check: cp = subprocess.run(*args, **kwargs) # pylint: disable=subprocess-run-check else: @@ -915,9 +913,9 @@ def scons_subproc_run(scons_env, *args, **kwargs) -> subprocess.CompletedProcess cp = subprocess.run(*args, **kwargs) # pylint: disable=subprocess-run-check except OSError as exc: argline = ' '.join(*args) - cp = subprocess.CompletedProcess(argline, 1) - cp.stdout = "" - cp.stderr = "" + cp = subprocess.CompletedProcess( + args=argline, returncode=1, stdout="", stderr="" + ) return cp diff --git a/SCons/ActionTests.py b/SCons/ActionTests.py index 7239f4f..4e0ae72 100644 --- a/SCons/ActionTests.py +++ b/SCons/ActionTests.py @@ -41,11 +41,13 @@ import os import sys import types import unittest +from unittest import mock from subprocess import PIPE import SCons.Action import SCons.Environment import SCons.Errors +from SCons.Action import scons_subproc_run import TestCmd @@ -2329,7 +2331,7 @@ class ObjectContentsTestCase(unittest.TestCase): def test_uncaught_exception_bubbles(self): """Test that scons_subproc_run bubbles uncaught exceptions""" try: - cp = SCons.Action.scons_subproc_run(Environment(), None, stdout=PIPE) + cp = scons_subproc_run(Environment(), None, stdout=PIPE) except EnvironmentError: pass except Exception: @@ -2338,6 +2340,53 @@ class ObjectContentsTestCase(unittest.TestCase): raise Exception("expected a non-EnvironmentError exception") + + def mock_subprocess_run(*args, **kwargs): + """Replacement subprocess.run: return kwargs for checking.""" + kwargs.pop("env") # the value of env isn't interesting here + return kwargs + + @mock.patch("subprocess.run", mock_subprocess_run) + def test_scons_subproc_run(self): + """Test the argument remapping options.""" + env = Environment() + self.assertEqual(scons_subproc_run(env), {"check": False}) + with self.subTest(): + self.assertEqual( + scons_subproc_run(env, error="raise"), + {"check": True} + ) + with self.subTest(): + self.assertEqual( + scons_subproc_run(env, capture_output=True), + {"capture_output": True, "check": False}, + ) + with self.subTest(): + self.assertEqual( + scons_subproc_run(env, text=True), + {"text": True, "check": False}, + ) + + # 3.6: + save_info, sys.version_info = sys.version_info, (3, 6, 2) + with self.subTest(): + self.assertEqual( + scons_subproc_run(env, capture_output=True), + {"check": False, "stdout": PIPE, "stderr": PIPE}, + ) + with self.subTest(): + self.assertEqual( + scons_subproc_run(env, text=True), + {"check": False, "universal_newlines": True}, + ) + with self.subTest(): + self.assertEqual( + scons_subproc_run(env, universal_newlines=True), + {"universal_newlines": True, "check": False}, + ) + sys.version_info = save_info + + if __name__ == "__main__": unittest.main() -- cgit v0.12 From bbe8226efe1d57b5ca70bd5abfb5c5458d46ad10 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 4 Oct 2023 07:13:21 -0600 Subject: Make sure scons_subproc_run test also passes on py3.6 Signed-off-by: Mats Wichmann --- SCons/ActionTests.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/SCons/ActionTests.py b/SCons/ActionTests.py index 4e0ae72..7d99a25 100644 --- a/SCons/ActionTests.py +++ b/SCons/ActionTests.py @@ -2349,6 +2349,9 @@ class ObjectContentsTestCase(unittest.TestCase): @mock.patch("subprocess.run", mock_subprocess_run) def test_scons_subproc_run(self): """Test the argument remapping options.""" + # set phony Python versions to trigger the logic in scons_subproc_run: + # any version greater than 3.6, really + save_info, sys.version_info = sys.version_info, (3, 11, 1) env = Environment() self.assertEqual(scons_subproc_run(env), {"check": False}) with self.subTest(): @@ -2368,7 +2371,7 @@ class ObjectContentsTestCase(unittest.TestCase): ) # 3.6: - save_info, sys.version_info = sys.version_info, (3, 6, 2) + sys.version_info = (3, 6, 2) with self.subTest(): self.assertEqual( scons_subproc_run(env, capture_output=True), -- cgit v0.12 From aca9467fc18bca65ab625bed66ff7095753c5d44 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Fri, 6 Oct 2023 13:25:11 -0600 Subject: Fixup one test framework test [skip appveyor] Looking at wrong exceptions for possible rmdir failures Signed-off-by: Mats Wichmann --- testing/framework/TestCmdTests.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/testing/framework/TestCmdTests.py b/testing/framework/TestCmdTests.py index dc752ba..7cabf44 100644 --- a/testing/framework/TestCmdTests.py +++ b/testing/framework/TestCmdTests.py @@ -1601,10 +1601,10 @@ class rmdir_TestCase(TestCmdTestCase): try: test.rmdir(['no', 'such', 'dir']) - except EnvironmentError: + except FileNotFoundError: pass else: - raise Exception("did not catch expected SConsEnvironmentError") + raise Exception("did not catch expected FileNotFoundError") test.subdir(['sub'], ['sub', 'dir'], @@ -1616,19 +1616,19 @@ class rmdir_TestCase(TestCmdTestCase): try: test.rmdir(['sub']) - except EnvironmentError: + except OSError: pass else: - raise Exception("did not catch expected SConsEnvironmentError") + raise Exception("did not catch expected OSError") assert os.path.isdir(s_d_o), f"{s_d_o} is gone?" try: test.rmdir(['sub']) - except EnvironmentError: + except OSError: pass else: - raise Exception("did not catch expected SConsEnvironmentError") + raise Exception("did not catch expected OSError") assert os.path.isdir(s_d_o), f"{s_d_o} is gone?" @@ -1647,7 +1647,6 @@ class rmdir_TestCase(TestCmdTestCase): assert not os.path.exists(s), f"{s} exists?" - class run_TestCase(TestCmdTestCase): def test_run(self) -> None: """Test run()""" -- cgit v0.12 From 50783560e935d55dfe0f3e437b978be9ae267788 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 11 Oct 2023 08:05:22 -0600 Subject: Update assembly code in one test Avoids warnings, which fail the test. Fixes #4238 Signed-off-by: Mats Wichmann --- CHANGES.txt | 2 ++ test/AS/as-live.py | 73 ++++++++++++++++++++++++++++-------------------------- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a55c532..b2e7182 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -179,6 +179,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER Python stdlib types module. - TeX tests: skip tests that use makeindex or epstopdf not installed, or if `kpsewhich glossaries.sty` fails. + - Added a .note.GNU-stack section to the test assembler files to + avoid the GNU linker issuing warnings for its absence. From Jonathon Reinhart: diff --git a/test/AS/as-live.py b/test/AS/as-live.py index 676b537..4a21fcf 100644 --- a/test/AS/as-live.py +++ b/test/AS/as-live.py @@ -1,6 +1,8 @@ #!/usr/bin/env python # -# __COPYRIGHT__ +# MIT License +# +# Copyright The SCons Foundation # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -20,9 +22,6 @@ # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -# - -__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" """ Verify correct use of the live 'as' assembler. @@ -37,24 +36,22 @@ _exe = TestSCons._exe test = TestSCons.TestSCons() - - if not test.detect('AS', 'as'): test.skip_test("as not found; skipping test\n") x86 = (sys.platform == 'win32' or sys.platform.find('linux') != -1) - if not x86: test.skip_test("skipping as test on non-x86 platform '%s'\n" % sys.platform) namelbl = "name" -testccc = """ccc = aaa.Clone(CPPPATH=['.']) -ccc.Program(target = 'ccc', source = ['ccc.S', 'ccc_main.c']) +testccc = """\ +ccc = aaa.Clone(CPPPATH=['.']) +ccc.Program(target='ccc', source=['ccc.S', 'ccc_main.c']) """ if sys.platform == "win32": namelbl = "_name" testccc = "" - + test.write("wrapper.py", """\ import subprocess import sys @@ -66,30 +63,34 @@ subprocess.run(cmd, shell=True) test.write('SConstruct', """\ aaa = Environment() -aaa.Program(target = 'aaa', source = ['aaa.s', 'aaa_main.c']) -bbb = aaa.Clone(AS = r'%(_python_)s wrapper.py ' + WhereIs('as')) -bbb.Program(target = 'bbb', source = ['bbb.s', 'bbb_main.c']) +aaa.Program(target='aaa', source=['aaa.s', 'aaa_main.c']) +bbb = aaa.Clone(AS=r'%(_python_)s wrapper.py ' + WhereIs('as')) +bbb.Program(target='bbb', source=['bbb.s', 'bbb_main.c']) %(testccc)s """ % locals()) -test.write('aaa.s', -""" .file "aaa.s" -.data -.align 4 -.globl %(namelbl)s +test.write('aaa.s', """ + .file "aaa.s" + .data + .align 4 + .globl %(namelbl)s %(namelbl)s: - .ascii "aaa.s" - .byte 0 + .ascii "aaa.s" + .byte 0 + .ident "handcrafted test assembly" + .section .note.GNU-stack,"",@progbits """ % locals()) test.write('bbb.s', """\ -.file "bbb.s" -.data -.align 4 -.globl %(namelbl)s + .file "bbb.s" + .data + .align 4 + .globl %(namelbl)s %(namelbl)s: - .ascii "bbb.s" - .byte 0 + .ascii "bbb.s" + .byte 0 + .ident "handcrafted test assembly" + .section .note.GNU-stack,"",@progbits """ % locals()) test.write('ccc.h', """\ @@ -98,13 +99,15 @@ test.write('ccc.h', """\ test.write('ccc.S', """\ #include -.file STRING -.data -.align 4 -.globl name + .file STRING + .data + .align 4 + .globl name name: - .ascii STRING - .byte 0 + .ascii STRING + .byte 0 + .ident "handcrafted test assembly" + .section .note.GNU-stack,"",@progbits """) test.write('aaa_main.c', r""" @@ -171,13 +174,13 @@ test.run(program = test.workpath('bbb'), stdout = "bbb_main.c bbb.s\n") if sys.platform != "win32": test.run(program = test.workpath('ccc'), stdout = "ccc_main.c ccc.S\n") - + test.must_match('wrapper.out', "wrapper.py: bbb.s\n") - + test.write("ccc.h", """\ #define STRING "ccc.S 2" """) - + test.run() test.run(program = test.workpath('ccc'), stdout = "ccc_main.c ccc.S 2\n") -- cgit v0.12 From 78b73a7e3d80190058154aad3cf4fc709638fcf0 Mon Sep 17 00:00:00 2001 From: Joseph Brill <48932340+jcbrill@users.noreply.github.com> Date: Wed, 11 Oct 2023 13:00:30 -0400 Subject: Change the module imports from relative imports to top-level absolute imports for the Microsoft tools. Moving any of the Microsoft tools that used relative imports to the scons site tools folder would fail on import (i.e., the relative import paths become invalid when the tools are moved). --- CHANGES.txt | 5 +++++ RELEASE.txt | 5 +++++ SCons/Tool/midl.py | 2 +- SCons/Tool/mslib.py | 5 ++++- SCons/Tool/mslink.py | 7 +++++-- SCons/Tool/mssdk.py | 6 ++++-- SCons/Tool/msvc.py | 9 +++++++-- SCons/Tool/msvs.py | 5 ++++- 8 files changed, 35 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a55c532..ff7ef50 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -42,6 +42,11 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER registry query that returns a path that does not exist. Multiple invocation paths were not prepared to handle the MissingConfiguration exception. The MissingConfiguration exception type was removed. + - The MSCommon module import was changed from a relative import to a top-level + absolute import in the following Microsoft tools: midl, mslib, mslink, mssdk, msvc, + msvs. Moving any of these tools that used relative imports to the scons site tools + folder would fail on import (i.e., the relative import paths become invalid when + moved). From Vitaly Cheptsov: - Fix race condition in `Mkdir` which can happen when two `SConscript` diff --git a/RELEASE.txt b/RELEASE.txt index 12bb3ca..764cc40 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -120,6 +120,11 @@ IMPROVEMENTS ------------ - Now tries to find mingw if it comes from Chocolatey install of msys2. +- MSVC: Module imports were changed from a relative import to a top-level + absolute import in the following Microsoft tools: midl, mslib, mslink, mssdk, msvc, + msvs. Moving any of these tools that used relative imports to the scons site tools + folder would fail on import (i.e., the relative import paths become invalid when + moved). PACKAGING --------- diff --git a/SCons/Tool/midl.py b/SCons/Tool/midl.py index 0c640f5..2ae3f73 100644 --- a/SCons/Tool/midl.py +++ b/SCons/Tool/midl.py @@ -37,7 +37,7 @@ import SCons.Defaults import SCons.Scanner.IDL import SCons.Util -from .MSCommon import msvc_setup_env_tool +from SCons.Tool.MSCommon import msvc_setup_env_tool tool_name = 'midl' diff --git a/SCons/Tool/mslib.py b/SCons/Tool/mslib.py index 6e15a80..bdce135 100644 --- a/SCons/Tool/mslib.py +++ b/SCons/Tool/mslib.py @@ -41,7 +41,10 @@ import SCons.Tool.msvs import SCons.Tool.msvc import SCons.Util -from .MSCommon import msvc_setup_env_tool, msvc_setup_env_once +from SCons.Tool.MSCommon import ( + msvc_setup_env_tool, + msvc_setup_env_once, +) tool_name = 'mslib' diff --git a/SCons/Tool/mslink.py b/SCons/Tool/mslink.py index 1e5b71a..74ceaa8 100644 --- a/SCons/Tool/mslink.py +++ b/SCons/Tool/mslink.py @@ -43,8 +43,11 @@ import SCons.Tool.msvc import SCons.Tool.msvs import SCons.Util -from .MSCommon import msvc_setup_env_once, msvc_setup_env_tool -from .MSCommon.common import get_pch_node +from SCons.Tool.MSCommon import ( + msvc_setup_env_once, + msvc_setup_env_tool, +) +from SCons.Tool.MSCommon.common import get_pch_node tool_name = 'mslink' diff --git a/SCons/Tool/mssdk.py b/SCons/Tool/mssdk.py index 0151eff..ef272c0 100644 --- a/SCons/Tool/mssdk.py +++ b/SCons/Tool/mssdk.py @@ -33,8 +33,10 @@ It will usually be imported through the generic SCons.Tool.Tool() selection method. """ -from .MSCommon import mssdk_exists, \ - mssdk_setup_env +from SCons.Tool.MSCommon import ( + mssdk_exists, + mssdk_setup_env, +) def generate(env) -> None: """Add construction variables for an MS SDK to an Environment.""" diff --git a/SCons/Tool/msvc.py b/SCons/Tool/msvc.py index 33a67d0..6afa171 100644 --- a/SCons/Tool/msvc.py +++ b/SCons/Tool/msvc.py @@ -44,8 +44,13 @@ import SCons.Util import SCons.Warnings import SCons.Scanner.RC -from .MSCommon import msvc_setup_env_tool, msvc_setup_env_once, msvc_version_to_maj_min, msvc_find_vswhere -from .MSCommon.common import get_pch_node +from SCons.Tool.MSCommon import ( + msvc_setup_env_tool, + msvc_setup_env_once, + msvc_version_to_maj_min, + msvc_find_vswhere, +) +from SCons.Tool.MSCommon.common import get_pch_node tool_name = 'msvc' diff --git a/SCons/Tool/msvs.py b/SCons/Tool/msvs.py index 153b84e..16e422d 100644 --- a/SCons/Tool/msvs.py +++ b/SCons/Tool/msvs.py @@ -45,7 +45,10 @@ import SCons.Util import SCons.Warnings from SCons.Defaults import processDefines from SCons.compat import PICKLE_PROTOCOL -from .MSCommon import msvc_setup_env_tool, msvc_setup_env_once +from SCons.Tool.MSCommon import ( + msvc_setup_env_tool, + msvc_setup_env_once, +) tool_name = 'msvs' -- cgit v0.12