From ba3dcaab98cf8c5156c7d5f80589a2b57162d02b Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Fri, 6 May 2022 09:19:24 -0500 Subject: improved ninja default targets and for ninja file as only target when generating. --- CHANGES.txt | 3 + RELEASE.txt | 4 + SCons/Tool/ninja/Globals.py | 1 + SCons/Tool/ninja/NinjaState.py | 93 +++++++++++++++------- SCons/Tool/ninja/__init__.py | 22 ++++- SCons/Tool/ninja/ninja_daemon_build.py | 9 ++- SCons/Tool/ninja/ninja_scons_daemon.py | 29 ++++--- test/ninja/default_targets.py | 83 +++++++++++++++++++ test/ninja/force_scons_callback.py | 2 +- .../sconstruct_default_targets | 14 ++++ 10 files changed, 215 insertions(+), 45 deletions(-) create mode 100644 test/ninja/default_targets.py create mode 100644 test/ninja/ninja_test_sconscripts/sconstruct_default_targets diff --git a/CHANGES.txt b/CHANGES.txt index 42d20d7..eee3fd2 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -80,6 +80,9 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Added user configurable setting of ninja depfile format via NINJA_DEPFILE_PARSE_FORMAT. Now setting NINJA_DEPFILE_PARSE_FORMAT to [msvc,gcc,clang] can force the ninja expected format. Compiler tools will also configure the variable automatically. + - Made ninja tool force the ninja file as the only target. Also improved the default + targets setup and made sure there is always a default target for + the ninja file, which excludes targets that start and stop the daemon. From Mats Wichmann: - Tweak the way default site_scons paths on Windows are expressed to diff --git a/RELEASE.txt b/RELEASE.txt index 6bb1546..3f8a11e 100755 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -49,6 +49,10 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY output format written to stdout to include more information about the source for each message of MSVC initialization debugging output. A single space was added before the message for all debugging output records written to stdout and to files. +- Made ninja tool force the ninja file as the only target. Also improved the default + targets setup and made sure there is always a default target for + the ninja file, which excludes targets that start and stop the daemon. + FIXES ----- diff --git a/SCons/Tool/ninja/Globals.py b/SCons/Tool/ninja/Globals.py index 4d1d38b..d84cae6 100644 --- a/SCons/Tool/ninja/Globals.py +++ b/SCons/Tool/ninja/Globals.py @@ -29,6 +29,7 @@ NINJA_CUSTOM_HANDLERS = "__NINJA_CUSTOM_HANDLERS" NINJA_BUILD = "NINJA_BUILD" NINJA_WHEREIS_MEMO = {} NINJA_STAT_MEMO = {} +NINJA_DEFAULT_TARGETS = [] __NINJA_RULE_MAPPING = {} # These are the types that get_command can do something with diff --git a/SCons/Tool/ninja/NinjaState.py b/SCons/Tool/ninja/NinjaState.py index 63ea3a1..5f8d5c7 100644 --- a/SCons/Tool/ninja/NinjaState.py +++ b/SCons/Tool/ninja/NinjaState.py @@ -29,6 +29,7 @@ import signal import tempfile import shutil import sys +import time from os.path import splitext from tempfile import NamedTemporaryFile import ninja @@ -39,7 +40,7 @@ from SCons.Script import COMMAND_LINE_TARGETS from SCons.Util import is_List from SCons.Errors import InternalError from .Globals import COMMAND_TYPES, NINJA_RULES, NINJA_POOLS, \ - NINJA_CUSTOM_HANDLERS + NINJA_CUSTOM_HANDLERS, NINJA_DEFAULT_TARGETS from .Rules import _install_action_function, _mkdir_action_function, _lib_symlink_action_function, _copy_action_function from .Utils import get_path, alias_to_ninja_build, generate_depfile, ninja_noop, get_order_only, \ get_outputs, get_inputs, get_dependencies, get_rule, get_command_env, to_escaped_list @@ -68,7 +69,7 @@ class NinjaState: # couldn't find it, just give the bin name and hope # its in the path later self.ninja_bin_path = ninja_bin - + self.ninja_syntax = ninja_syntax self.writer_class = ninja_syntax.Writer self.__generated = False self.translator = SConsToNinjaTranslator(env) @@ -77,11 +78,6 @@ class NinjaState: # List of generated builds that will be written at a later stage self.builds = dict() - # List of targets for which we have generated a build. This - # allows us to take multiple Alias nodes as sources and to not - # fail to build if they have overlapping targets. - self.built = set() - # SCons sets this variable to a function which knows how to do # shell quoting on whatever platform it's run on. Here we use it # to make the SCONS_INVOCATION variable properly quoted for things @@ -96,6 +92,11 @@ class NinjaState: python_bin = ninja_syntax.escape(scons_escape(sys.executable)) self.variables = { "COPY": "cmd.exe /c 1>NUL copy" if sys.platform == "win32" else "cp", + 'PORT': scons_daemon_port, + 'NINJA_DIR_PATH': env.get('NINJA_DIR').abspath, + 'PYTHON_BIN': sys.executable, + 'NINJA_TOOL_DIR': pathlib.Path(__file__).parent, + 'NINJA_SCONS_DAEMON_KEEP_ALIVE': str(env.get('NINJA_SCONS_DAEMON_KEEP_ALIVE')), "SCONS_INVOCATION": '{} {} --disable-ninja __NINJA_NO=1 $out'.format( python_bin, " ".join( @@ -209,7 +210,7 @@ class NinjaState: "restat": 1, }, "TEMPLATE": { - "command": f"{sys.executable} {pathlib.Path(__file__).parent / 'ninja_daemon_build.py'} {scons_daemon_port} {get_path(env.get('NINJA_DIR'))} $out", + "command": "$PYTHON_BIN $NINJA_TOOL_DIR/ninja_daemon_build.py $PORT $NINJA_DIR_PATH $out", "description": "Defer to SCons to build $out", "pool": "local_pool", "restat": 1 @@ -238,7 +239,7 @@ class NinjaState: }, "SCONS_DAEMON": { - "command": f"{sys.executable} {pathlib.Path(__file__).parent / 'ninja_run_daemon.py'} {scons_daemon_port} {env.get('NINJA_DIR').abspath} {str(env.get('NINJA_SCONS_DAEMON_KEEP_ALIVE'))} $SCONS_INVOCATION", + "command": "$PYTHON_BIN $NINJA_TOOL_DIR/ninja_run_daemon.py $PORT $NINJA_DIR_PATH $NINJA_SCONS_DAEMON_KEEP_ALIVE $SCONS_INVOCATION", "description": "Starting scons daemon...", "pool": "local_pool", # restat @@ -317,7 +318,6 @@ class NinjaState: else: raise InternalError("Node {} added to ninja build state more than once".format(node_string)) self.builds[node_string] = build - self.built.update(build["outputs"]) return True # TODO: rely on SCons to tell us what is generated source @@ -361,8 +361,7 @@ class NinjaState: self.rules[rule]["depfile"] = "$out.d" else: raise Exception(f"Unknown 'NINJA_DEPFILE_PARSE_FORMAT'={self.env['NINJA_DEPFILE_PARSE_FORMAT']}, use 'mvsc', 'gcc', or 'clang'.") - - + for key, rule in self.env.get(NINJA_RULES, {}).items(): # make a non response file rule for users custom response file rules. if rule.get('rspfile') is not None: @@ -374,7 +373,6 @@ class NinjaState: else: self.rules.update({key: rule}) - self.rules.update(self.env.get(NINJA_RULES, {})) self.pools.update(self.env.get(NINJA_POOLS, {})) content = io.StringIO() @@ -451,10 +449,20 @@ class NinjaState: template_builders = [] scons_compiledb = False + if SCons.Script._Get_Default_Targets == SCons.Script._Set_Default_Targets_Has_Not_Been_Called: + all_targets = set() + else: + all_targets = None + for build in [self.builds[key] for key in sorted(self.builds.keys())]: if "compile_commands.json" in build["outputs"]: scons_compiledb = True + # this is for the no command line targets, no SCons default case. We want this default + # to just be all real files in the build. + if all_targets is not None and build['rule'] != 'phony': + all_targets = all_targets | set(build["outputs"]) + if build["rule"] == "TEMPLATE": template_builders.append(build) continue @@ -604,6 +612,22 @@ class NinjaState: ) + if all_targets is None: + # Look in SCons's list of DEFAULT_TARGETS, find the ones that + # we generated a ninja build rule for. + all_targets = [str(node) for node in NINJA_DEFAULT_TARGETS] + else: + all_targets = list(all_targets) + + if len(all_targets) == 0: + all_targets = ["phony_default"] + ninja.build( + outputs=all_targets, + rule="phony", + ) + + ninja.default([self.ninja_syntax.escape_path(path) for path in sorted(all_targets)]) + daemon_dir = pathlib.Path(tempfile.gettempdir()) / ('scons_daemon_' + str(hashlib.md5(str(get_path(self.env["NINJA_DIR"])).encode()).hexdigest())) pidfile = None if os.path.exists(scons_daemon_dirty): @@ -619,24 +643,37 @@ class NinjaState: except OSError: pass + # wait for the server process to fully killed + try: + import psutil + while True: + if pid not in [proc.pid for proc in psutil.process_iter()]: + break + else: + time.sleep(0.1) + except ImportError: + # if psutil is not installed we can do this the hard way + while True: + if sys.platform == 'win32': + import ctypes + PROCESS_QUERY_INFORMATION = 0x1000 + processHandle = ctypes.windll.kernel32.OpenProcess(PROCESS_QUERY_INFORMATION, 0,pid) + if processHandle == 0: + break + else: + ctypes.windll.kernel32.CloseHandle(processHandle) + time.sleep(0.1) + else: + try: + os.kill(pid, 0) + except OSError: + break + else: + time.sleep(0.1) + if os.path.exists(scons_daemon_dirty): os.unlink(scons_daemon_dirty) - - # Look in SCons's list of DEFAULT_TARGETS, find the ones that - # we generated a ninja build rule for. - scons_default_targets = [ - get_path(tgt) - for tgt in SCons.Script.DEFAULT_TARGETS - if get_path(tgt) in self.built - ] - - # If we found an overlap between SCons's list of default - # targets and the targets we created ninja builds for then use - # those as ninja's default as well. - if scons_default_targets: - ninja.default(" ".join(scons_default_targets)) - with NamedTemporaryFile(delete=False, mode='w') as temp_ninja_file: temp_ninja_file.write(content.getvalue()) shutil.move(temp_ninja_file.name, ninja_file_path) diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py index 526db8a..4429217 100644 --- a/SCons/Tool/ninja/__init__.py +++ b/SCons/Tool/ninja/__init__.py @@ -34,7 +34,7 @@ import SCons import SCons.Tool.ninja.Globals from SCons.Script import GetOption -from .Globals import NINJA_RULES, NINJA_POOLS, NINJA_CUSTOM_HANDLERS +from .Globals import NINJA_RULES, NINJA_POOLS, NINJA_CUSTOM_HANDLERS, NINJA_DEFAULT_TARGETS from .Methods import register_custom_handler, register_custom_rule_mapping, register_custom_rule, register_custom_pool, \ set_build_node_callback, get_generic_shell_command, CheckNinjaCompdbExpand, get_command, \ gen_get_response_file_command @@ -118,6 +118,7 @@ def ninja_builder(env, target, source): # leaving warnings and other output, seems a bit # prone to failure with such a simple check erase_previous = output.startswith('[') + sys.stdout.write("\n") def exists(env): @@ -200,7 +201,7 @@ def generate(env): ninja_file = env.Ninja() env['NINJA_FILE'] = ninja_file[0] env.AlwaysBuild(ninja_file) - env.Alias("$NINJA_ALIAS_NAME", ninja_file) + SCons.Script.BUILD_TARGETS = SCons.Script.TargetList(env.Alias("$NINJA_ALIAS_NAME", ninja_file)) else: if str(NINJA_STATE.ninja_file) != env["NINJA_FILE_NAME"]: SCons.Warnings.SConsWarning("Generating multiple ninja files not supported, set ninja file name before tool initialization.") @@ -445,6 +446,23 @@ def generate(env): # date-ness. SCons.Script.Main.BuildTask.needs_execute = lambda x: True + + def ninja_Set_Default_Targets(env, tlist): + """ + Record the default targets if they were ever set by the user. Ninja + will need to write the default targets and make sure not to include + the scons daemon shutdown target. + """ + SCons.Script._Get_Default_Targets = SCons.Script._Set_Default_Targets_Has_Been_Called + SCons.Script.DEFAULT_TARGETS = ninja_file + for t in tlist: + if isinstance(t, SCons.Node.Node): + NINJA_DEFAULT_TARGETS.append(t) + else: + nodes = env.arg2nodes(t, env.fs.Entry) + NINJA_DEFAULT_TARGETS.extend(nodes) + SCons.Script._Set_Default_Targets = ninja_Set_Default_Targets + # We will eventually need to overwrite TempFileMunge to make it # handle persistent tempfiles or get an upstreamed change to add # some configurability to it's behavior in regards to tempfiles. diff --git a/SCons/Tool/ninja/ninja_daemon_build.py b/SCons/Tool/ninja/ninja_daemon_build.py index f34935b..62749b0 100644 --- a/SCons/Tool/ninja/ninja_daemon_build.py +++ b/SCons/Tool/ninja/ninja_daemon_build.py @@ -39,6 +39,7 @@ import pathlib import tempfile import hashlib import traceback +import socket ninja_builddir = pathlib.Path(sys.argv[2]) daemon_dir = pathlib.Path(tempfile.gettempdir()) / ( @@ -65,8 +66,8 @@ while True: while not response: try: response = conn.getresponse() - except (http.client.RemoteDisconnected, http.client.ResponseNotReady): - time.sleep(0.01) + except (http.client.RemoteDisconnected, http.client.ResponseNotReady, socket.timeout): + time.sleep(0.1) except http.client.HTTPException: logging.debug(f"Error: {traceback.format_exc()}") exit(1) @@ -79,6 +80,10 @@ while True: logging.debug(f"Request Done: {sys.argv[3]}") exit(0) + except ConnectionRefusedError: + logging.debug(f"Server refused connection to build {sys.argv[3]}, maybe it was too busy, tring again: {traceback.format_exc()}") + time.sleep(0.1) + except Exception: logging.debug(f"Failed to send command: {traceback.format_exc()}") exit(1) diff --git a/SCons/Tool/ninja/ninja_scons_daemon.py b/SCons/Tool/ninja/ninja_scons_daemon.py index f900343..a50a478 100644 --- a/SCons/Tool/ninja/ninja_scons_daemon.py +++ b/SCons/Tool/ninja/ninja_scons_daemon.py @@ -50,6 +50,7 @@ from timeit import default_timer as timer import traceback import tempfile import hashlib +import signal port = int(sys.argv[1]) ninja_builddir = pathlib.Path(sys.argv[2]) @@ -134,6 +135,15 @@ building_cv = Condition() error_cv = Condition() thread_error = False +httpd = None +daemon_needs_to_shutdown = False + +def sigint_func(signum, frame): + global httpd, daemon_needs_to_shutdown + daemon_needs_to_shutdown = True + + +signal.signal(signal.SIGINT, sigint_func) def daemon_thread_func(): @@ -213,6 +223,8 @@ def daemon_thread_func(): finished_building += [building_node] daemon_ready = False + if daemon_needs_to_shutdown: + break time.sleep(0.01) except Exception: thread_error = True @@ -283,6 +295,7 @@ def server_thread_func(): def log_message(self, format, *args): return + socketserver.TCPServer.allow_reuse_address = True httpd = socketserver.TCPServer(("127.0.0.1", port), S) httpd.serve_forever() @@ -291,31 +304,23 @@ server_thread = threading.Thread(target=server_thread_func) server_thread.daemon = True server_thread.start() -while timer() - keep_alive_timer < daemon_keep_alive and not thread_error: +while timer() - keep_alive_timer < daemon_keep_alive and not thread_error and not daemon_needs_to_shutdown: time.sleep(1) if thread_error: daemon_log(f"Shutting server on port {port} down because thread error.") +elif daemon_needs_to_shutdown: + daemon_log("Server shutting down upon request.") else: daemon_log( f"Shutting server on port {port} down because timed out: {daemon_keep_alive}" ) - -# if there are errors, don't immediately shut down the daemon -# the process which started the server is attempt to connect to -# the daemon before allowing jobs to start being sent. If the daemon -# shuts down too fast, the launch script will think it has not -# started yet and sit and wait. If the launch script is able to connect -# and then the connection is dropped, it will immediately exit with fail. -time.sleep(5) - +httpd.shutdown() if os.path.exists(ninja_builddir / "scons_daemon_dirty"): os.unlink(ninja_builddir / "scons_daemon_dirty") if os.path.exists(daemon_dir / "pidfile"): os.unlink(daemon_dir / "pidfile") -httpd.shutdown() -server_thread.join() # Local Variables: diff --git a/test/ninja/default_targets.py b/test/ninja/default_targets.py new file mode 100644 index 0000000..dc7c7c1 --- /dev/null +++ b/test/ninja/default_targets.py @@ -0,0 +1,83 @@ +#!/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') + +test.file_fixture('ninja_test_sconscripts/sconstruct_default_targets', 'SConstruct') + +# generate simple build +test.run(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.must_not_exist([test.workpath('out1.txt')]) +test.must_exist([test.workpath('out2.txt')]) + +# clean build and ninja files +test.run(arguments='-c', stdout=None) +test.must_contain_all_lines(test.stdout(), [ + 'Removed out2.txt', + 'Removed build.ninja']) + +# only generate the ninja file +test.run(arguments='--disable-execute-ninja', stdout=None) +test.must_contain_all_lines(test.stdout(), ['Generating: build.ninja']) +test.must_not_exist(test.workpath('out1.txt')) +test.must_not_exist(test.workpath('out2.txt')) + +# run ninja independently +program = test.workpath('run_ninja_env.bat') if IS_WINDOWS else ninja_bin +test.run(program=program, stdout=None) +test.must_not_exist([test.workpath('out1.txt')]) +test.must_exist(test.workpath('out2.txt')) + +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/force_scons_callback.py b/test/ninja/force_scons_callback.py index c99ed58..e0da0dd 100644 --- a/test/ninja/force_scons_callback.py +++ b/test/ninja/force_scons_callback.py @@ -80,7 +80,7 @@ test.must_match("out2.txt", "test2.cpp" + os.linesep) # only generate the ninja file with specific NINJA_SCONS_DAEMON_PORT test.run(arguments="PORT=9999 --disable-execute-ninja", stdout=None) # Verify that port # propagates to call to ninja_run_daemon.py -test.must_contain(test.workpath("build.ninja"), "ninja_run_daemon.py 9999") +test.must_contain(test.workpath("build.ninja"), "PORT = 9999") test.pass_test() diff --git a/test/ninja/ninja_test_sconscripts/sconstruct_default_targets b/test/ninja/ninja_test_sconscripts/sconstruct_default_targets new file mode 100644 index 0000000..8963270 --- /dev/null +++ b/test/ninja/ninja_test_sconscripts/sconstruct_default_targets @@ -0,0 +1,14 @@ +import SCons + +SetOption('experimental','ninja') +DefaultEnvironment(tools=[]) + +env = Environment(tools=[]) + +env.Tool('ninja') + +env.Command('out1.txt', 'foo.c', 'echo test > $TARGET' ) +out2_node = env.Command('out2.txt', 'foo.c', 'echo test > $TARGET', NINJA_FORCE_SCONS_BUILD=True) +alias = env.Alias('def', out2_node) + +env.Default(alias) -- cgit v0.12