From e7888b15628b09af093621825b78df5cbcead088 Mon Sep 17 00:00:00 2001 From: Gary Oberbrunner Date: Sun, 27 Oct 2013 07:46:41 -0400 Subject: Correctly fix bug #2903, failure to rebuild when linker options change. The failure to rebuild when linker options change was introduced in abded0675444, "Add library version support to Shared Lib builder", between 2.2.0 and 2.3.0. Turning ShlinkAction into a FunctionAction instead of a CommandAction made it stop depending on $SHLINKCOM. Normally, a CommandAction calls get_presig (Action.py:815) which removes $( ... $) and uses the rest of that string (recursively fully substituted) as the contents to hash. FunctionActions only look at the body of the function, so that removed the dependency on $SHLINKCOM altogether. Adding it back in the varlist does this (Action.py:443): for v in vl: result.append(env.subst('${'+v+'}')) so it deep-substitutes the whole thing, including all the $( ... $) parts. (All varlist vars do this, not just in FunctionActions.) What we really want is to depend on the value of env['SHLINKCOM'] in the way CommandActions do, i.e. without the $( ... $) parts, definitely not the fully substituted version of it. I'm pretty sure the ignored $(...$) parts should not ever be included in the signature, so this change updates the varlist handling code to work the way CommandActions always have. This change also renames the test files to use the correct bug number and updates the test. --- src/engine/SCons/Action.py | 3 ++- src/engine/SCons/Tool/__init__.py | 8 ++++++-- test/Libs/SharedLibrary-update-deps.py | 17 +++++++++++++++-- test/Libs/bug2903/SConstruct | 13 +++++++++++++ test/Libs/bug2903/SConstruct-libs | 3 +++ test/Libs/bug2903/lib.c | 1 + test/Libs/bug2903/main.c | 3 +++ test/Libs/bug2909/SConstruct | 3 --- test/Libs/bug2909/SConstruct-libs | 3 --- test/Libs/bug2909/lib.c | 1 - test/Libs/bug2909/main.c | 3 --- 11 files changed, 43 insertions(+), 15 deletions(-) create mode 100644 test/Libs/bug2903/SConstruct create mode 100644 test/Libs/bug2903/SConstruct-libs create mode 100644 test/Libs/bug2903/lib.c create mode 100644 test/Libs/bug2903/main.c delete mode 100644 test/Libs/bug2909/SConstruct delete mode 100644 test/Libs/bug2909/SConstruct-libs delete mode 100644 test/Libs/bug2909/lib.c delete mode 100644 test/Libs/bug2909/main.c diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py index c1eef75..543c37c 100644 --- a/src/engine/SCons/Action.py +++ b/src/engine/SCons/Action.py @@ -439,7 +439,8 @@ class ActionBase(object): vl = self.get_varlist(target, source, env) if is_String(vl): vl = (vl,) for v in vl: - result.append(env.subst('${'+v+'}')) + # do the subst this way to ignore $(...$) parts: + result.append(env.subst_target_source('${'+v+'}', SCons.Subst.SUBST_SIG, target, source)) return ''.join(result) def __add__(self, other): diff --git a/src/engine/SCons/Tool/__init__.py b/src/engine/SCons/Tool/__init__.py index 7477f75..c09f8e4 100644 --- a/src/engine/SCons/Tool/__init__.py +++ b/src/engine/SCons/Tool/__init__.py @@ -364,8 +364,12 @@ symlinks for the platform we are on""" print "VerShLib: made sym link of %s -> %s" % (linkname, lib_ver) return result -# Fix http://scons.tigris.org/issues/show_bug.cgi?id=2903 : -# varlist=['$SHLINKCOM']: ensure we still depend on SCons.Defaults.ShLinkAction command line which is $SHLINKCOM +# Fix http://scons.tigris.org/issues/show_bug.cgi?id=2903 : +# Ensure we still depend on SCons.Defaults.ShLinkAction command line which is $SHLINKCOM. +# This was tricky because we don't want changing LIBPATH to cause a rebuild, but +# changing other link args should. LIBPATH has $( ... $) around it but until this +# fix, when the varlist was added to the build sig those ignored parts weren't getting +# ignored. ShLibAction = SCons.Action.Action(VersionedSharedLibrary, None, varlist=['SHLINKCOM']) def createSharedLibBuilder(env): diff --git a/test/Libs/SharedLibrary-update-deps.py b/test/Libs/SharedLibrary-update-deps.py index 24c5262..e8ec2ab 100644 --- a/test/Libs/SharedLibrary-update-deps.py +++ b/test/Libs/SharedLibrary-update-deps.py @@ -26,18 +26,20 @@ __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" """ Test that SharedLibrary() updates when a different lib is linked, even if it has the same md5. -This is Tigris bug #2909. +This is Tigris bug #2903. """ +import sys import os.path import TestSCons test = TestSCons.TestSCons() -test.dir_fixture( "bug2909" ) +test.dir_fixture( "bug2903" ) # Build the sub-libs (don't care about details of this) test.run(arguments='-f SConstruct-libs') + # This should build the main lib, using libfoo.so test.run(arguments='libname=foo') # This should rebuild the main lib, using libbar.so; @@ -48,6 +50,17 @@ test.must_not_contain_any_line(test.stdout(), ["is up to date"]) test.run(arguments='libname=foo') test.must_not_contain_any_line(test.stdout(), ["is up to date"]) +# Now try changing the link command line (in an innocuous way); should rebuild. +if sys.platform == 'win32': + extraflags='shlinkflags=/DEBUG' +else: + extraflags='shlinkflags=-g' + +test.run(arguments=['libname=foo', extraflags]) +test.must_not_contain_any_line(test.stdout(), ["is up to date"]) +test.run(arguments=['libname=foo', extraflags]) +test.must_contain_all_lines(test.stdout(), ["is up to date"]) + test.pass_test() # Local Variables: diff --git a/test/Libs/bug2903/SConstruct b/test/Libs/bug2903/SConstruct new file mode 100644 index 0000000..f7b66a8 --- /dev/null +++ b/test/Libs/bug2903/SConstruct @@ -0,0 +1,13 @@ +# SConstruct for testing but #2903. +# The test changes the lib name to make sure it rebuilds +# when the name changes, even if the content of the lib is the same. +# Also, the test passes in extra shlinkflags to ensure things rebuild +# when other linker options change, and not when they don't. +# (This doesn't specifically test LIBPATH, but there's a test for +# that already.) +env=Environment() +libname=ARGUMENTS.get('libname', 'foo') +env['SHLINKCOM'] = env['SHLINKCOM'] + ' $EXTRA_SHLINKFLAGS' +shlinkflags=ARGUMENTS.get('shlinkflags', '') +env.SharedLibrary('myshared', ['main.c'], + LIBS=[libname], LIBPATH='.', EXTRA_SHLINKFLAGS=shlinkflags) diff --git a/test/Libs/bug2903/SConstruct-libs b/test/Libs/bug2903/SConstruct-libs new file mode 100644 index 0000000..3f59f9c --- /dev/null +++ b/test/Libs/bug2903/SConstruct-libs @@ -0,0 +1,3 @@ +env=Environment() +libfoo = env.SharedLibrary('foo', 'lib.c') +env.InstallAs('${SHLIBPREFIX}bar${SHLIBSUFFIX}', libfoo) \ No newline at end of file diff --git a/test/Libs/bug2903/lib.c b/test/Libs/bug2903/lib.c new file mode 100644 index 0000000..048f715 --- /dev/null +++ b/test/Libs/bug2903/lib.c @@ -0,0 +1 @@ +int i; diff --git a/test/Libs/bug2903/main.c b/test/Libs/bug2903/main.c new file mode 100644 index 0000000..3fe7d49 --- /dev/null +++ b/test/Libs/bug2903/main.c @@ -0,0 +1,3 @@ +void func() +{ +} diff --git a/test/Libs/bug2909/SConstruct b/test/Libs/bug2909/SConstruct deleted file mode 100644 index 2c5440b..0000000 --- a/test/Libs/bug2909/SConstruct +++ /dev/null @@ -1,3 +0,0 @@ -env=Environment() -libname=ARGUMENTS.get('libname', 'foo') -env.SharedLibrary('myshared', ['main.c'], LIBS=[libname], LIBPATH='.') \ No newline at end of file diff --git a/test/Libs/bug2909/SConstruct-libs b/test/Libs/bug2909/SConstruct-libs deleted file mode 100644 index 3f59f9c..0000000 --- a/test/Libs/bug2909/SConstruct-libs +++ /dev/null @@ -1,3 +0,0 @@ -env=Environment() -libfoo = env.SharedLibrary('foo', 'lib.c') -env.InstallAs('${SHLIBPREFIX}bar${SHLIBSUFFIX}', libfoo) \ No newline at end of file diff --git a/test/Libs/bug2909/lib.c b/test/Libs/bug2909/lib.c deleted file mode 100644 index 048f715..0000000 --- a/test/Libs/bug2909/lib.c +++ /dev/null @@ -1 +0,0 @@ -int i; diff --git a/test/Libs/bug2909/main.c b/test/Libs/bug2909/main.c deleted file mode 100644 index 3fe7d49..0000000 --- a/test/Libs/bug2909/main.c +++ /dev/null @@ -1,3 +0,0 @@ -void func() -{ -} -- cgit v0.12