Re: possible solution for -Otarget recurse (was: Re: Some serious issues with the new -O option)

[Eli Zaretskii]

> From: Paul Smith <address@hidden>
> Cc: address@hidden
> Date: Sat, 04 May 2013 17:51:05 -0400
> 
> Eli, I did some cleanup in job.c to try to make things simpler and
> reduce duplication.  I tried to be careful but it's quite possible I did
> something to disrupt the Windows version again.  It's up to you if you
> want to fix any problems now or wait until I solve the above two issues
> and look at it all at the same time.

There's nothing to fix, as things still seem to work.  Thanks!

However, I wonder what was the reason for splitting the definition of
GMK_EXPORT in two, and putting each part in a different file.  The way
they were together before is how programmers are used to see this
stuff on Windows; splitting them will not something people will
expect.  I'm sure you can see this in quite a few packages out there.
Here's a random example from GMP's gmp.h, slightly edited for clarity:

  #define __GMP_DECLSPEC_EXPORT  __declspec(__dllexport__)
  #define __GMP_DECLSPEC_IMPORT  __declspec(__dllimport__)

  #if __GMP_LIBGMP_DLL
  #if __GMP_WITHIN_GMP
  /* compiling to go into a DLL libgmp */
  #define __GMP_DECLSPEC  __GMP_DECLSPEC_EXPORT
  #else
  /* compiling to go into an application which will link to a DLL libgmp */
  #define __GMP_DECLSPEC  __GMP_DECLSPEC_IMPORT
  #endif
  #else
  /* all other cases */
  #define __GMP_DECLSPEC
  #endif

If you didn't like the symbol "MAIN", then we can use any other
symbol, like BUILDING_GMAKE or whatever.  But having this split in two
is not something I'd recommend.  I think it's bad for maintenance, if
nothing else.

> There will be more disruption I think.

Looking forward to it ;-)

> One other thing: I changed the pump function to read from a FD but write
> to a FILE*, because all our other uses of stdout/stderr use FILE* and
> it's not wise to mix them.  It works fine.  While I was in there I
> noticed the handling of the text/binary mode.  I wonder if this is not
> quite right.  It seems to me that since we're going to be writing to
> stdout/stderr anyway, if we're going to set the mode at all we should be
> setting the mode on the temporary file to match the mode on
> stdout/stderr, before we write to it, rather than setting the mode on
> stdout/stderr to match the temporary file.
> 
> What do you think?

Make never changes the I/O mode of its standard streams.  And it
shouldn't, since everything Make itself writes or reads is always
plain text.  Since the standard streams always start in text mode,
your suggestion boils down to make input from the temporary file be
always in text mode.

That wouldn't be right.  The issue here is that Make has no idea what
mode will be used by its children.  We redirect the children's
standard streams to a file, but we cannot force them to use this or
that mode (nor should we, because that is up to the child program).
Since we have no idea, and we must copy to our stdout/stderr
everything the child wrote, we must assume the worst.  And the worst
is that the child did use binary mode, and as result wrote there some
bytes that cannot be read in text mode without corrupting child's
output.  Examples include the ^Z byte, which stops text-mode reads and
writes, and lone CR characters that get stripped by text-mode reads to
disappear without a trace.

So we use binary mode to read from the temporary file, because that's
the only way to guarantee that everything the child wrote will be read
verbatim.  We then need to write that stuff to our output in the same
mode, to produce identical output, as if the intermediate temporary
file never happened.

Does this make sense?  If you want a practical example to illustrate
this conundrum, just ask.