Boost logo

Boost-Commit :

From: jurko.gospodnetic_at_[hidden]
Date: 2008-08-28 15:20:28


Author: jurko
Date: 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
New Revision: 48426
URL: http://svn.boost.org/trac/boost/changeset/48426

Log:
Fixed a Boost Jam bug causing it to sometimes trigger actions depending on targets that have not been built yet. Test case included. Updated related code comments.

Bug was happening when we had a multifile action that got triggered to build its non-initial target. Then while that action was being executed all the other targets were reporting as 'already built' and were getting used by other actions prematurely. Quick-fixed by making all targets built by a single action list each other as 'included' causing anything else depending on any of these targets to automatically depend on all the others in the group as well.

The solution is not perfect as it might have some unexpected interactions with other uses of 'included' targets and now if any target in a group is not up to date then all of them will be rebuilt even if actually did not need the target that was up to date. On the other hand this should be a really rare use case as it would require the one target in a group to be up to date and be needed while another in the same group (i.e. built by the same action) to not be up to date.
Added:
   trunk/tools/jam/test/parallel_multifile_actions_1.jam
      - copied, changed from r48362, /trunk/tools/jam/test/parallel_multifile_actions.jam
   trunk/tools/jam/test/parallel_multifile_actions_2.jam (contents, props changed)
Removed:
   trunk/tools/jam/test/parallel_multifile_actions.jam
Text files modified:
   trunk/tools/jam/src/compile.c | 45 ++++++++++++++++++++++++++++++---------
   trunk/tools/jam/src/rules.c | 18 +++++++++++++++
   trunk/tools/jam/src/rules.h | 1
   trunk/tools/jam/test/parallel_multifile_actions_1.jam | 4 +-
   trunk/tools/jam/test/test.jam | 3 +
   5 files changed, 56 insertions(+), 15 deletions(-)

Modified: trunk/tools/jam/src/compile.c
==============================================================================
--- trunk/tools/jam/src/compile.c (original)
+++ trunk/tools/jam/src/compile.c 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
@@ -1060,22 +1060,45 @@
         action->targets = targetlist( (TARGETS *)0, lol_get( frame->args, 0 ) );
         action->sources = targetlist( (TARGETS *)0, lol_get( frame->args, 1 ) );
         
- /* Make targets[1,N-1] depend on targets[0], to describe the multply
- generated targets for the rule. Do it with includes, to reflect
- non-build dependency. */
+ /* If we have a group of targets all being built using the same action
+ * then we must not allow any of them to be used as sources unless they
+ * had all already been built in the first place or their joined action
+ * has had a chance to finish its work and build all of them anew.
+ *
+ * Without this it might be possible, in case of a multi-process build,
+ * for their action, triggered by buiding one of the targets, to still
+ * be running when another target in the group reports as done in order
+ * to avoid triggering the same action again and gets used prematurely.
+ *
+ * As a quick-fix to achieve this effect we make all the targets list
+ * each other as 'included targets'. More precisely, we mark the first
+ * listed target as including all the other targets in the list and vice
+ * versa. This makes anyone depending on any of those targets implicitly
+ * depend on all of them, thus making sure none of those targets can be
+ * used as sources until all of them have been built. Note that direct
+ * dependencies could not have been used due to the 'circular
+ * dependency' issue.
+ *
+ * TODO: Although the current implementation solves the problem of one
+ * of the targets getting used before its action completes its work it
+ * also forces the action to run whenever any of the targets in the
+ * group is not up to date even though some of them might not actually
+ * be used by the targets being built. We should see how we can
+ * correctly recognize such cases and use that to avoid running the
+ * action if possible and not rebuild targets not actually depending on
+ * targets that are not up to date.
+ *
+ * TODO: Using the 'include' feature might have side-effects due to
+ * interaction with the actual 'inclusion scanning' system. This should
+ * be checked.
+ */
         if ( action->targets )
         {
             TARGET * t0 = action->targets->target;
             for ( t = action->targets->next; t; t = t->next )
             {
- TARGET * tn = t->target;
- if ( !tn->includes )
- {
- tn->includes = copytarget( tn );
- tn->includes->original_target = tn;
- }
- tn = tn->includes;
- tn->depends = targetentry( tn->depends, t0 );
+ add_include( t->target, t0 );
+ add_include( t0, t->target );
             }
         }
 

Modified: trunk/tools/jam/src/rules.c
==============================================================================
--- trunk/tools/jam/src/rules.c (original)
+++ trunk/tools/jam/src/rules.c 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
@@ -60,7 +60,23 @@
 static struct hash *located_targets = 0;
 
 
-
+/*
+ * add_include() - adds the 'included' TARGET to the list of targets included by
+ * the 'including' TARGET. Such targets are modeled as dependencies of the
+ * internal include node belonging to the 'including' TARGET.
+ */
+void add_include( TARGET * including, TARGET * included )
+{
+ TARGET * internal;
+ if ( !including->includes )
+ {
+ including->includes = copytarget( including );
+ including->includes->original_target = including;
+ }
+ internal = including->includes;
+ internal->depends = targetentry( internal->depends, included );
+}
+
 
 /*
  * enter_rule() - return pointer to RULE, creating it if necessary in

Modified: trunk/tools/jam/src/rules.h
==============================================================================
--- trunk/tools/jam/src/rules.h (original)
+++ trunk/tools/jam/src/rules.h 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
@@ -230,6 +230,7 @@
     char* failed;
 } ;
 
+void add_include( TARGET * including, TARGET * included );
 RULE *bindrule( char *rulename, module_t* );
 
 RULE* import_rule( RULE* source, module_t* m, char* name );

Deleted: trunk/tools/jam/test/parallel_multifile_actions.jam
==============================================================================
--- trunk/tools/jam/test/parallel_multifile_actions.jam 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
+++ (empty file)
@@ -1,45 +0,0 @@
-#~ Copyright 2007 Rene Rivera.
-#~ Distributed under the Boost Software License, Version 1.0.
-#~ (See accompanying file LICENSE_1_0.txt or http://www.boost.org/LICENSE_1_0.txt)
-
-if ! $(BJAM_SUBTEST)
-{
- ECHO --- Testing -jN parallel execution of multi-file actions... ;
-
- assert "...found 6 targets...
-...updating 4 targets...
-.gen. g1.generated
-001
-002
-.use.1 u1.user
-003
-.use.2 u2.user
-004
-...updated 4 targets...
-" : (==) : [ SHELL "\"$(ARGV[1])\" -f parallel_multifile_actions.jam -sBJAM_SUBTEST=1 -j2" ] ;
-}
-else
-{
- actions .gen. {
-echo 001
-sleep 4
-echo 002
-}
- rule .use.1 { DEPENDS $(<) : $(>) ; }
- actions .use.1 {
-echo 003
-}
- rule .use.2 { DEPENDS $(<) : $(>) ; }
- actions .use.2 {
-sleep 1
-echo 004
-}
-
- .gen. g1.generated g2.generated ;
- .use.1 u1.user : g1.generated ;
- .use.2 u2.user : g2.generated ;
-
- NOTFILE root ;
- DEPENDS g1.generated g2.generated : root ;
- DEPENDS all : u1.user u2.user ;
-}

Copied: trunk/tools/jam/test/parallel_multifile_actions_1.jam (from r48362, /trunk/tools/jam/test/parallel_multifile_actions.jam)
==============================================================================
--- /trunk/tools/jam/test/parallel_multifile_actions.jam (original)
+++ trunk/tools/jam/test/parallel_multifile_actions_1.jam 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
@@ -4,7 +4,7 @@
 
 if ! $(BJAM_SUBTEST)
 {
- ECHO --- Testing -jN parallel execution of multi-file actions... ;
+ ECHO --- Testing -jN parallel execution of multi-file actions - 1... ;
 
     assert "...found 6 targets...
 ...updating 4 targets...
@@ -16,7 +16,7 @@
 .use.2 u2.user
 004
 ...updated 4 targets...
-" : (==) : [ SHELL "\"$(ARGV[1])\" -f parallel_multifile_actions.jam -sBJAM_SUBTEST=1 -j2" ] ;
+" : (==) : [ SHELL "\"$(ARGV[1])\" -f parallel_multifile_actions_1.jam -sBJAM_SUBTEST=1 -j2" ] ;
 }
 else
 {

Added: trunk/tools/jam/test/parallel_multifile_actions_2.jam
==============================================================================
--- (empty file)
+++ trunk/tools/jam/test/parallel_multifile_actions_2.jam 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
@@ -0,0 +1,49 @@
+# Copyright 2008 Jurko Gospodnetic, Vladimir Prus
+# Distributed under the Boost Software License, Version 1.0.
+# (See accompanying file LICENSE_1_0.txt or http://www.boost.org/LICENSE_1_0.txt)
+
+# Added to guard against a bug causing targets to be used before they
+# themselves have finished building. This used to happen for targets built by a
+# multi-file action that got triggered by another target, except when the target
+# triggering the action was the first one in the list of targets produced by
+# that action.
+#
+# Example:
+# When target A and target B were declared as created by a single action with
+# A being the first one listed, and target B triggered running that action then
+# while the action was still running, target A was already reporting as being
+# built causing other targets depending on target A to be built prematurely.
+
+if ! $(BJAM_SUBTEST)
+{
+ ECHO --- Testing -jN parallel execution of multi-file actions - 2... ;
+
+ assert "...found 4 targets...
+...updating 3 targets...
+link dll
+001 - linked
+install installed_dll
+002 - installed
+...updated 3 targets...
+" : (==) : [ SHELL "\"$(ARGV[1])\" -f parallel_multifile_actions_2.jam -sBJAM_SUBTEST=1 -j2" ] ;
+}
+else
+{
+ actions link
+ {
+ sleep 1
+ echo 001 - linked
+ }
+
+ link dll lib ;
+
+ actions install
+ {
+ echo 002 - installed
+ }
+
+ install installed_dll : dll ;
+ DEPENDS installed_dll : dll ;
+
+ DEPENDS all : lib installed_dll ;
+}

Modified: trunk/tools/jam/test/test.jam
==============================================================================
--- trunk/tools/jam/test/test.jam (original)
+++ trunk/tools/jam/test/test.jam 2008-08-28 15:20:27 EDT (Thu, 28 Aug 2008)
@@ -58,7 +58,8 @@
 include option_l.jam ;
 include option_n.jam ;
 include parallel_actions.jam ;
-include parallel_multifile_actions.jam ;
+include parallel_multifile_actions_1.jam ;
+include parallel_multifile_actions_2.jam ;
 include stress_var_expand.jam ;
 include target_var.jam ;
 include var_expand.jam ;


Boost-Commit list run by bdawes at acm.org, david.abrahams at rcn.com, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk