[bug #33138] .PARLLELSYNC enhancement with patch

2013-01-05 Thread David Boyce
Follow-up Comment #4, bug #33138 (project make):

This is the original author. I've become very busy at my day job in the last
year or two so I've lost track of this. Thanks for picking it up and improving
it. I haven't had time to look at your new patch yet, and not sure when I
will, but here are a few responses to your comments:

 Target vs flag

I have to agree that a flag is more correct. It's more convenient to use
.PARALLELSYNC because getting users to use a flag can be difficult (in fact
the major insight of Feldman's original make was that a naive user should be
able to just type make and the right thing will happen), but a flag is more
flexible. To the best of my memory (not so good) the reasons I went with a
special target are:

1. Given the clever --eval flag Paul added in 3.82 any special target can be
used as a flag, e.g. --eval .PARALLELSYNC: (BTW I tried to argue that --eval
was worthy of a scarce single-letter alias (-E) on the grounds that it was a
flag-to-end-all-flags but I don't think that happened).

2. Paul has some concerns about make becoming like GNU tar which apparently
has too many options. Personally I'm always willing to trade a few more
minutes of RTFM for more capability, and have no problem with GNU tar, but
tastes differ.

Bottom line, I'm more or less agnostic on flag-vs-target.

 tmpfile vs mkstemp

IIRC the reason I used tmpfile() instead of mkstemp() is that mkstemp doesn't
unlink the file automatically, which leads to a number of risk factors such as
filling up /tmp and so on.

Possibly the best/most portable approach would be to refactor the existing
open_tmpfile() function, which returns a FILE *, into an open_tmpfd() which
returns a descriptor wrapped by open_tmpfile() which just converts the
descriptor into a FILE * using fdopen(). This is already what happens, it's
just a matter of splitting existing logic into two functions. Now open_tmpfd()
is logically just mkstemp-with-porting-hacks and we could enhance it with an
unlink option for this feature.

 make[1]: Leaving directory 'bar' 
 make[1]: Leaving directory 'bar'

The problem with doubling these is that it becomes non-deterministic. What if
there was a directory structure foo/bar/bar/baz?

Additionally, would it be possible to implement the enhancement proposed in
the email discussion? See
http://lists.gnu.org/archive/html/bug-make/2011-04/msg00041.html for context?
Or was that already done? I don't remember.

Last, Paul has been pretty clear lately that new patches should be accompanied
by associated regression tests. That might be the remaining hurdle.

___

Reply to this item at:

  http://savannah.gnu.org/bugs/?33138

___
  Message sent via/by Savannah
  http://savannah.gnu.org/


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


[bug #33138] .PARLLELSYNC enhancement with patch

2013-01-05 Thread Frank Heckenbach
Follow-up Comment #5, bug #33138 (project make):

David Boyce wrote:

 This is the original author. I've become very busy at my day job in the
last
 year or two so I've lost track of this. Thanks for picking it up and
improving
 it. I haven't had time to look at your new patch yet, and not sure when I
 will, but here are a few responses to your comments:

Thanks for your reply.

  Target vs flag
 
 I have to agree that a flag is more correct. It's more convenient to use
 .PARALLELSYNC because getting users to use a flag can be difficult (in fact
 the major insight of Feldman's original make was that a naive user should
be
 able to just type make and the right thing will happen),

If the right thing depends on the setting (like with
.SECONDEXPANSION or various other flags), I definitely agree with
this reasoning.

However, in this case, it's not a question whether the build works
correctly or not, just how the output is presented to the user.
Therefore, different users may legitimately want different settings,
and this is easier to do with an argument than by modifying the
Makefile. Furthermore, this particular feature is interesting only
with parallel builds which require an option (-j) anyway. And, as
I mentioned, passing it to recursive makes is also easier this way.

 but a flag is more
 flexible. To the best of my memory (not so good) the reasons I went with a
 special target are:
 
 1. Given the clever --eval flag Paul added in 3.82 any special target can
be
 used as a flag, e.g. --eval .PARALLELSYNC: (BTW I tried to argue that
--eval
 was worthy of a scarce single-letter alias (-E) on the grounds that it was
a
 flag-to-end-all-flags but I don't think that happened).

I wasn't aware of this feature. So it should also work this way. but
if we want to support both ways I implemented (fine and coarse) we'd
need two special targets. So I still prefer the option.

 2. Paul has some concerns about make becoming like GNU tar which
apparently
 has too many options. Personally I'm always willing to trade a few more
 minutes of RTFM for more capability, and have no problem with GNU tar, but
 tastes differ.

Same for me. (So far make is quite short of the 52 available
letters. :)

What I could imagine is actually to make it the default when using
-j on the assumption that most users would prefer a properly
grouped output, and turn it off with an option. But it may be too
intrusive a change for a new feature, and I don't have a strong
opinion on that. (For me, it's just a one-time setup in my script
that invokes make with the right -j option depending on the system
configuration anyway.)

  tmpfile vs mkstemp
 
 IIRC the reason I used tmpfile() instead of mkstemp() is that mkstemp
doesn't
 unlink the file automatically, which leads to a number of risk factors such
as
 filling up /tmp and so on.
 
 Possibly the best/most portable approach would be to refactor the existing
 open_tmpfile() function, which returns a FILE *, into an open_tmpfd() which
 returns a descriptor wrapped by open_tmpfile() which just converts the
 descriptor into a FILE * using fdopen(). This is already what happens, it's
 just a matter of splitting existing logic into two functions. Now
open_tmpfd()
 is logically just mkstemp-with-porting-hacks and we could enhance it with
an
 unlink option for this feature.

I agree this seems useful. However, since it would involve changes
to existing code which are not strictly needed for this new feature
(it works as it is), I'd rather wait for the go-ahead from one of
the maintainers before making such a change.

  make[1]: Leaving directory 'bar'
  make[1]: Leaving directory 'bar'
 
 The problem with doubling these is that it becomes non-deterministic. What
if
 there was a directory structure foo/bar/bar/baz?

Actually the messages contain the absolute directory name (retrieved
by getcwd (current_directory, GET_PATH_MAX)); I just cut them in
my mail to avoid clutter and not expose my directories. So I don't
think there's actually a problem.

But I noticed another problem: Messages output by make itself (such
as Makefile:42: recipe for target 'foo' failed) are not always
properly enclosed with enter/leave messages. If they mention file
names, as here Makefile, this leads to the same issue.

Therefore I modified message(), error() and fatal() to enclose each
message with enter/leave messages, but only if parallel_sync is
active. (Otherwise, for simple uses it would clutter the output too
much.) The special case of message() with fmt==0 is left unchanged,
so the basic enter/leave messages still appear, even if no other
message are produced (the normal case with -s and clean builds),
as a kind of progress indicator.

Of course, when parallel_sync is used, this now results in even more
enter/leave messages, but if we want messages properly associated
with directories and don't want to mess with the messages
themselves, I see no alternative. (Again, this mode would usually be
used with machine-parsers like 

[bug #33138] .PARLLELSYNC enhancement with patch

2013-01-05 Thread Frank Heckenbach
Follow-up Comment #6, bug #33138 (project make):

PS: Ignore the ugly line breaks in the mail. I posted the comment via Savannah
(where the lines appear correctly). Apparently it messed them up when
forwarding it via email. :-(

___

Reply to this item at:

  http://savannah.gnu.org/bugs/?33138

___
  Nachricht gesendet von/durch Savannah
  http://savannah.gnu.org/


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make


[bug #33138] .PARLLELSYNC enhancement with patch

2013-01-05 Thread David Boyce
Follow-up Comment #7, bug #33138 (project make):

 I agree this seems useful. However, since it would involve changes
 to existing code which are not strictly needed...

Not functional changes, just  a little refactoring, and adding a new reliance
on tmpfile() may cause more stress to ports than enhancing the existing use of
mkstemp(). But I agree, this patch has reached the point where it needs an
up-or-down vote from Paul et al before any such final finish work is done.

David

___

Reply to this item at:

  http://savannah.gnu.org/bugs/?33138

___
  Message sent via/by Savannah
  http://savannah.gnu.org/


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make