From 9b420fed7e736c75cc5fd81613355f0b3db6e873 Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Thu, 5 Aug 2021 15:38:22 -0500 Subject: update ninja tool to support $ in scons command line args from https://github.com/mongodb/mongo/commit/dee0f733cdb3cab7714326574efc8a1ae4ec750f --- CHANGES.txt | 7 ++ RELEASE.txt | 6 ++ SCons/Tool/ninja/NinjaState.py | 12 ++-- SCons/Tool/ninja/__init__.py | 2 +- test/ninja/ninja_command_line.py | 82 ++++++++++++++++++++++ .../sconstruct_ninja_command_line | 12 ++++ 6 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 test/ninja/ninja_command_line.py create mode 100644 test/ninja/ninja_test_sconscripts/sconstruct_ninja_command_line diff --git a/CHANGES.txt b/CHANGES.txt index bedc08d..a27e0af 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,6 +34,13 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Fix ninja tool to never use for_sig substitution because ninja does not use signatures. This issue affected CommandGeneratorAction function actions specifically. - Added support for the PCH environment variable to support subst generators. + - Fix command line escaping for ninja dollar sign escape. Without escaping ninja properly, + the ninja file scons regenerate and callback invocations will lose the $ characters used in + the scons command line which ninja uses itself for escaping. For Example: + scons BUILD=xyz OTHERVAR=$BUILD + Prior to this fix, it would cause ninja to fail to escape the dollar sign, leading to the + single dollar sign being used as a ninja escape character in the ninja file. + From Mats Wichmann: - Two small Python 3.10 fixes: one more docstring turned into raw diff --git a/RELEASE.txt b/RELEASE.txt index f1411e4..439d7c4 100755 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -38,6 +38,12 @@ FIXES - Fix PCH not being evaluated by subst() where necessary. - Fix issue #4021. Change the way subst() is used in Textfile() to not evaluate '$$(' -> '$', but instead it should yield '$('. +- Fix command line escaping for ninja dollar sign escape. Without escaping ninja properly, + the ninja file scons regenerate and callback invocations will lose the $ characters used in + the scons command line which ninja uses itself for escaping. For Example: + scons BUILD=xyz OTHERVAR=$BUILD + Prior to this fix, it would cause ninja to fail to escape the dollar sign, leading to the + single dollar sign being used as a ninja escape character in the ninja file. IMPROVEMENTS ------------ diff --git a/SCons/Tool/ninja/NinjaState.py b/SCons/Tool/ninja/NinjaState.py index e7d9882..9c988b9 100644 --- a/SCons/Tool/ninja/NinjaState.py +++ b/SCons/Tool/ninja/NinjaState.py @@ -44,7 +44,7 @@ from .Methods import get_command class NinjaState: """Maintains state of Ninja build system as it's translated from SCons.""" - def __init__(self, env, ninja_file, writer_class): + def __init__(self, env, ninja_file, ninja_syntax): self.env = env self.ninja_file = ninja_file @@ -63,7 +63,7 @@ class NinjaState: # its in the path later self.ninja_bin_path = ninja_bin - self.writer_class = writer_class + self.writer_class = ninja_syntax.Writer self.__generated = False self.translator = SConsToNinjaTranslator(env) self.generated_suffixes = env.get("NINJA_GENERATED_SOURCE_SUFFIXES", []) @@ -80,23 +80,23 @@ class NinjaState: # shell quoting on whatever platform it's run on. Here we use it # to make the SCONS_INVOCATION variable properly quoted for things # like CCFLAGS - escape = env.get("ESCAPE", lambda x: x) + scons_escape = env.get("ESCAPE", lambda x: x) # if SCons was invoked from python, we expect the first arg to be the scons.py # script, otherwise scons was invoked from the scons script python_bin = '' if os.path.basename(sys.argv[0]) == 'scons.py': - python_bin = escape(sys.executable) + python_bin = ninja_syntax.escape(scons_escape(sys.executable)) self.variables = { "COPY": "cmd.exe /c 1>NUL copy" if sys.platform == "win32" else "cp", "SCONS_INVOCATION": '{} {} --disable-ninja __NINJA_NO=1 $out'.format( python_bin, " ".join( - [escape(arg) for arg in sys.argv if arg not in COMMAND_LINE_TARGETS] + [ninja_syntax.escape(scons_escape(arg)) for arg in sys.argv if arg not in COMMAND_LINE_TARGETS] ), ), "SCONS_INVOCATION_W_TARGETS": "{} {}".format( - python_bin, " ".join([escape(arg) for arg in sys.argv]) + python_bin, " ".join([ninja_syntax.escape(scons_escape(arg)) for arg in sys.argv]) ), # This must be set to a global default per: # https://ninja-build.org/manual.html#_deps diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py index 363c05d..6c70952 100644 --- a/SCons/Tool/ninja/__init__.py +++ b/SCons/Tool/ninja/__init__.py @@ -383,7 +383,7 @@ def generate(env): ninja_syntax = importlib.import_module(".ninja_syntax", package='ninja') if NINJA_STATE is None: - NINJA_STATE = NinjaState(env, ninja_file[0], ninja_syntax.Writer) + NINJA_STATE = NinjaState(env, ninja_file[0], ninja_syntax) # TODO: this is hacking into scons, preferable if there were a less intrusive way # We will subvert the normal builder execute to make sure all the ninja file is dependent diff --git a/test/ninja/ninja_command_line.py b/test/ninja/ninja_command_line.py new file mode 100644 index 0000000..d8e3c08 --- /dev/null +++ b/test/ninja/ninja_command_line.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python +# +# 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 +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY +# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE +# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +# 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. +# + +import os + +import TestSCons +from TestCmd import IS_WINDOWS + +test = TestSCons.TestSCons() + +try: + import ninja +except ImportError: + test.skip_test("Could not find module in python") + +_python_ = TestSCons._python_ +_exe = TestSCons._exe + +ninja_bin = os.path.abspath(os.path.join( + ninja.__file__, + os.pardir, + 'data', + 'bin', + 'ninja' + _exe)) + +test.dir_fixture('ninja-fixture', 'src') + +test.file_fixture('ninja_test_sconscripts/sconstruct_ninja_command_line', 'SConstruct') + +# generate simple build +test.run(arguments=['BUILD=build', 'OTHER_VAR=$BUILD'], stdout=None) +test.must_contain_all_lines(test.stdout(), ['Generating: build.ninja']) +test.must_contain_all(test.stdout(), 'Executing:') +test.must_contain_all(test.stdout(), 'ninja%(_exe)s -f' % locals()) +test.run(program=test.workpath('foo' + _exe), stdout="foo.c") + +# clean build and ninja files +test.run(arguments=['-c', 'BUILD=build', 'OTHER_VAR=$BUILD'], stdout=None) +test.must_contain_all_lines(test.stdout(), [ + 'Removed build%sbar2.c' % os.sep, + 'Removed build%sbar2.o' % os.sep, + 'Removed foo', + 'Removed build.ninja']) + +# only generate the ninja file +test.run(arguments=['--disable-execute-ninja', 'BUILD=build', 'OTHER_VAR=$BUILD'], stdout=None) +test.must_contain_all_lines(test.stdout(), ['Generating: build.ninja']) +test.must_not_exist(test.workpath('foo' + _exe)) + +# run ninja independently +program = test.workpath('run_ninja_env.bat') if IS_WINDOWS else ninja_bin +test.run(program=program, stdout=None) +test.run(program=test.workpath('foo' + _exe), stdout="foo.c") + +test.pass_test() + +# Local Variables: +# tab-width:4 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=4 shiftwidth=4: diff --git a/test/ninja/ninja_test_sconscripts/sconstruct_ninja_command_line b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_command_line new file mode 100644 index 0000000..14ae633 --- /dev/null +++ b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_command_line @@ -0,0 +1,12 @@ +SetOption('experimental','ninja') +DefaultEnvironment(tools=[]) + +env_vars=Variables(args=ARGUMENTS) +env_vars.Add('BUILD') +env_vars.Add('OTHER_VAR') +env = Environment(variables=env_vars) + +env.VariantDir(env['OTHER_VAR'], 'src') +env.Tool('ninja') +env.Textfile('$BUILD/bar2.c', open('src/foo.c').read()) +env.Program(target = 'foo', source = '$BUILD/bar2.c', CPPPATH='src') -- cgit v0.12 From 1b357561a89f090a7e5a9ee6b9dc5e4cac9e5c1e Mon Sep 17 00:00:00 2001 From: William Deegan Date: Sun, 26 Sep 2021 13:33:57 -0700 Subject: minor changes to RELEASE.txt and CHANGES.txt to have explicit example --- CHANGES.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index a27e0af..8948208 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -41,7 +41,6 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER Prior to this fix, it would cause ninja to fail to escape the dollar sign, leading to the single dollar sign being used as a ninja escape character in the ninja file. - From Mats Wichmann: - Two small Python 3.10 fixes: one more docstring turned into raw because it contained an escape; updated "helpful" syntax error message -- cgit v0.12