Re: [bug #33138] .PARLLELSYNC enhancement with patch

[Frank Heckenbach]

(I'm afraid I hadn't seen your message from Sep 12, only your
followup on Sep 14, so I'm a bit late ...)

Paul Smith wrote:

> > One of the issues you were running into with your makefile is that
> > output from functions like $(info ...) that were expanded inside the
> > recipe were simply being printed to stdout directly rather than to the
> > sync file.  I fixed this so that it shouldn't be possible to get output
> > outside the appropriate Enter/Leave messages.
> >
> > I think there may still be some change needed for directory tracking for
> > the -Orecurse mode.  If we're collecting output for an entire recursive
> > make invocation we don't need enter/leave notifications around each
> > individual target or line, which is what we have now.  We only need it
> > around the entire makefile.  I'll look into this.
>
> OK, this should be all fixed now.  I believe it's operating the way I
> want.  Please try it out and report anything unusual or that you have
> problems with.

I did some testing now. So far, it mostly works well, though I found
a few issues. Most of them are minor or stylistic, but the first two
might be more important:

1.

% cat Makefile
include foo

all:

foo:
        touch foo

% rm -f foo; make -w
make: Entering directory 'foo'
Makefile:1: foo: No such file or directory
touch foo
make: Entering directory 'foo'
make: Nothing to be done for 'all'.
make: Leaving directory 'foo'

Twice "Entering", once "Leaving". I think there's an output_close()
missing before reexec (which might also lose other messages in other
situations):

--- main.c
+++ main.c
@@ -2330,6 +2330,7 @@
               putenv (b);
             }
 
+          output_close (NULL);
           fflush (stdout);
           fflush (stderr);
 

2.

% cat Makefile
all: t1 t2

t1:
        $(MAKE) -Cd1

t2:
        $(MAKE) -Cd2
% cat d1/Makefile
$(info d1)

all:
% cat d2/Makefile
$(info d2)

all:

% make -j -Oline # or -Otarget
make -Cd1
make -Cd2
make[1]: Entering directory 'foo/d1'
d1
make[1]: Entering directory 'foo/d2'
d2
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory 'foo/d1'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory 'foo/d2'

(Non-deterministic, but this it the output I get most of the time.)

So the $(info) output is now inside the appropriate Enter/Leave
messages, but those are nested improperly because they are not
synced.

Of course, not syncing them seems right at first glance, when I
request syncing on a target level, and no (non-recursive) targets
are being run, but the improper nesting doesn't look nice (and might
confuse tools that try to parse the messages).

As I understand it, you addressed this kind of problem for the
-Orecurse case with the "output_sync == OUTPUT_SYNC_RECURSE"
condition in output_start(), but unfortunately not for
-Otarget/-Oline.

To fix it for all cases, I'm afraid the only way I can see is to set
up an extra temp file in this situation and dump it before a child
is run. At least this seems easier to do now with your new
infrastructure.

As a side benefit, this would simplify the logic in output_start();
AFAICS, it could just say "if (!output_sync)" then; and
correspondingly remove the "output_sync != OUTPUT_SYNC_RECURSE"
condition in output_dump(), i.e. those messages are always produced
by output_dump() if syncing and by output_start()/output_close()
otherwise, which would incidentally also fix the next problem:

3.

% cat Makefile
$(shell echo foo >&2)

all:
        echo bar

% make -s -w -Onone # or -Orecurse
make: Entering directory 'foo'
foo
bar
make: Leaving directory 'foo'
% make -s -w -Oline # or -Otarget
make: Entering directory 'foo'
foo
make: Entering directory 'foo'
bar
make: Leaving directory 'foo'
make: Leaving directory 'foo'

So with -Otarget or -Oline, there's double Enter/Leave messages.
(I should note that I personally don't mind this so much, seeing as
I had implemented "excessive" messages before. :) Anyway, I think
that's due to both of these being executed here:

  stdio_traced = log_working_directory (NULL, 1);

  traced = log_working_directory (output_context, 1);

If this is not fixed together with 2., a solution might be to just
suppress the 2nd one if the 1st one was already done, but I haven't
checked this carefully.

4.

% cat Makefile
all:
        $(shell echo foo >&2)

% make -s -w -Onone # or -Orecurse
make: Entering directory 'foo'
foo
make: Leaving directory 'foo'
% make -s -w -Oline # or -Otarget
foo

Here, with -Otarget or -Oline there's no Enter/Leave messages if
there's no other output. Not sure if you consider this an actual
problem. The solution seems to be to redirect stderr (only) before
$(shell) executions just like we do for recipe jobs.

5.

ISTM when output_dump() is called, output_context always ought to
be NULL (the call is either outside of any OUTPUT_SET/OUTPUT_UNSET
section, or (job.c:1274) child->output.syncout is NULL), is that
right?

If so, the use of output_context might be slightly irritating
(though not wrong) -- at first I wondered where the
log_working_directory() output after the pump_from_tmp() calls was
going to and whether it didn't need pumping too if it was going to
the temp file, but apparently this never happens.

In fact, if that's so, it seems you could eliminate
the out parameter to log_working_directory() entirely.

6.

output.c:379: /* We've entered the "critical section" during which a lock is 
held. ... */

You might want to move this comment a few lines up since the
previous if-statement is already within the critical section.

7.

job.c:1258: OUTPUT_UNSET();

Just wondering, is this necessary? (It's before OUTPUT_SET().)