Bug in update_goal_chain() logic in make 3.79.1

[Patrick J. LoPresti]

I have attached a Makefile to the end of this message which
illustrates this bug.  Make should exit successfully when run on this
Makefile, but it does not if you invoke it like this (assuming your
load average is greater than zero):

  rm -f Makefile2 Makefile3 ; make -j 2 -l 0.0

The problem is that the logic for seeing if a goal has "changed" is
faulty.  In remake.c:update_goal_chain(), g->changed is computed by
the following code:

              /* Save the old value of `commands_started' so we can compare
                 later.  It will be incremented when any commands are
                 actually run.  */
              ocommands_started = commands_started;

              x = update_file (file, makefiles ? 1 : 0);
              check_renamed (file);

              /* Set the goal's `changed' flag if any commands were started
                 by calling update_file above.  We check this flag below to
                 decide when to give an "up to date" diagnostic.  */
              g->changed += commands_started - ocommands_started;

The problem is that the call to update_file() does not always cause
commands_started to be incremented, even when there is work to be done
for the goal.  This is because of the following logic in
job.c:start_waiting_job():

  /* If we are running at least one job already and the load average
     is too high, make this one wait.  */
  if (!c->remote && job_slots_used > 0 && load_too_high ())
    {
      /* Put this child on the chain of children waiting for the load average
         to go down.  */
      set_command_state (f, cs_running);
      c->next = waiting_jobs;
      waiting_jobs = c;
      return 0;
    }

  /* Start the first command; reap_children will run later command lines.  */
  start_job_command (c);

That is, when the job is being placed on the waiting chain because the
load is too high, start_job_command() is never called.  And then
commands_started is never incremented, so update_goal_chain() does not
set g->changed.

There is more than one way to fix this.  One way would be to
re-activate the following code in update_goal_chain(), which was
commented out for a long time and finally removed in the current
version of make:

#if 0
  /* Only run one job at a time when building makefiles.
     No one seems to know why this was done, and no one can think of a good
     reason to do it.  Hopefully an obvious one won't appear as soon as we
     release the next version :-/.  */
  if (makefiles)
    job_slots = 1;
#endif

(As an aside...  With this code gone, the variable "j" in
update_goal_chain() can be removed, since it is only used to save and
restore the value of job_slots around this no-longer-present code.)

But that would be an ugly fix.  A better (but still somewhat ugly) fix
would be to move the "commands_started++" line out of
job.c:start_job_command() and put it in job.c:start_waiting_job()
instead.

The best fix would be to gut the logic for computing g->changed and
rewrite it completely, by decided exactly what question you are trying
to answer, seeing which code actually knows the answer, and having
that code tell you.  I do not understand these internals well enough
to answer these questions, but it is very clear that
start_job_command() is *not* the code which can answer questions about
g->changed.

If this message was unclear or not sufficiently detailed, please let
me know.  Thanks.

 - Pat

Makefile
Description: test Makefile to illustrate bug