Merge pull request #4161 from dmoody256/fix_configure_marking_up_to_date

Non-conftest nodes involved in configure checks now get node info cleared after check.

12 files changed, 129 insertions, 8 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 93be68dfd..7221d20c4 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -54,6 +54,12 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       for a specified builder from the CacheDir, fails to do so, and then runs into an error
       when deleting the files which were retrieved. Previously if this happened there was no
       errors or warnings.
+    - Fix issue #2757, where Configure checks that perform a check which reads a modified source
+      (including program, source or header file(s)) would incorrectly mark that file "up to date" so the
+      actual build would not see the file as modified. Leading to incorrect incremental builds.
+      Now configure checks now clear node info for non conftest nodes, so they will be re-evaluated for
+      the real taskmaster run when the build commences.
+
 
   From Andrew Morrow
     - Avoid returning UniqueList for `children` and other `Executor` APIs. This type
diff --git a/RELEASE.txt b/RELEASE.txt
index 01bf63b12..f182c8635 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -83,6 +83,11 @@ FIXES
   SHF90, SHF03, SHF09) is now honored; previously the tool overwrote the
   settings to 'gfortran', which made it difficult reference a cross-compile
   version for dialects.
+- Fix issue #2757, where Configure checks that perform a check which reads a modified source
+  (including program, source or header file(s)) would incorrectly mark that file "up to date" so the
+  actual build would not see the file as modified. Leading to incorrect incremental builds.
+  Now configure checks now clear node info for non conftest nodes, so they will be re-evaluated for
+  the real taskmaster run when the build commences.
 
 
 IMPROVEMENTS
diff --git a/SCons/SConf.py b/SCons/SConf.py
index 051c14ce6..633c41178 100644
--- a/SCons/SConf.py
+++ b/SCons/SConf.py
@@ -227,6 +227,8 @@ class SConfBuildTask(SCons.Taskmaster.AlwaysTask):
     This is almost the same as SCons.Script.BuildTask. Handles SConfErrors
     correctly and knows about the current cache_mode.
     """
+    non_sconf_nodes = set()
+
     def display(self, message):
         if sconf_global.logstream:
             sconf_global.logstream.write("scons: Configure: " + message + "\n")
@@ -376,6 +378,25 @@ class SConfBuildTask(SCons.Taskmaster.AlwaysTask):
                     sconsign.set_entry(t.name, sconsign_entry)
                     sconsign.merge()
 
+    def make_ready_current(self):
+        # We're overriding make_ready_current() call to add to the list
+        # of nodes used by this task, filtering out any nodes created
+        # by the checker for it's own purpose.
+        self.non_sconf_nodes.update([t for t in self.targets if not t.is_conftest()])
+        super().make_ready_current()
+    make_ready = make_ready_current
+
+    def postprocess(self):
+        # We're done executing this task, so now we'll go through all the
+        # nodes used by this task which aren't nodes created for
+        # Configure checkers, but rather are existing or built files
+        # and reset their node info.
+        # If we do not reset their node info, any changes in these
+        # nodes will not trigger builds in the normal build process
+        for node in self.non_sconf_nodes:
+            node.ninfo = node.new_ninfo()
+        super().postprocess()
+
 class SConfBase:
     """This is simply a class to represent a configure context. After
     creating a SConf object, you can call any tests. After finished with your
diff --git a/SCons/SConfTests.py b/SCons/SConfTests.py
index 172fba75c..033a4ffd2 100644
--- a/SCons/SConfTests.py
+++ b/SCons/SConfTests.py
@@ -214,6 +214,8 @@ class SConfTestCase(unittest.TestCase):
                         pass
                     def get_stored_info(self):
                         pass
+                    def is_conftest(self):
+                        return True
                     def get_executor(self):
                         class Executor:
                             def __init__(self, targets):
diff --git a/test/Configure/conftest_source_file.py b/test/Configure/conftest_source_file.py
new file mode 100644
index 000000000..879c43656
--- /dev/null
+++ b/test/Configure/conftest_source_file.py
@@ -0,0 +1,69 @@
+#!/usr/bin/env python
+#
+# 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
+# "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.
+
+"""
+Template for end-to-end test file.
+Replace this with a description of the test.
+"""
+
+import textwrap
+import os 
+
+import TestSCons
+
+test = TestSCons.TestSCons()
+
+test.dir_fixture("conftest_source_file")
+
+test.run(arguments='.')
+
+conf_text = textwrap.dedent("""\
+    Checking for C header file header1.h... {arg1}yes
+    Checking for C header file header3.h... {arg2}yes
+""")
+
+test.up_to_date(read_str=conf_text.format(arg1='(cached) ', arg2='(cached) '))
+
+test.write('header2.h', """
+#pragma once
+int test_header = 2;
+""")
+
+test.not_up_to_date(read_str=conf_text.format(arg1='(cached) ', arg2='(cached) '))
+
+test.up_to_date(read_str=conf_text.format(arg1='', arg2='(cached) '))
+os.environ['SCONSFLAGS'] = '--config=force'
+test.up_to_date(read_str=conf_text.format(arg1='', arg2=''))
+os.environ['SCONSFLAGS'] = ''
+
+test.up_to_date(read_str=conf_text.format(arg1='(cached) ', arg2='(cached) '))
+
+test.pass_test()
+
+# Local Variables:
+# tab-width:4
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=4 shiftwidth=4:
diff --git a/test/Configure/conftest_source_file/SConstruct b/test/Configure/conftest_source_file/SConstruct
new file mode 100644
index 000000000..dd8d28e3c
--- /dev/null
+++ b/test/Configure/conftest_source_file/SConstruct
@@ -0,0 +1,8 @@
+DefaultEnvironment(tools=[])
+env = Environment()
+env.Append(CPPPATH=['.'])
+conf1 = Configure(env)
+conf1.CheckHeader("header1.h")
+conf1.CheckHeader("header3.h")
+conf1.Finish()
+env.Program('out', 'main.c')
\ No newline at end of file
diff --git a/test/Configure/conftest_source_file/header1.h b/test/Configure/conftest_source_file/header1.h
new file mode 100644
index 000000000..85dcd6890
--- /dev/null
+++ b/test/Configure/conftest_source_file/header1.h
@@ -0,0 +1,2 @@
+#pragma once
+#include "header2.h"
\ No newline at end of file
diff --git a/test/Configure/conftest_source_file/header2.h b/test/Configure/conftest_source_file/header2.h
new file mode 100644
index 000000000..2cf8e9057
--- /dev/null
+++ b/test/Configure/conftest_source_file/header2.h
@@ -0,0 +1,2 @@
+#pragma once
+int test_header = 1;
\ No newline at end of file
diff --git a/test/Configure/conftest_source_file/header3.h b/test/Configure/conftest_source_file/header3.h
new file mode 100644
index 000000000..dc4359efd
--- /dev/null
+++ b/test/Configure/conftest_source_file/header3.h
@@ -0,0 +1,2 @@
+#pragma once
+int test_header = 3;
\ No newline at end of file
diff --git a/test/Configure/conftest_source_file/main.c b/test/Configure/conftest_source_file/main.c
new file mode 100644
index 000000000..a9f95703c
--- /dev/null
+++ b/test/Configure/conftest_source_file/main.c
@@ -0,0 +1,2 @@
+#include "header1.h"
+int main(){return 0;}
\ No newline at end of file
diff --git a/test/sconsign/script/Configure.py b/test/sconsign/script/Configure.py
index 02a2c2044..2e1e9c145 100644
--- a/test/sconsign/script/Configure.py
+++ b/test/sconsign/script/Configure.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 that we can print .sconsign files with Configure context
@@ -58,6 +57,9 @@ CC_file = re.escape(CC_file)
 
 test.write('SConstruct', """
 import os
+
+DefaultEnvironment(tools=[])
+
 env = Environment(ENV={'PATH' : os.environ.get('PATH','')})
 conf = Configure(env)
 r1 = conf.CheckCHeader( 'math.h' )
@@ -84,10 +86,10 @@ conftest_%(sig_re)s_0.c:
         %(sig_re)s \[.*\]
 conftest_%(sig_re)s_0_%(sig_re)s%(_obj)s:
         %(_sconf_temp_conftest_0_c)s: %(sig_re)s \d+ \d+
-        %(CC)s: %(sig_re)s \d+ \d+
+        %(CC)s: %(sig_re)s None None
         %(sig_re)s \[.*\]
 === %(CC_dir)s:
-%(CC_file)s: %(sig_re)s \d+ \d+
+%(CC_file)s: None None None
 """ % locals()
 
 # grab .sconsign or .sconsign_<hashname>
diff --git a/testing/framework/TestSCons.py b/testing/framework/TestSCons.py
index dae6b0e2e..469f6c1c5 100644
--- a/testing/framework/TestSCons.py
+++ b/testing/framework/TestSCons.py
@@ -498,7 +498,7 @@ class TestSCons(TestCommon):
         kw['match'] = self.match_re_dotall
         self.run(**kw)
 
-    def not_up_to_date(self, arguments='.', **kw):
+    def not_up_to_date(self, arguments='.', read_str="", **kw):
         """Asserts that none of the targets listed in arguments is
         up to date, but does not make any assumptions on other targets.
         This function is most useful in conjunction with the -n option.
@@ -508,7 +508,7 @@ class TestSCons(TestCommon):
             s = f"{s}(?!scons: `{re.escape(arg)}' is up to date.)"
         s = f"({s}[^\n]*\n)*"
         kw['arguments'] = arguments
-        stdout = re.escape(self.wrap_stdout(build_str='ARGUMENTSGOHERE'))
+        stdout = re.escape(self.wrap_stdout(read_str=read_str, build_str='ARGUMENTSGOHERE'))
         kw['stdout'] = stdout.replace('ARGUMENTSGOHERE', s)
         kw['match'] = self.match_re_dotall
         self.run(**kw)