diff --git a/CHANGES.txt b/CHANGES.txt index 77fb110d47..00825ae8e4 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -35,6 +35,11 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER User-facing behavior does not change with this fix (GH Issue #3726). - Fix occasional test failures caused by not being able to find a file or directory fixture when running multiple tests with multiple jobs. + - Fix MSVC build failures when compiling multiple files from the same directory that + all use `#import` to import the same typelibs (GH Issue #2161). To fix this, SCons now + supports a new parameter to the `SideEffect` function: `temporary`. This parameter + should only be set to `True` in special cases where a build tool performs a one-time + one-time generation of a side effect and then leaves it in place for the duration of the build. From Joachim Kuebart: - Suppress missing SConscript deprecation warning if `must_exist=False` diff --git a/SCons/Environment.py b/SCons/Environment.py index 4ebd5156b1..5e1eaaf1c6 100644 --- a/SCons/Environment.py +++ b/SCons/Environment.py @@ -2243,7 +2243,7 @@ def SConsignFile(self, name=".sconsign", dbm_module=None): self.Execute(SCons.Defaults.Mkdir(sconsign_dir)) SCons.SConsign.File(name, dbm_module) - def SideEffect(self, side_effect, target): + def SideEffect(self, side_effect, target, temporary=False): """Tell scons that side_effects are built as side effects of building targets.""" side_effects = self.arg2nodes(side_effect, self.fs.Entry) @@ -2252,8 +2252,16 @@ def SideEffect(self, side_effect, target): for side_effect in side_effects: if side_effect.multiple_side_effect_has_builder(): raise UserError("Multiple ways to build the same target were specified for: %s" % str(side_effect)) + + if temporary and side_effect.is_up_to_date(): + # Only add a temporary side effect if it is not already + # up-to-date. Otherwise, we can consider the temporary + # side effect to not be necessary. + continue + side_effect.add_source(targets) side_effect.side_effect = 1 + side_effect.side_effect_temporary = temporary and 1 or 0 self.Precious(side_effect) for target in targets: target.side_effects.append(side_effect) diff --git a/SCons/Environment.xml b/SCons/Environment.xml index a1fd8ece7e..71fec8ab69 100644 --- a/SCons/Environment.xml +++ b/SCons/Environment.xml @@ -2839,7 +2839,7 @@ if 'FOO' not in env: env['FOO'] = 'foo' -(side_effect, target) +(side_effect, target, temporary=False) @@ -2897,6 +2897,19 @@ or &f-env-Clean; function. + + +If temporary is set to +, the side effect is removed +from all build actions once at least one action +completes. This parameter should only be used in +special cases where a build tool performs a +one-time generation of a side effect and then leaves +it in place for the duration of the build. In this case, +SCons will avoid executing these actions at the same time +until the first one completes. After that, SCons will +no longer treat this file as a side effect. + diff --git a/SCons/EnvironmentTests.py b/SCons/EnvironmentTests.py index 53dd9a7c92..611cfa3193 100644 --- a/SCons/EnvironmentTests.py +++ b/SCons/EnvironmentTests.py @@ -3298,6 +3298,7 @@ def test_SideEffect(self): env = self.TestEnvironment(LIB='lll', FOO='fff', BAR='bbb') env.File('mylll.pdb') env.Dir('mymmm.pdb') + env.File('mynnn.pdb') foo = env.Object('foo.obj', 'foo.cpp')[0] bar = env.Object('bar.obj', 'bar.cpp')[0] @@ -3305,6 +3306,7 @@ def test_SideEffect(self): assert s.__class__.__name__ == 'Entry', s.__class__.__name__ assert s.get_internal_path() == 'mylib.pdb' assert s.side_effect + assert not s.side_effect_temporary assert foo.side_effects == [s] assert bar.side_effects == [s] @@ -3314,6 +3316,7 @@ def test_SideEffect(self): assert s.__class__.__name__ == 'File', s.__class__.__name__ assert s.get_internal_path() == 'mylll.pdb' assert s.side_effect + assert not s.side_effect_temporary assert fff.side_effects == [s], fff.side_effects assert bbb.side_effects == [s], bbb.side_effects @@ -3323,9 +3326,20 @@ def test_SideEffect(self): assert s.__class__.__name__ == 'Dir', s.__class__.__name__ assert s.get_internal_path() == 'mymmm.pdb' assert s.side_effect + assert not s.side_effect_temporary assert ggg.side_effects == [s], ggg.side_effects assert ccc.side_effects == [s], ccc.side_effects + hhh = env.Object('hhh.obj', 'hhh.cpp')[0] + ddd = env.Object('ddd.obj', 'ddd.cpp')[0] + s = env.SideEffect('mynnn.pdb', ['hhh.obj', 'ddd.obj'], temporary=True)[0] + assert s.__class__.__name__ == 'File', s.__class__.__name__ + assert s.get_internal_path() == 'mynnn.pdb' + assert s.side_effect + assert s.side_effect_temporary + assert hhh.side_effects == [s], hhh.side_effects + assert ddd.side_effects == [s], ddd.side_effects + def test_Split(self): """Test the Split() method""" env = self.TestEnvironment(FOO = 'fff', BAR = 'bbb') diff --git a/SCons/Node/FSTests.py b/SCons/Node/FSTests.py index 65cc363dff..f0f9a3ce3b 100644 --- a/SCons/Node/FSTests.py +++ b/SCons/Node/FSTests.py @@ -86,6 +86,9 @@ def Override(self, overrides): def _update(self, dict): pass + def get(self, key, default=None): + return default + class Action: def __call__(self, targets, sources, env, **kw): diff --git a/SCons/Node/__init__.py b/SCons/Node/__init__.py index 8a1677e60b..c695c9428d 100644 --- a/SCons/Node/__init__.py +++ b/SCons/Node/__init__.py @@ -42,6 +42,7 @@ import collections import copy +import os from itertools import chain try: @@ -539,6 +540,7 @@ class Node(object, metaclass=NoSlotsPyPy): 'includes', 'attributes', 'side_effect', + 'side_effect_temporary', 'side_effects', 'linked', '_memo', @@ -602,6 +604,7 @@ def __init__(self): self.includes = None self.attributes = self.Attrs() # Generic place to stick information about the Node. self.side_effect = 0 # true iff this node is a side effect + self.side_effect_temporary = 0 # true iff this node is a temporary side effect self.side_effects = [] # the side effects of building this target self.linked = 0 # is this node linked to the variant directory? self.changed_since_last_build = 0 @@ -1059,6 +1062,9 @@ def add_to_implicit(self, deps): self._children_reset() self._add_child(self.implicit, self.implicit_set, deps) + if os.name == 'nt': + self._add_windows_typelib_dependencies(deps) + def scan(self): """Scan this node's dependents for implicit dependencies.""" # Don't bother scanning non-derived files, because we don't @@ -1331,6 +1337,28 @@ def _add_child(self, collection, set, child): if added: self._children_reset() + def _add_windows_typelib_dependencies(self, deps): + """ + Adds typelib side effects on Windows. This is useful if a file + #import's a type library because the compiler automatically generates + .tlh and .tli files in the build directory. The compiler does not + synchronize writes of these files so they should be considered side + effects. Make them temporary because once the first action runs, + no more writes will happen. + """ + env = self.get_build_env() + if env.get('TYPELIB_SIDE_EFFECTS', True): + executor = self.get_executor() + targets = executor.get_all_targets() if executor else None + if targets: + for d in deps: + if d.name.endswith(('.dll', '.tlb', '.ocx')): + basename = os.path.splitext(d.name)[0] + env.SideEffect([ + targets[0].dir.File(basename + '.tlh'), + targets[0].dir.File(basename + '.tli'), + ], targets, temporary=True) + def set_specific_source(self, source): self.add_source(source) self._specific_sources = True diff --git a/SCons/Taskmaster.py b/SCons/Taskmaster.py index a1358e0476..eb36d227a7 100644 --- a/SCons/Taskmaster.py +++ b/SCons/Taskmaster.py @@ -452,6 +452,7 @@ def postprocess(self): parents[p] = parents.get(p, 0) + 1 t.waiting_parents = set() + temporary_side_effects_seen = set() for t in targets: if t.side_effects is not None: for s in t.side_effects: @@ -471,6 +472,18 @@ def postprocess(self): if p.ref_count == 0: self.tm.candidates.append(p) + if s.side_effect_temporary: + temporary_side_effects_seen.add(s) + + # Remove all temporary side effects from all sources. This prevents + # problems if the temporary side effect is picked up again by another + # target. + for s in temporary_side_effects_seen: + for side_effect_source in s.sources: + side_effect_source.side_effects.remove(s) + s.sources = [] + + for p, subtract in parents.items(): p.ref_count = p.ref_count - subtract if T: T.write(self.trace_message('Task.postprocess()', diff --git a/SCons/TaskmasterTests.py b/SCons/TaskmasterTests.py index f20fd716f6..6083ce2b6c 100644 --- a/SCons/TaskmasterTests.py +++ b/SCons/TaskmasterTests.py @@ -59,6 +59,7 @@ def targets(self, node): self.waiting_parents = set() self.waiting_s_e = set() self.side_effect = 0 + self.side_effect_temporary = 0 self.side_effects = [] self.alttargets = [] self.postprocessed = None diff --git a/test/Win32/typelib.py b/test/Win32/typelib.py new file mode 100644 index 0000000000..9c5f0cc8a4 --- /dev/null +++ b/test/Win32/typelib.py @@ -0,0 +1,56 @@ +#!/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__" + +import sys +import TestSCons + +# Run with 4 jobs to expose any race conditions related to the generated .tlh +# and .tli files. Then run again with only 1 job to expose any issues handling +# two different binaries one after another with the same typelib dependency. +print_skip_message = True +for arg in ['-j 4 .', '.']: + test = TestSCons.TestSCons() + + if sys.platform != 'win32': + # Only print the skip message once. + msg = "Skipping typelib test on non-Windows platform '%s'\n" % sys.platform \ + if print_skip_message else None + test.skip_test(msg) + print_skip_message = False + + test.dir_fixture('typelib') + + test.run('-j 4 .') + test.run('--clean .') + test.run() + + test.pass_test() + +# Local Variables: +# tab-width:4 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=4 shiftwidth=4: diff --git a/test/Win32/typelib/SConstruct b/test/Win32/typelib/SConstruct new file mode 100644 index 0000000000..21e7d816c6 --- /dev/null +++ b/test/Win32/typelib/SConstruct @@ -0,0 +1,15 @@ +import atexit + +program = Program('typelib.exe', source=[ + 'another.cpp', 'common.cpp', 'main.cpp']) + +program2 = Program('typelib2.exe', source=[ + 'another2.cpp', 'another3.cpp', 'common.cpp', 'main.cpp']) + + +def VerifySideEffects(): + # All side effects were temporary so they should be gone by now. + for source in program[0].sources: + assert 0 == len(source.side_effects), len(source.side_effects) + +atexit.register(VerifySideEffects) \ No newline at end of file diff --git a/test/Win32/typelib/another.cpp b/test/Win32/typelib/another.cpp new file mode 100644 index 0000000000..67c02a20a0 --- /dev/null +++ b/test/Win32/typelib/another.cpp @@ -0,0 +1 @@ +#include "common.h" diff --git a/test/Win32/typelib/another2.cpp b/test/Win32/typelib/another2.cpp new file mode 100644 index 0000000000..67c02a20a0 --- /dev/null +++ b/test/Win32/typelib/another2.cpp @@ -0,0 +1 @@ +#include "common.h" diff --git a/test/Win32/typelib/another3.cpp b/test/Win32/typelib/another3.cpp new file mode 100644 index 0000000000..67c02a20a0 --- /dev/null +++ b/test/Win32/typelib/another3.cpp @@ -0,0 +1 @@ +#include "common.h" diff --git a/test/Win32/typelib/common.cpp b/test/Win32/typelib/common.cpp new file mode 100644 index 0000000000..67c02a20a0 --- /dev/null +++ b/test/Win32/typelib/common.cpp @@ -0,0 +1 @@ +#include "common.h" diff --git a/test/Win32/typelib/common.h b/test/Win32/typelib/common.h new file mode 100644 index 0000000000..a6839012c7 --- /dev/null +++ b/test/Win32/typelib/common.h @@ -0,0 +1 @@ +#import "test.tlb" diff --git a/test/Win32/typelib/main.cpp b/test/Win32/typelib/main.cpp new file mode 100644 index 0000000000..acfa59c636 --- /dev/null +++ b/test/Win32/typelib/main.cpp @@ -0,0 +1,6 @@ +#include "common.h" + +int main() +{ + return 0; +} \ No newline at end of file diff --git a/test/Win32/typelib/test.tlb b/test/Win32/typelib/test.tlb new file mode 100644 index 0000000000..7ff565619f Binary files /dev/null and b/test/Win32/typelib/test.tlb differ