You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Martin Sebor <se...@roguewave.com> on 2006/10/13 23:58:00 UTC

Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

sebor@apache.org wrote:
> Author: sebor
> Date: Thu Oct 12 17:49:34 2006
> New Revision: 463535
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=463535
> Log:
> 2006-10-12  Andrew Black  <ab...@roguewave.com>
> 
> 	* makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp): Add
> 	$(TEEOPTS) to compile/link line so that output is routed to log files.

Andrew, I'm afraid there's a problem with this patch. The exit status
of a pipeline is the exit status of the last command so piping stderr
of the compiler (or linker) to tee will cause the pipeline to exit with
the status of tee rather than that of the compiler or linker. That in
turn prevents make from stopping after a compiler or linker error and
allows it to continue processing subsequent commands. I'm going to have
to revert the makefile changes. Can you please look into how else this
could be done?

Martin

Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

Posted by Martin Sebor <se...@roguewave.com>.
Andrew Black wrote:

> Martin Sebor wrote:
> [...]
> 
>>
>>
>> $ echo foo; echo bar | wc -l
>> foo
>> 1
>>
>> Here's a simple experiment that tries to duplicate what you're
>> doing in the patch. AFAICT, it doesn't behave the way we want.
>> What am I missing?
>>
>> Martin
> 
> 
> Ok...
> 
> What happened was I was focusing on the return code propagation, and 
> overlooked the output.  I had assumed that the output was being 
> correctly propagated, as it was being displayed on the console, but this 
> assumption was incorrect.  Attached is a different approach to the 
> solution, where the output is redirected to the target log file, then 
> echoed back to the console (and teed to LOGFILE) after the 
> compile/linking completes.

I'm not completely comfortable with this approach. Often it takes
a while for the compilation of a file to finish, even when there
are errors or other diagnostics. Redirecting the output to a file
and displaying the contents of the file only after translation has
completed would change the perceived behavior of the compiler (or
linker) in these cases. I would have no problem with this in batch
mode but I wouldn't like it in interactive mode.

> 
> A disadvantage of this method is that the output may live in the disk 
> buffer for a time before being flushed.  If the system happens to crash 
> during this time, the log will be lost.

That can happen in any case if the OS goes down.

> If the crash was caused by the 
> compiler or linker, this would lead to a loss of potentially useful 
> information.
> 
> I tried altering things to use a command similar to the following, but 
> the result propagation failed (It seemed to work in bash, but the 
> behavior wasn't quite the same in gmake).
> { $(COMMAND) 2>&1 ; cache=$$?; } | tee $@.log; (exit $$cache)

If it works in the shell it should work in make. If it's not
portable, though (e.g., because it's a Bash extension), it
might be better to avoid it and use the same portable
solution everywhere (e.g., our own implementation of the
pipe under our own command).

Martin

> 
> --Andrew Black
> 
> 
> ------------------------------------------------------------------------
> 
> Index: etc/config/makefile.common
> ===================================================================
> --- etc/config/makefile.common	(revision 465655)
> +++ etc/config/makefile.common	(working copy)
> @@ -141,10 +141,16 @@
>  # file to write log of the build to
>  LOGFILE = /dev/null
>  
> -# if LOGFILE is being created, tee command output into it
> +# file target output is logged to (file used in exec utility)
> +TARGETLOGFILE = $@.log
> +
> +# if LOGFILE is not being created, tee command output into TARGETLOGFILE
> +# otherwise, tee command output into both TARGETLOGFILE and LOGFILE.
>  # IMPORTANT: $(TEEOPTS) must be last on the command line
> -ifneq ($(LOGFILE),/dev/null)
> -  TEEOPTS = 2>&1 | tee -a $(LOGFILE)
> +ifeq ($(LOGFILE),/dev/null)
> +  TEEOPTS = >$(TARGETLOGFILE) 2>&1 ; cache=$$?; cat $(TARGETLOGFILE); (exit $$cache)
> +else
> +  TEEOPTS = >$(TARGETLOGFILE) 2>&1 ; cache=$$?; cat $(TARGETLOGFILE) | tee -a $(LOGFILE); (exit $$cache)
>  endif
>  
>  # determine the name of the results file against which to compute regressions
> Index: etc/config/makefile.rules
> ===================================================================
> --- etc/config/makefile.rules	(revision 465655)
> +++ etc/config/makefile.rules	(working copy)
> @@ -60,16 +60,16 @@
>      ifneq ($(AS_EXT),".")
>  
>  %.o: %$(AS_EXT)
> -	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
> +	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
>  
>      endif   # ifneq ($(AS_EXT),".")
>    endif   # ifneq ($(AS_EXT),)
>  
>  %.o: %.cpp
> -	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
> +	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
>  
>  %: %.o
> -	$(LD) $< -o $@ $(LDFLAGS) $(LDLIBS)
> +	$(LD) $< -o $@ $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
>  
>  # disable compilation and linking in the same step
>  # %: %.cpp
> @@ -78,7 +78,7 @@
>  
>  # compile and link in one step to eliminate the space overhead of .o files
>  %: %.cpp
> -	$(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS)
> +	$(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
>  
>  endif   # eq ($(NO_DOT_O),)
>  


Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

Posted by Andrew Black <ab...@roguewave.com>.
Martin Sebor wrote:
[...]
> 
> 
> $ echo foo; echo bar | wc -l
> foo
> 1
> 
> Here's a simple experiment that tries to duplicate what you're
> doing in the patch. AFAICT, it doesn't behave the way we want.
> What am I missing?
> 
> Martin

Ok...

What happened was I was focusing on the return code propagation, and 
overlooked the output.  I had assumed that the output was being 
correctly propagated, as it was being displayed on the console, but this 
assumption was incorrect.  Attached is a different approach to the 
solution, where the output is redirected to the target log file, then 
echoed back to the console (and teed to LOGFILE) after the 
compile/linking completes.

A disadvantage of this method is that the output may live in the disk 
buffer for a time before being flushed.  If the system happens to crash 
during this time, the log will be lost.  If the crash was caused by the 
compiler or linker, this would lead to a loss of potentially useful 
information.

I tried altering things to use a command similar to the following, but 
the result propagation failed (It seemed to work in bash, but the 
behavior wasn't quite the same in gmake).
{ $(COMMAND) 2>&1 ; cache=$$?; } | tee $@.log; (exit $$cache)

--Andrew Black

Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

Posted by Martin Sebor <se...@roguewave.com>.
Andrew Black wrote:

> Martin Sebor wrote:
> 
>> Andrew Black wrote:
>>
>> [...]
>>
>>> I believe the attached patch might be an option that doesn't rely on 
>>> bash, though it has a slight inelegant feel to it.  In particular, 
>>> the part reading '(echo $$cache > /dev/null)' is a hack to keep bash 
>>> from losing the cache variable in the pipeline (perhaps 'export 
>>> cache' should be used instead?).
>>
>>
>> I'm not sure I understand what this does. The pipe takes the
>> stdout of the echo $cache >/dev/null command, not that of the
>> prior command. Doesn't that lose the output of the compiler?
>>
>> Martin
> 
> 
> In my (somewhat limited) testing, it does not.  What is happening is 
> that the output of the list of commands is being sent through the 
> pipeline, rather than the output of the last command executed, 
> reflecting the bash documentation on pipelines.  It would be somewhat 
> possible to clarify this behavior by adding {}s around the compile 
> command and the output redirection sequence, but this would result in a 
> minor rethink of how to define redirected commands.


$ echo foo; echo bar | wc -l
foo
1

Here's a simple experiment that tries to duplicate what you're
doing in the patch. AFAICT, it doesn't behave the way we want.
What am I missing?

Martin

Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

Posted by Andrew Black <ab...@roguewave.com>.
Martin Sebor wrote:
> Andrew Black wrote:
> 
> [...]
>> I believe the attached patch might be an option that doesn't rely on 
>> bash, though it has a slight inelegant feel to it.  In particular, the 
>> part reading '(echo $$cache > /dev/null)' is a hack to keep bash from 
>> losing the cache variable in the pipeline (perhaps 'export cache' 
>> should be used instead?).
> 
> I'm not sure I understand what this does. The pipe takes the
> stdout of the echo $cache >/dev/null command, not that of the
> prior command. Doesn't that lose the output of the compiler?
> 
> Martin

In my (somewhat limited) testing, it does not.  What is happening is 
that the output of the list of commands is being sent through the 
pipeline, rather than the output of the last command executed, 
reflecting the bash documentation on pipelines.  It would be somewhat 
possible to clarify this behavior by adding {}s around the compile 
command and the output redirection sequence, but this would result in a 
minor rethink of how to define redirected commands.

The definition of the shell grammar rules can be found at
http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_10

--Andrew Black

> 
>> I have tested this change on Linux, Solaris and HPUX, and it appears 
>> to behave correctly on all three.
>>
>> --Andrew Black
>>
>> Log:
>>     * makefile.common (TEEOPTS): Always tee output to $@.log, prevent 
>> setting of LOGFILE from breaking result capturing. (Reimplementation 
>> of rev 463535, reverted in rev 463850 due to loss of return code from 
>> compile/link commands)
>>     * makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp): 
>> Add $(TEEOPTS) to compile/link line so that output is routed to log 
>> files. (Reverts rev 463850, which reverted rev 463535)
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: etc/config/makefile.common
>> ===================================================================
>> --- etc/config/makefile.common    (revision 464511)
>> +++ etc/config/makefile.common    (working copy)
>> @@ -141,10 +141,13 @@
>>  # file to write log of the build to
>>  LOGFILE = /dev/null
>>  
>> -# if LOGFILE is being created, tee command output into it
>> +# if LOGFILE is not being created, tee command output into target.log
>> +# otherwise, tee command output into both target.log and LOGFILE.
>>  # IMPORTANT: $(TEEOPTS) must be last on the command line
>> -ifneq ($(LOGFILE),/dev/null)
>> -  TEEOPTS = 2>&1 | tee -a $(LOGFILE)
>> +ifeq ($(LOGFILE),/dev/null)
>> +  TEEOPTS = 2>&1 ; cache=$$?; (echo $$cache > /dev/null) | tee 
>> $@.log; (exit $$cache)
>> +else
>> +  TEEOPTS = 2>&1 ; cache=$$?; (echo $$cache > /dev/null) | tee $@.log 
>> | tee -a $(LOGFILE); (exit $$cache)
>>  endif
>>  
>>  # determine the name of the results file against which to compute 
>> regressions
>> Index: etc/config/makefile.rules
>> ===================================================================
>> --- etc/config/makefile.rules    (revision 464511)
>> +++ etc/config/makefile.rules    (working copy)
>> @@ -60,16 +60,16 @@
>>      ifneq ($(AS_EXT),".")
>>  
>>  %.o: %$(AS_EXT)
>> -    $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
>> +    $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
>>  
>>      endif   # ifneq ($(AS_EXT),".")
>>    endif   # ifneq ($(AS_EXT),)
>>  
>>  %.o: %.cpp
>> -    $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
>> +    $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
>>  
>>  %: %.o
>> -    $(LD) $< -o $@ $(LDFLAGS) $(LDLIBS)
>> +    $(LD) $< -o $@ $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
>>  
>>  # disable compilation and linking in the same step
>>  # %: %.cpp
>> @@ -78,7 +78,7 @@
>>  
>>  # compile and link in one step to eliminate the space overhead of .o 
>> files
>>  %: %.cpp
>> -    $(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS)
>> +    $(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS) 
>> $(TEEOPTS)
>>  
>>  endif   # eq ($(NO_DOT_O),)
>>  
>> Index: etc/config/src/libc_decl.sh
>> ===================================================================
>> --- etc/config/src/libc_decl.sh    (revision 464511)
>> +++ etc/config/src/libc_decl.sh    (working copy)
>> @@ -275,7 +275,7 @@
>>          continue
>>      fi
>>  
>> -    if [ "$hdr_base" == math ] ; then
>> +    if [ "$hdr_base" = "math" ] ; then
>>          lib=m
>>      else
>>          lib=c
> 

Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

Posted by Martin Sebor <se...@roguewave.com>.
Andrew Black wrote:

[...]
> I believe the attached patch might be an option that doesn't rely on 
> bash, though it has a slight inelegant feel to it.  In particular, the 
> part reading '(echo $$cache > /dev/null)' is a hack to keep bash from 
> losing the cache variable in the pipeline (perhaps 'export cache' should 
> be used instead?).

I'm not sure I understand what this does. The pipe takes the
stdout of the echo $cache >/dev/null command, not that of the
prior command. Doesn't that lose the output of the compiler?

Martin

> I have tested this change on Linux, Solaris and 
> HPUX, and it appears to behave correctly on all three.
> 
> --Andrew Black
> 
> Log:
>     * makefile.common (TEEOPTS): Always tee output to $@.log, prevent 
> setting of LOGFILE from breaking result capturing. (Reimplementation of 
> rev 463535, reverted in rev 463850 due to loss of return code from 
> compile/link commands)
>     * makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp): Add 
> $(TEEOPTS) to compile/link line so that output is routed to log files. 
> (Reverts rev 463850, which reverted rev 463535)
> 
> 
> ------------------------------------------------------------------------
> 
> Index: etc/config/makefile.common
> ===================================================================
> --- etc/config/makefile.common	(revision 464511)
> +++ etc/config/makefile.common	(working copy)
> @@ -141,10 +141,13 @@
>  # file to write log of the build to
>  LOGFILE = /dev/null
>  
> -# if LOGFILE is being created, tee command output into it
> +# if LOGFILE is not being created, tee command output into target.log
> +# otherwise, tee command output into both target.log and LOGFILE.
>  # IMPORTANT: $(TEEOPTS) must be last on the command line
> -ifneq ($(LOGFILE),/dev/null)
> -  TEEOPTS = 2>&1 | tee -a $(LOGFILE)
> +ifeq ($(LOGFILE),/dev/null)
> +  TEEOPTS = 2>&1 ; cache=$$?; (echo $$cache > /dev/null) | tee $@.log; (exit $$cache)
> +else
> +  TEEOPTS = 2>&1 ; cache=$$?; (echo $$cache > /dev/null) | tee $@.log | tee -a $(LOGFILE); (exit $$cache)
>  endif
>  
>  # determine the name of the results file against which to compute regressions
> Index: etc/config/makefile.rules
> ===================================================================
> --- etc/config/makefile.rules	(revision 464511)
> +++ etc/config/makefile.rules	(working copy)
> @@ -60,16 +60,16 @@
>      ifneq ($(AS_EXT),".")
>  
>  %.o: %$(AS_EXT)
> -	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
> +	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
>  
>      endif   # ifneq ($(AS_EXT),".")
>    endif   # ifneq ($(AS_EXT),)
>  
>  %.o: %.cpp
> -	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $<
> +	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< $(TEEOPTS)
>  
>  %: %.o
> -	$(LD) $< -o $@ $(LDFLAGS) $(LDLIBS)
> +	$(LD) $< -o $@ $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
>  
>  # disable compilation and linking in the same step
>  # %: %.cpp
> @@ -78,7 +78,7 @@
>  
>  # compile and link in one step to eliminate the space overhead of .o files
>  %: %.cpp
> -	$(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS)
> +	$(CXX) $< -o $@ $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) $(LDLIBS) $(TEEOPTS)
>  
>  endif   # eq ($(NO_DOT_O),)
>  
> Index: etc/config/src/libc_decl.sh
> ===================================================================
> --- etc/config/src/libc_decl.sh	(revision 464511)
> +++ etc/config/src/libc_decl.sh	(working copy)
> @@ -275,7 +275,7 @@
>          continue
>      fi
>  
> -    if [ "$hdr_base" == math ] ; then
> +    if [ "$hdr_base" = "math" ] ; then
>          lib=m
>      else
>          lib=c


Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

Posted by Andrew Black <ab...@roguewave.com>.
Martin Sebor wrote:
> Martin Sebor wrote:
>> sebor@apache.org wrote:
>>
>>> Author: sebor
>>> Date: Thu Oct 12 17:49:34 2006
>>> New Revision: 463535
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=463535
>>> Log:
>>> 2006-10-12  Andrew Black  <ab...@roguewave.com>
>>>
>>>     * makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp): Add
>>>     $(TEEOPTS) to compile/link line so that output is routed to log 
>>> files.
>>
>>
>> Andrew, I'm afraid there's a problem with this patch. The exit status
>> of a pipeline is the exit status of the last command so piping stderr
>> of the compiler (or linker) to tee will cause the pipeline to exit with
>> the status of tee rather than that of the compiler or linker. That in
>> turn prevents make from stopping after a compiler or linker error and
>> allows it to continue processing subsequent commands. I'm going to have
>> to revert the makefile changes. Can you please look into how else this
>> could be done?
> 
> FYI, we might be able to use the PIPESTATUS bash special variable
> when the shell is bash. With shells that don't provide equivalent
> functionality we will either need to come up with an alternative
> (e.g., our own implementation of a pipe that returns the exit
> status of the first command instead of the last one), or count
> the warnings only in batch builds and skip that part in interactive
> ones. Are there any other options?
> 
> Martin

Greetings Martin

I believe the attached patch might be an option that doesn't rely on 
bash, though it has a slight inelegant feel to it.  In particular, the 
part reading '(echo $$cache > /dev/null)' is a hack to keep bash from 
losing the cache variable in the pipeline (perhaps 'export cache' should 
be used instead?).  I have tested this change on Linux, Solaris and 
HPUX, and it appears to behave correctly on all three.

--Andrew Black

Log:
     * makefile.common (TEEOPTS): Always tee output to $@.log, prevent 
setting of LOGFILE from breaking result capturing. (Reimplementation of 
rev 463535, reverted in rev 463850 due to loss of return code from 
compile/link commands)
     * makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp): 
Add $(TEEOPTS) to compile/link line so that output is routed to log 
files. (Reverts rev 463850, which reverted rev 463535)

Re: svn commit: r463535 - in /incubator/stdcxx/trunk: etc/config/makefile.common etc/config/makefile.rules util/cmdopt.cpp util/display.cpp util/output.cpp util/runall.cpp util/target.h

Posted by Martin Sebor <se...@roguewave.com>.
Martin Sebor wrote:
> sebor@apache.org wrote:
> 
>> Author: sebor
>> Date: Thu Oct 12 17:49:34 2006
>> New Revision: 463535
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=463535
>> Log:
>> 2006-10-12  Andrew Black  <ab...@roguewave.com>
>>
>>     * makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp): Add
>>     $(TEEOPTS) to compile/link line so that output is routed to log 
>> files.
> 
> 
> Andrew, I'm afraid there's a problem with this patch. The exit status
> of a pipeline is the exit status of the last command so piping stderr
> of the compiler (or linker) to tee will cause the pipeline to exit with
> the status of tee rather than that of the compiler or linker. That in
> turn prevents make from stopping after a compiler or linker error and
> allows it to continue processing subsequent commands. I'm going to have
> to revert the makefile changes. Can you please look into how else this
> could be done?

FYI, we might be able to use the PIPESTATUS bash special variable
when the shell is bash. With shells that don't provide equivalent
functionality we will either need to come up with an alternative
(e.g., our own implementation of a pipe that returns the exit
status of the first command instead of the last one), or count
the warnings only in batch builds and skip that part in interactive
ones. Are there any other options?

Martin