From 0ed5eea9af9358a0bfbeefa42507775947fe2d5f Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Tue, 7 Jun 2022 12:30:50 -0500 Subject: Added option to allow scons to determine if it should skip ninja regeneration. --- CHANGES.txt | 2 + RELEASE.txt | 2 + SCons/Script/SConsOptions.py | 3 +- SCons/Tool/ninja/NinjaState.py | 119 ++++++++++++++------- SCons/Tool/ninja/Utils.py | 25 ++++- SCons/Tool/ninja/__init__.py | 11 +- test/ninja/ninja_file_deterministic.py | 105 ++++++++++++++++++ .../sconstruct_ninja_determinism | 20 ++++ 8 files changed, 241 insertions(+), 46 deletions(-) create mode 100644 test/ninja/ninja_file_deterministic.py create mode 100644 test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism diff --git a/CHANGES.txt b/CHANGES.txt index 03692d6..4541168 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -116,6 +116,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER Note that these are called for every build command run by SCons. It could have considerable performance impact if not used carefully. to connect to the server during start up. + - Ninja: added option to skip ninja regeneration if scons can determine the ninja file does + not need to be regenerated. From Mats Wichmann: - Tweak the way default site_scons paths on Windows are expressed to diff --git a/RELEASE.txt b/RELEASE.txt index cfa8afc..0dae549 100755 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -88,6 +88,8 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY require delayed expansion to be enabled which is currently not supported and is typically not enabled by default on the host system. The batch files may also require environment variables that are not included by default in the msvc environment. +- Ninja: added option to skip ninja regeneration if scons can determine the ninja file does + not need to be regenerated. FIXES ----- diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py index e2631fb..0210b79 100644 --- a/SCons/Script/SConsOptions.py +++ b/SCons/Script/SConsOptions.py @@ -151,7 +151,8 @@ class SConsValues(optparse.Values): # Requested setable flag in : https://github.com/SCons/scons/issues/3983 # From experimental ninja 'disable_execute_ninja', - 'disable_ninja' + 'disable_ninja', + 'skip_ninja_regen' ] def set_option(self, name, value): diff --git a/SCons/Tool/ninja/NinjaState.py b/SCons/Tool/ninja/NinjaState.py index c168c7f..c75c966 100644 --- a/SCons/Tool/ninja/NinjaState.py +++ b/SCons/Tool/ninja/NinjaState.py @@ -29,6 +29,8 @@ import signal import tempfile import shutil import sys +import random +import filecmp from os.path import splitext from tempfile import NamedTemporaryFile import ninja @@ -42,7 +44,7 @@ from .Globals import COMMAND_TYPES, NINJA_RULES, NINJA_POOLS, \ 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 + get_outputs, get_inputs, get_dependencies, get_rule, get_command_env, to_escaped_list, ninja_sorted_build from .Methods import get_command @@ -82,7 +84,26 @@ class NinjaState: # to make the SCONS_INVOCATION variable properly quoted for things # like CCFLAGS scons_escape = env.get("ESCAPE", lambda x: x) - scons_daemon_port = int(env.get('NINJA_SCONS_DAEMON_PORT',-1)) + + # The daemon port should be the same across runs, unless explicitly set + # or if the portfile is deleted. This ensures the ninja file is deterministic + # across regen's if nothings changed. The construction var should take preference, + # then portfile is next, and then otherwise create a new random port to persist in + # use. + scons_daemon_port = None + os.makedirs(get_path(self.env.get("NINJA_DIR")), exist_ok=True) + scons_daemon_port_file = str(pathlib.Path(get_path(self.env.get("NINJA_DIR"))) / "scons_daemon_portfile") + + if env.get('NINJA_SCONS_DAEMON_PORT') is not None: + scons_daemon_port = int(env.get('NINJA_SCONS_DAEMON_PORT')) + elif os.path.exists(scons_daemon_port_file): + with open(scons_daemon_port_file) as f: + scons_daemon_port = int(f.read()) + else: + scons_daemon_port = random.randint(10000, 60000) + + with open(scons_daemon_port_file, 'w') as f: + f.write(str(scons_daemon_port)) # 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 @@ -381,13 +402,13 @@ class NinjaState: ninja.variable("builddir", get_path(self.env.Dir(self.env['NINJA_DIR']).path)) - for pool_name, size in self.pools.items(): + for pool_name, size in sorted(self.pools.items()): ninja.pool(pool_name, min(self.env.get('NINJA_MAX_JOBS', size), size)) - for var, val in self.variables.items(): + for var, val in sorted(self.variables.items()): ninja.variable(var, val) - for rule, kwargs in self.rules.items(): + for rule, kwargs in sorted(self.rules.items()): if self.env.get('NINJA_MAX_JOBS') is not None and 'pool' not in kwargs: kwargs['pool'] = 'local_pool' ninja.rule(rule, **kwargs) @@ -529,8 +550,9 @@ class NinjaState: ) if remaining_outputs: - ninja.build( - outputs=sorted(remaining_outputs), rule="phony", implicit=first_output, + ninja_sorted_build( + ninja, + outputs=remaining_outputs, rule="phony", implicit=first_output, ) build["outputs"] = first_output @@ -548,12 +570,18 @@ class NinjaState: if "inputs" in build: build["inputs"].sort() - ninja.build(**build) + ninja_sorted_build( + ninja, + **build + ) scons_daemon_dirty = str(pathlib.Path(get_path(self.env.get("NINJA_DIR"))) / "scons_daemon_dirty") for template_builder in template_builders: template_builder["implicit"] += [scons_daemon_dirty] - ninja.build(**template_builder) + ninja_sorted_build( + ninja, + **template_builder + ) # We have to glob the SCons files here to teach the ninja file # how to regenerate itself. We'll never see ourselves in the @@ -563,17 +591,19 @@ class NinjaState: ninja_file_path = self.env.File(self.ninja_file).path regenerate_deps = to_escaped_list(self.env, self.env['NINJA_REGENERATE_DEPS']) - ninja.build( - ninja_file_path, + ninja_sorted_build( + ninja, + outputs=ninja_file_path, rule="REGENERATE", implicit=regenerate_deps, variables={ - "self": ninja_file_path, + "self": ninja_file_path } ) - ninja.build( - regenerate_deps, + ninja_sorted_build( + ninja, + outputs=regenerate_deps, rule="phony", variables={ "self": ninja_file_path, @@ -584,8 +614,9 @@ class NinjaState: # If we ever change the name/s of the rules that include # compile commands (i.e. something like CC) we will need to # update this build to reflect that complete list. - ninja.build( - "compile_commands.json", + ninja_sorted_build( + ninja, + outputs="compile_commands.json", rule="CMD", pool="console", implicit=[str(self.ninja_file)], @@ -601,12 +632,14 @@ class NinjaState: }, ) - ninja.build( - "compiledb", rule="phony", implicit=["compile_commands.json"], + ninja_sorted_build( + ninja, + outputs="compiledb", rule="phony", implicit=["compile_commands.json"], ) - ninja.build( - ["run_ninja_scons_daemon_phony", scons_daemon_dirty], + ninja_sorted_build( + ninja, + outputs=["run_ninja_scons_daemon_phony", scons_daemon_dirty], rule="SCONS_DAEMON", ) @@ -620,39 +653,45 @@ class NinjaState: if len(all_targets) == 0: all_targets = ["phony_default"] - ninja.build( + ninja_sorted_build( + ninja, 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): - pidfile = scons_daemon_dirty - elif os.path.exists(daemon_dir / 'pidfile'): - pidfile = daemon_dir / 'pidfile' - - if pidfile: - with open(pidfile) as f: - pid = int(f.readline()) - try: - os.kill(pid, signal.SIGINT) - except OSError: - pass + with NamedTemporaryFile(delete=False, mode='w') as temp_ninja_file: + temp_ninja_file.write(content.getvalue()) + + if self.env.GetOption('skip_ninja_regen') and os.path.exists(ninja_file_path) and filecmp.cmp(temp_ninja_file.name, ninja_file_path): + os.unlink(temp_ninja_file.name) + else: + + 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): + pidfile = scons_daemon_dirty + elif os.path.exists(daemon_dir / 'pidfile'): + pidfile = daemon_dir / 'pidfile' + + if pidfile: + with open(pidfile) as f: + pid = int(f.readline()) + try: + os.kill(pid, signal.SIGINT) + except OSError: + pass # wait for the server process to fully killed # TODO: update wait_for_process_to_die() to handle timeout and then catch exception # here and do something smart. wait_for_process_to_die(pid) - if os.path.exists(scons_daemon_dirty): - os.unlink(scons_daemon_dirty) + if os.path.exists(scons_daemon_dirty): + os.unlink(scons_daemon_dirty) - 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) + shutil.move(temp_ninja_file.name, ninja_file_path) self.__generated = True diff --git a/SCons/Tool/ninja/Utils.py b/SCons/Tool/ninja/Utils.py index b2c3cd3..dec6d2c 100644 --- a/SCons/Tool/ninja/Utils.py +++ b/SCons/Tool/ninja/Utils.py @@ -23,6 +23,7 @@ import os import shutil from os.path import join as joinpath +from collections import OrderedDict import SCons from SCons.Action import get_default_ENV, _string_from_cmd_list @@ -52,6 +53,15 @@ def ninja_add_command_line_options(): help='Disable ninja generation and build with scons even if tool is loaded. '+ 'Also used by ninja to build targets which only scons can build.') + AddOption('--skip-ninja-regen', + dest='skip_ninja_regen', + metavar='BOOL', + action="store_true", + default=False, + help='Allow scons to skip regeneration of the ninja file and restarting of the daemon. ' + + 'Care should be taken in cases where Glob is in use or generated files are used in ' + + 'command lines.') + def is_valid_dependent_node(node): """ @@ -126,7 +136,7 @@ def get_dependencies(node, skip_sources=False): get_path(src_file(child)) for child in filter_ninja_nodes(node.children()) if child not in node.sources - ] + ] return [get_path(src_file(child)) for child in filter_ninja_nodes(node.children())] @@ -261,6 +271,19 @@ def ninja_noop(*_args, **_kwargs): """ return None +def ninja_recursive_sorted_dict(build): + sorted_dict = OrderedDict() + for key, val in sorted(build.items()): + if isinstance(val, dict): + sorted_dict[key] = ninja_recursive_sorted_dict(val) + else: + sorted_dict[key] = val + return sorted_dict + + +def ninja_sorted_build(ninja, **build): + sorted_dict = ninja_recursive_sorted_dict(build) + ninja.build(**sorted_dict) def get_command_env(env, target, source): """ diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py index 04a7abb..ad75978 100644 --- a/SCons/Tool/ninja/__init__.py +++ b/SCons/Tool/ninja/__init__.py @@ -26,7 +26,7 @@ import importlib import os -import random +import traceback import subprocess import sys @@ -66,8 +66,12 @@ def ninja_builder(env, target, source): print("Generating:", str(target[0])) generated_build_ninja = target[0].get_abspath() - NINJA_STATE.generate() - + try: + NINJA_STATE.generate() + except Exception as e: + raise SCons.Errors.BuildError( + errstr=f"ERROR: an excetion occurred while generating the ninja file:\n{traceback.format_exc()}", + node=target) if env["PLATFORM"] == "win32": # TODO: Is this necessary as you set env variable in the ninja build file per target? # this is not great, its doesn't consider specific @@ -194,7 +198,6 @@ def generate(env): env["NINJA_ALIAS_NAME"] = env.get("NINJA_ALIAS_NAME", "generate-ninja") env['NINJA_DIR'] = env.Dir(env.get("NINJA_DIR", '#/.ninja')) env["NINJA_SCONS_DAEMON_KEEP_ALIVE"] = env.get("NINJA_SCONS_DAEMON_KEEP_ALIVE", 180000) - env["NINJA_SCONS_DAEMON_PORT"] = env.get('NINJA_SCONS_DAEMON_PORT', random.randint(10000, 60000)) if GetOption("disable_ninja"): env.SConsignFile(os.path.join(str(env['NINJA_DIR']),'.ninja.sconsign')) diff --git a/test/ninja/ninja_file_deterministic.py b/test/ninja/ninja_file_deterministic.py new file mode 100644 index 0000000..2ac5e1a --- /dev/null +++ b/test/ninja/ninja_file_deterministic.py @@ -0,0 +1,105 @@ +#!/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 shutil +import filecmp + +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.BIN_DIR, + 'ninja' + _exe)) + +test.dir_fixture('ninja-fixture') + +test.file_fixture('ninja_test_sconscripts/sconstruct_ninja_determinism', '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_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) +shutil.copyfile(test.workpath('build.ninja'), test.workpath('build.ninja.orig')) + +# generate same build again +test.run(stdout=None) +test.must_contain_all_lines(test.stdout(), ['Generating: build.ninja', 'ninja: no work to do.']) +test.must_contain_all(test.stdout(), 'Executing:') +test.must_contain_all(test.stdout(), 'ninja%(_exe)s -f' % locals()) +test.must_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) + +# make sure the ninja file was deterministic +if not filecmp.cmp(test.workpath('build.ninja'), test.workpath('build.ninja.orig')): + test.fail_test() + +# clean build and ninja files +os.unlink(test.workpath('build.ninja.orig')) +test.run(arguments='-c', stdout=None) + +# 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.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_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) +shutil.copyfile(test.workpath('build.ninja'), test.workpath('build.ninja.orig')) + +# only generate the ninja file again +test.run(arguments='--disable-execute-ninja', stdout=None) +test.must_contain_all_lines(test.stdout(), ['Generating: build.ninja']) +test.must_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) + +# run ninja independently again +program = test.workpath('run_ninja_env.bat') if IS_WINDOWS else ninja_bin +test.run(program=program, stdout=None) +test.must_contain_all_lines(test.stdout(), ['ninja: no work to do.']) +test.must_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) + +# make sure the ninja file was deterministic +if not filecmp.cmp(test.workpath('build.ninja'), test.workpath('build.ninja.orig')): + test.fail_test() + +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_determinism b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism new file mode 100644 index 0000000..6bd7c26 --- /dev/null +++ b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism @@ -0,0 +1,20 @@ +import SCons +import random +SetOption('experimental','ninja') +SetOption('skip_ninja_regen', True) +DefaultEnvironment(tools=[]) + +env = Environment(tools=[]) + +env.Tool('ninja') + +# make the dependency list vary in order. Ninja tool should sort them to be deterministic. +for i in range(1, 10): + node = env.Command(f'out{i}.txt', 'foo.c', 'echo test > $TARGET') + deps = list(range(1, i)) + random.shuffle(deps) + for j in deps: + if j == i: + continue + env.Depends(node, f'out{j}.txt') + -- cgit v0.12 From 60b6724b28a909f63fa99f970bc39111f039024d Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Tue, 7 Jun 2022 12:35:08 -0500 Subject: improve help text slightly --- SCons/Tool/ninja/Utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SCons/Tool/ninja/Utils.py b/SCons/Tool/ninja/Utils.py index dec6d2c..583301e 100644 --- a/SCons/Tool/ninja/Utils.py +++ b/SCons/Tool/ninja/Utils.py @@ -59,7 +59,7 @@ def ninja_add_command_line_options(): action="store_true", default=False, help='Allow scons to skip regeneration of the ninja file and restarting of the daemon. ' + - 'Care should be taken in cases where Glob is in use or generated files are used in ' + + 'Care should be taken in cases where Glob is in use or SCons generated files are used in ' + 'command lines.') -- cgit v0.12 From b1ede4ce01b6163086c748f509c51616b9dec051 Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Tue, 7 Jun 2022 14:28:15 -0500 Subject: fixed sider complaint --- SCons/Tool/ninja/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py index ad75978..7c32676 100644 --- a/SCons/Tool/ninja/__init__.py +++ b/SCons/Tool/ninja/__init__.py @@ -68,7 +68,7 @@ def ninja_builder(env, target, source): generated_build_ninja = target[0].get_abspath() try: NINJA_STATE.generate() - except Exception as e: + except Exception: raise SCons.Errors.BuildError( errstr=f"ERROR: an excetion occurred while generating the ninja file:\n{traceback.format_exc()}", node=target) -- cgit v0.12 From 905fa507cafdb05e66d7e8f60a43648b8753cae7 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Tue, 14 Jun 2022 15:38:20 -0700 Subject: Fix typo --- SCons/Tool/ninja/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py index 61f98f5..9eaeacb 100644 --- a/SCons/Tool/ninja/__init__.py +++ b/SCons/Tool/ninja/__init__.py @@ -70,8 +70,9 @@ def ninja_builder(env, target, source): NINJA_STATE.generate() except Exception: raise SCons.Errors.BuildError( - errstr=f"ERROR: an excetion occurred while generating the ninja file:\n{traceback.format_exc()}", + errstr=f"ERROR: an exception occurred while generating the ninja file:\n{traceback.format_exc()}", node=target) + if env["PLATFORM"] == "win32": # TODO: Is this necessary as you set env variable in the ninja build file per target? # this is not great, its doesn't consider specific -- cgit v0.12 From 4692902f97761a349fd0ea119d4c1169cf5919e5 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Tue, 14 Jun 2022 15:56:50 -0700 Subject: Fixed typo --- SCons/Script/SConsOptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py index 0210b79..0dff6be 100644 --- a/SCons/Script/SConsOptions.py +++ b/SCons/Script/SConsOptions.py @@ -147,8 +147,8 @@ class SConsValues(optparse.Values): 'warn', # TODO: Remove these once we update the AddOption() API to allow setting - # added flag as setable. - # Requested setable flag in : https://github.com/SCons/scons/issues/3983 + # added flag as settable. + # Requested settable flag in : https://github.com/SCons/scons/issues/3983 # From experimental ninja 'disable_execute_ninja', 'disable_ninja', -- cgit v0.12 From dfc8eacbc4e0157aa235b7f6bacf1d956f8ba673 Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Tue, 14 Jun 2022 23:13:12 -0500 Subject: update changes/release notes, and add mtime check to determinism test --- CHANGES.txt | 6 ++++-- RELEASE.txt | 7 ++++--- test/ninja/ninja_file_deterministic.py | 5 +++++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 226102f..28785e1 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -135,8 +135,10 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER Note that these are called for every build command run by SCons. It could have considerable performance impact if not used carefully. to connect to the server during start up. - - Ninja: added option to skip ninja regeneration if scons can determine the ninja file does - not need to be regenerated. + - Ninja: added option "--skip-ninja-regen" to enable skipping regeneration of the ninja file + if scons can determine the ninja file doesnot need to be regenerated, which will also + skip restarting the scons daemon. Note this option is could result in incorrect rebuilds + if scons Glob or scons generated files are used in ninja build target's command lines. - Ninja: Added new alias "shutdown-ninja-scons-daemon" to allow ninja to shutdown the daemon. Also added cleanup to test framework to kill ninja scons daemons and clean ip daemon logs. NOTE: Test for this requires python psutil module. It will be skipped if not present. diff --git a/RELEASE.txt b/RELEASE.txt index 6e0d30d..abecf39 100755 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -107,9 +107,10 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY require delayed expansion to be enabled which is currently not supported and is typically not enabled by default on the host system. The batch files may also require environment variables that are not included by default in the msvc environment. -- Ninja: added option to skip ninja regeneration if scons can determine the ninja file does - not need to be regenerated. - +- Ninja: added option "--skip-ninja-regen" to enable skipping regeneration of the ninja file + if scons can determine the ninja file doesnot need to be regenerated, which will also + skip restarting the scons daemon. Note this option is could result in incorrect rebuilds + if scons Glob or scons generated files are used in ninja build target's command lines. FIXES ----- diff --git a/test/ninja/ninja_file_deterministic.py b/test/ninja/ninja_file_deterministic.py index 2ac5e1a..ef1abdc 100644 --- a/test/ninja/ninja_file_deterministic.py +++ b/test/ninja/ninja_file_deterministic.py @@ -55,6 +55,8 @@ test.must_contain_all(test.stdout(), 'ninja%(_exe)s -f' % locals()) test.must_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) shutil.copyfile(test.workpath('build.ninja'), test.workpath('build.ninja.orig')) +ninja_file_mtime = os.path.getmtime(test.workpath('build.ninja')) + # generate same build again test.run(stdout=None) test.must_contain_all_lines(test.stdout(), ['Generating: build.ninja', 'ninja: no work to do.']) @@ -62,6 +64,9 @@ test.must_contain_all(test.stdout(), 'Executing:') test.must_contain_all(test.stdout(), 'ninja%(_exe)s -f' % locals()) test.must_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) +if os.path.getmtime(test.workpath('build.ninja')) != ninja_file_mtime: + test.fail_test() + # make sure the ninja file was deterministic if not filecmp.cmp(test.workpath('build.ninja'), test.workpath('build.ninja.orig')): test.fail_test() -- cgit v0.12 From ebddf72883feda3d7e70b2abcba0a44f284a7dd1 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Wed, 15 Jun 2022 10:34:28 -0700 Subject: added message to test failure, and removed unneeded import SCons in fixture sconstruct --- SCons/Tool/ninja/__init__.py | 2 +- test/ninja/ninja_file_deterministic.py | 2 +- test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py index 9eaeacb..f0bdee5 100644 --- a/SCons/Tool/ninja/__init__.py +++ b/SCons/Tool/ninja/__init__.py @@ -72,7 +72,7 @@ def ninja_builder(env, target, source): raise SCons.Errors.BuildError( errstr=f"ERROR: an exception occurred while generating the ninja file:\n{traceback.format_exc()}", node=target) - + if env["PLATFORM"] == "win32": # TODO: Is this necessary as you set env variable in the ninja build file per target? # this is not great, its doesn't consider specific diff --git a/test/ninja/ninja_file_deterministic.py b/test/ninja/ninja_file_deterministic.py index ef1abdc..9832f22 100644 --- a/test/ninja/ninja_file_deterministic.py +++ b/test/ninja/ninja_file_deterministic.py @@ -65,7 +65,7 @@ test.must_contain_all(test.stdout(), 'ninja%(_exe)s -f' % locals()) test.must_exist([test.workpath('out1.txt'), test.workpath('out2.txt')]) if os.path.getmtime(test.workpath('build.ninja')) != ninja_file_mtime: - test.fail_test() + test.fail_test(message="build.ninja file has been updated (mtime changed) and should not have been") # make sure the ninja file was deterministic if not filecmp.cmp(test.workpath('build.ninja'), test.workpath('build.ninja.orig')): diff --git a/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism index 6bd7c26..3d3eecb 100644 --- a/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism +++ b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism @@ -1,6 +1,6 @@ -import SCons import random -SetOption('experimental','ninja') + +SetOption('experimental', 'ninja') SetOption('skip_ninja_regen', True) DefaultEnvironment(tools=[]) @@ -17,4 +17,3 @@ for i in range(1, 10): if j == i: continue env.Depends(node, f'out{j}.txt') - -- cgit v0.12