Skip to content

Commit

Permalink
Make the handling of actions that produce multiple targets more correct.
Browse files Browse the repository at this point in the history
[SVN r86561]
  • Loading branch information
swatanabe committed Nov 4, 2013
1 parent 5e39960 commit c1395b4
Show file tree
Hide file tree
Showing 8 changed files with 572 additions and 135 deletions.
37 changes: 37 additions & 0 deletions src/engine/command.c
Expand Up @@ -23,6 +23,37 @@
#include <assert.h>


/*
* cmdlist_append_cmd
*/
CMDLIST * cmdlist_append_cmd( CMDLIST * l, CMD * cmd )
{
CMDLIST * result = (CMDLIST *)BJAM_MALLOC( sizeof( CMDLIST ) );
result->iscmd = 1;
result->next = l;
result->impl.cmd = cmd;
return result;
}

CMDLIST * cmdlist_append_target( CMDLIST * l, TARGET * t )
{
CMDLIST * result = (CMDLIST *)BJAM_MALLOC( sizeof( CMDLIST ) );
result->iscmd = 0;
result->next = l;
result->impl.t = t;
return result;
}

void cmdlist_free( CMDLIST * l )
{
while ( l )
{
CMDLIST * tmp = l->next;
BJAM_FREE( l );
l = tmp;
}
}

/*
* cmd_new() - return a new CMD.
*/
Expand All @@ -37,6 +68,10 @@ CMD * cmd_new( RULE * rule, LIST * targets, LIST * sources, LIST * shell )
cmd->shell = shell;
cmd->next = 0;
cmd->noop = 0;
cmd->asynccnt = 1;
cmd->status = 0;
cmd->lock = NULL;
cmd->unlock = NULL;

lol_init( &cmd->args );
lol_add( &cmd->args, targets );
Expand All @@ -62,9 +97,11 @@ CMD * cmd_new( RULE * rule, LIST * targets, LIST * sources, LIST * shell )

void cmd_free( CMD * cmd )
{
cmdlist_free( cmd->next );
lol_free( &cmd->args );
list_free( cmd->shell );
string_free( cmd->buf );
freetargets( cmd->unlock );
BJAM_FREE( (void *)cmd );
}

Expand Down
29 changes: 28 additions & 1 deletion src/engine/command.h
Expand Up @@ -46,14 +46,41 @@


typedef struct _cmd CMD;

/*
* A list whose elements are either TARGETS or CMDS.
* CMDLIST is used only by CMD. A TARGET means that
* the CMD is the last updating action required to
* build the target. A CMD is the next CMD required
* to build the same target. (Note that a single action
* can update more than one target, so the CMDs form
* a DAG, not a straight linear list.)
*/
typedef struct _cmdlist {
struct _cmdlist * next;
union {
CMD * cmd;
TARGET * t;
} impl;
char iscmd;
} CMDLIST;

CMDLIST * cmdlist_append_cmd( CMDLIST *, CMD * );
CMDLIST * cmdlist_append_target( CMDLIST *, TARGET * );
void cmd_list_free( CMDLIST * );

struct _cmd
{
CMD * next;
CMDLIST * next;
RULE * rule; /* rule->actions contains shell script */
LIST * shell; /* $(JAMSHELL) value */
LOL args; /* LISTs for $(<), $(>) */
string buf[ 1 ]; /* actual commands */
int noop; /* no-op commands should be faked instead of executed */
int asynccnt; /* number of outstanding dependencies */
TARGETS * lock; /* semaphores that are required by this cmd. */
TARGETS * unlock; /* semaphores that are released when this cmd finishes. */
char status; /* the command status */
};

CMD * cmd_new
Expand Down
49 changes: 5 additions & 44 deletions src/engine/compile.c
Expand Up @@ -117,56 +117,17 @@ LIST * evaluate_rule( RULE * rule, OBJECT * rulename, FRAME * frame )
action->refs = 1;

/* 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
* are all up to date and their action does not need to be run or their
* 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 to building 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: Current solution using fake INCLUDES relations may cause
* actions to be run when the affected targets are built by multiple
* actions. E.g. if we have the following actions registered in the
* order specified:
* (I) builds targets A & B
* (II) builds target B
* and we want to build a target depending on target A, then both
* actions (I) & (II) will be run, even though the second one does not
* have any direct relationship to target A. Consider whether this is
* desired behaviour or not. It could be that Boost Build should (or
* possibly already does) run all actions registered for a given target
* if any of them needs to be run in which case our INCLUDES relations
* are not actually causing any actions to be run that would not have
* been run without them.
* and any of these targets is updated, then we have to consider them
* all to be out-dated. We do this by adding a REBUILDS in both directions
* between the first target and all the other targets.
*/
if ( action->targets )
{
TARGET * const t0 = action->targets->target;
for ( t = action->targets->next; t; t = t->next )
{
target_include( t->target, t0 );
target_include( t0, t->target );
t->target->rebuilds = targetentry( t->target->rebuilds, t0 );
t0->rebuilds = targetentry( t0->rebuilds, t->target );
}
}

Expand Down
29 changes: 28 additions & 1 deletion src/engine/make.c
Expand Up @@ -161,6 +161,8 @@ int make( LIST * targets, int anyhow )
* make0() to be updated.
*/

static void force_rebuilds( TARGET * t );

static void update_dependants( TARGET * t )
{
TARGETS * q;
Expand Down Expand Up @@ -190,6 +192,8 @@ static void update_dependants( TARGET * t )
update_dependants( p );
}
}
/* Make sure that rebuilds can be chained. */
force_rebuilds( t );
}


Expand Down Expand Up @@ -676,7 +680,30 @@ void make0
else
fate = t->fate;

/* Step 4g: If this target needs to be built, force rebuild everything in
/*
* Step 4g: If this target needs to be built, make0 all targets
* that are updated by the same actions used to update this target.
* These have already been marked as REBUILDS, and make1 has
* special handling for them. We just need to make sure that
* they get make0ed.
*/
if ( ( fate >= T_FATE_BUILD ) && ( fate < T_FATE_BROKEN ) )
{
ACTIONS * a;
TARGETS * c;
for ( a = t->actions; a; a = a->next )
{
for ( c = a->action->targets; c; c = c->next )
{
if ( c->target->fate == T_FATE_INIT )
{
make0( c->target, ptime, depth + 1, counts, anyhow, rescanning );
}
}
}
}

/* Step 4h: If this target needs to be built, force rebuild everything in
* its rebuilds list.
*/
if ( ( fate >= T_FATE_BUILD ) && ( fate < T_FATE_BROKEN ) )
Expand Down

0 comments on commit c1395b4

Please sign in to comment.