From 4fea122590f994aa579167058f000c2dd6465b85 Mon Sep 17 00:00:00 2001 From: Dirk Baechle Date: Fri, 14 Feb 2014 21:42:39 +0100 Subject: - fix for spurious rebuilds, allow caching of the changed() method's value only when called from File.release_target_info() --- src/engine/SCons/Node/FS.py | 9 +++-- src/engine/SCons/Node/__init__.py | 11 +++++- test/Depends/spurious-rebuilds.py | 72 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 test/Depends/spurious-rebuilds.py diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py index aaa5b47..dc8affe 100644 --- a/src/engine/SCons/Node/FS.py +++ b/src/engine/SCons/Node/FS.py @@ -2779,7 +2779,7 @@ class File(Base): if not hasattr(self.attributes, 'keep_targetinfo'): # Cache some required values, before releasing # stuff like env, executor and builder... - self.changed() + self.changed(allowcache=True) self.get_contents_sig() self.get_build_env() # Now purge unneeded stuff to free memory... @@ -3034,7 +3034,7 @@ class File(Base): self.scanner_paths = None - def changed(self, node=None): + def changed(self, node=None, allowcache=False): """ Returns if the node is up-to-date with respect to the BuildInfo stored last time it was built. @@ -3042,6 +3042,8 @@ class File(Base): For File nodes this is basically a wrapper around Node.changed(), but we allow the return value to get cached after the reference to the Executor got released in release_target_info(). + + @see: Node.changed() """ if node is None: try: @@ -3050,7 +3052,8 @@ class File(Base): pass has_changed = SCons.Node.Node.changed(self, node) - self._memo['changed'] = has_changed + if allowcache: + self._memo['changed'] = has_changed return has_changed def changed_content(self, target, prev_ni): diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py index d6dbf2e..1f62971 100644 --- a/src/engine/SCons/Node/__init__.py +++ b/src/engine/SCons/Node/__init__.py @@ -1049,7 +1049,7 @@ class Node(object): def Decider(self, function): SCons.Util.AddMethod(self, function, 'changed_since_last_build') - def changed(self, node=None): + def changed(self, node=None, allowcache=False): """ Returns if the node is up-to-date with respect to the BuildInfo stored last time it was built. The default behavior is to compare @@ -1062,6 +1062,15 @@ class Node(object): any difference, but we now rely on checking every dependency to make sure that any necessary Node information (for example, the content signature of an #included .h file) is updated. + + The allowcache option was added for supporting the early + release of the executor/builder structures, right after + a File target was built. When set to true, the return + value of this changed method gets cached for File nodes. + Like this, the executor isn't needed any longer for subsequent + calls to changed(). + + @see: FS.File.changed(), FS.File.release_target_info() """ t = 0 if t: Trace('changed(%s [%s], %s)' % (self, classname(self), node)) diff --git a/test/Depends/spurious-rebuilds.py b/test/Depends/spurious-rebuilds.py new file mode 100644 index 0000000..6afc829 --- /dev/null +++ b/test/Depends/spurious-rebuilds.py @@ -0,0 +1,72 @@ +#!/usr/bin/env python +# +# __COPYRIGHT__ +# +# 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. +# + +__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" + +""" +After adding some code for reducing the overall memory consumption in +revision b4bc497, a number of spurious rebuilds was observed by different +people. The problem was, that the value of the Node.changed() method got cached +too early for File nodes. + +This test verifies that the changed() function works properly, especially +in connection with auto-generated sources, combined with an explicit Depends(). +""" + +import TestSCons + +test = TestSCons.TestSCons() + +test.write('SConstruct', """\ +# This tests the too-many-rebuilds problem with SCons 2.3.1 (test) +# Run like this: scons all-defuns.obj + +# Test setup (only runs once) +import os.path +if not os.path.exists('mkl'): + os.mkdir('mkl') +if not os.path.exists('test.c'): + open('test.c', 'w').write('int i;') + +env=Environment() +env.SharedObject('all-defuns.obj', 'all-defuns.c') +results = env.Command('all-defuns.c', 'test.c', Copy('$TARGET', '$SOURCE')) +env.Depends(results, '#mkl') +""") + +test.run(arguments = 'all-defuns.obj') + +test.must_exist('all-defuns.c') +test.must_exist('test.c') +test.must_exist('all-defuns.obj') + +test.up_to_date(arguments = 'all-defuns.obj') + +test.pass_test() + +# Local Variables: +# tab-width:4 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=4 shiftwidth=4: -- cgit v0.12