[PATCH, v3] Fix stamp rules in libstdc++-v3/include (was: bootstrap failure on powerpc-linux)

[PATCH, v3] Fix stamp rules in libstdc++-v3/include (was: bootstrap failure on powerpc-linux)

[Ralf Wildenhues]

[ moving from
<http://thread.gmane.org/gmane.comp.gcc.devel/97291/focus=97322> ]

Hello Ben,

* Ben Elliston wrote on Mon, Mar 17, 2008 at 08:30:11PM CET:
> > The file was forgotten in the original commit it was fixed by the
> > following revision:
> > r133278 | paolo | 2008-03-16 11:35:44 -0700 (Sun, 16 Mar 2008) | 34 lines
> > Changed paths:
> >    A /trunk/libstdc++-v3/include/std/date_time
> 
> I could have sworn my tree was sufficiently up to date.  Nonetheless, a
> complete rebuild of my tree seems to have worked.  Thanks,

Yep, your tree probably was up to date.  In this case, the build failure
is actually due to broken make rules in libstdc++-v3/include/Makefile.am.

Assume you have an up to date build tree.  You then add another file to
${std_headers} (as happened in your case, due to 'svn up').  Then,
stamp-std is out of date wrt. the new file, but the rule as it is
doesn't update the links, because the stamp-std file already exists:

| stamp-std: ${std_headers}
[...]
| 	if [ ! -f stamp-std ]; then \
| 	  (cd ${std_builddir} && $(LN_S) $? . || true) ;\
| 	fi ;\

Worse even, on a system where LN_S is 'cp -p', the rule doesn't cause
updated headers to propagate to the build tree.

The patch below fixes these two issues.  It also simplifies most stamp
rules a bit more by using separate commands.  Here, calling 'mkdir -p'
unconditionally is actually as efficient as testing for directory
presence first: the mkdir command line contains no shell special
characters, which enables GNU make to directly exec mkdir, bypassing
the shell.

The patch will cause warnings from 'ln -s' upon rebuild whenever the
respective headers in the source tree have changed (I assume this
was the reason for writing the rules the awkward way they were).
This could be "fixed" by either 2>/dev/null (ugly!) or checking for
a $(LN_S_F) in configure.ac so the noise is confined to a few old
systems with an ln that does not grok -s.  Should I do that?

Note that we ignore failure of 'mkdir -p' on purpose, as creating the
directory may have failed due to a parallel make on a system where
'mkdir -p' is not parallel-safe.  (An alternative would be to use
$(mkinstalldirs) but that would currently be slower on all non-GNU
systems.)  (BTW, with BSD make, the previous code could have barfed
out due to its implicit `set -e' when such a race were won.)

I removed one outdated comment which was introduced with
<http://gcc.gnu.org/ml/gcc-cvs/2002-01/msg00351.html> but doesn't any
longer match the code.

Note that the patch below does not fix the stamp-pb rule; it has more
problems.  For fixing it, I'd like to first know whether I may assume
GNU make for this file or whether libstdc++-v3 intends to be buildable
with other make implementations as well?

OK for trunk?

Thanks,
Ralf

libstdc++-v3/ChangeLog:
2008-03-18  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>

	* include/Makefile.am: Fix most stamp rules to regenerate
	the links for all sources newer than the stamp file.
	* include/Makefile.in: Regenerate.

diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index 641e395..92438de 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -855,72 +855,46 @@ allcreated = \
 # Here are the rules for building the headers
 all-local: ${allstamped} ${allcreated}
 
-# This rule is slightly different, in that we must change the name of the
-# local file from std_foo.h to foo.
+# Ignore errors from 'mkdir -p' to avoid parallel make failure on
+# systems with broken mkdir.  Call mkdir unconditionally because
+# it is just as cheap to avoid going through the shell.
+# Ignore errors from $(LN_S) because the links may already exist.
 stamp-std: ${std_headers}
-	@if [ ! -d "${std_builddir}" ]; then \
-	  mkdir -p ${std_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-std ]; then \
-	  (cd ${std_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-std
+	@-mkdir -p ${std_builddir}
+	@-cd ${std_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-std
 
 stamp-bits: ${bits_headers}
-	@if [ ! -d "${bits_builddir}" ]; then \
-	  mkdir -p ${bits_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-bits ]; then \
-	  (cd ${bits_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-bits
+	@-mkdir -p ${bits_builddir}
+	@-cd ${bits_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-bits
 
 stamp-c_base: ${c_base_headers}
-	@if [ ! -d "${c_base_builddir}" ]; then \
-	  mkdir -p ${c_base_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-c_base ]; then \
-	  (cd ${c_base_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-c_base
+	@-mkdir -p ${c_base_builddir}
+	@-cd ${c_base_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-c_base
 
 stamp-c_base_extra: ${c_base_headers_extra}
-	@if [ ! -d "${bits_builddir}" ]; then \
-	  mkdir -p ${bits_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-c_base_extra ]; then \
-	  (cd ${bits_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-c_base_extra
+	@-mkdir -p ${bits_builddir}
+	@-cd ${bits_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-c_base_extra
 
 stamp-c_compatibility: ${c_compatibility_headers_extra}
-	@if [ ! -d "${c_compatibility_builddir}" ]; then \
-	  mkdir -p ${c_compatibility_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-c_compatibility ]; then \
-	  if [ ! -z "${c_compatibility_headers_extra}" ]; then \
-	    (cd ${c_compatibility_builddir} && $(LN_S) $? . || true) ;\
-	  fi ;\
-	fi ;\
-	$(STAMP) stamp-c_compatibility
+	@-mkdir -p ${c_compatibility_builddir}
+	@-if [ ! -z "${c_compatibility_headers_extra}" ]; then \
+	  cd ${c_compatibility_builddir} && $(LN_S) $? . ;\
+	fi
+	@$(STAMP) stamp-c_compatibility
 
 stamp-backward: ${backward_headers}
-	@if [ ! -d "${backward_builddir}" ]; then \
-	  mkdir -p ${backward_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-backward ]; then \
-	  (cd ${backward_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-backward
+	@-mkdir -p ${backward_builddir}
+	@-cd ${backward_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-backward
 
 stamp-ext: ${ext_headers}
-	@if [ ! -d "${ext_builddir}" ]; then \
-	    mkdir -p ${ext_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-ext ]; then \
-	  (cd ${ext_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-ext
+	@-mkdir -p ${ext_builddir}
+	@-cd ${ext_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-ext
 
 # Have to deal with nested include directories, gah! Strip off source
 # directory before making the link.
@@ -974,52 +948,33 @@ stamp-pb:
 	$(STAMP) stamp-pb
 
 stamp-tr1: ${tr1_headers}
-	@if [ ! -d "${tr1_builddir}" ]; then \
-	    mkdir -p ${tr1_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-tr1 ]; then \
-	  (cd ${tr1_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-tr1
+	@-mkdir -p ${tr1_builddir}
+	@-cd ${tr1_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-tr1
 
 stamp-tr1-impl: ${tr1_impl_headers}
-	@if [ ! -d "${tr1_impl_builddir}" ]; then \
-	    mkdir -p ${tr1_impl_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-tr1-impl ]; then \
-	  (cd ${tr1_impl_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-tr1-impl
+	@-mkdir -p ${tr1_impl_builddir}
+	@-cd ${tr1_impl_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-tr1-impl
 
 stamp-debug: ${debug_headers}
-	@if [ ! -d "${debug_builddir}" ]; then \
-	  mkdir -p ${debug_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-debug ]; then \
-	  (cd ${debug_builddir} && @LN_S@ $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-debug
+	@-mkdir -p ${debug_builddir}
+	@-cd ${debug_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-debug
 
 stamp-parallel: ${parallel_headers}
-	@if [ ! -d "${parallel_builddir}" ]; then \
-	  mkdir -p ${parallel_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-parallel ]; then \
-	  (cd ${parallel_builddir} && @LN_S@ $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-parallel
+	@-mkdir -p ${parallel_builddir}
+	@-cd ${parallel_builddir} && $(LN_S) $? .
+	@$(STAMP) stamp-parallel
 
 stamp-${host_alias}:
-	@if [ ! -d ${host_builddir} ]; then \
-	  mkdir -p ${host_builddir} ;\
-	fi ;\
-	$(STAMP) stamp-${host_alias}
+	@-mkdir -p ${host_builddir}
+	@$(STAMP) stamp-${host_alias}
 
 # Host includes static.
 # XXX Missing dependency info for {host_headers_extra}
 stamp-host: ${host_headers} ${host_headers_noinst} stamp-${host_alias}
-	@if [ ! -f stamp-host ]; then \
-	  (cd ${host_builddir} ;\
+	@cd ${host_builddir} && {\
 	  $(LN_S) ${host_headers} . || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(BASIC_FILE_H) basic_file.h || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(ALLOCATOR_H) c++allocator.h || true ;\
@@ -1028,8 +983,8 @@ stamp-host: ${host_headers} ${host_headers_noinst} stamp-${host_alias}
 	  $(LN_S) ${glibcxx_srcdir}/$(CLOCALE_INTERNAL_H) . || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(COMPATIBILITY_H) . || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(CMESSAGES_H) messages_members.h || true ;\
-	  $(LN_S) ${glibcxx_srcdir}/$(CTIME_H) time_members.h || true);\
-	fi ;\
+	  $(LN_S) ${glibcxx_srcdir}/$(CTIME_H) time_members.h || true;\
+	}
 	$(STAMP) stamp-host
 
 # Host includes dynamic.

Re: [PATCH, v3] Fix stamp rules in libstdc++-v3/include

[Ralf Wildenhues]

* Ralf Wildenhues wrote on Tue, Mar 18, 2008 at 11:47:11PM CET:
> <http://thread.gmane.org/gmane.comp.gcc.devel/97291/focus=97322> ]

After reviews from Benjamin and Paolo,
<http://gcc.gnu.org/ml/libstdc++/2008-03/msg00083.html> and
<http://gcc.gnu.org/ml/gcc-patches/2008-04/msg01426.html>,
I've committed this.  The extra regenerated Makefile.in files stem from
a recent commit that causes ../config/override.m4 to be added to
distribution file list.

Cheers,
Ralf

libstdc++-v3:
2008-04-20  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>

	* include/Makefile.am: Fix most stamp rules to regenerate
	the links for all sources newer than the stamp file.
	* include/Makefile.in: Regenerate.
	* Makefile.in: Likewise.
	* src/Makefile.in: Likewise.
	* doc/Makefile.in: Likewise.
	* po/Makefile.in: Likewise.
	* libmath/Makefile.in: Likewise.
	* libsupc++/Makefile.in: Likewise.
	* testsuite/Makefile.in: Likewise.

Index: libstdc++-v3/include/Makefile.am
===================================================================
--- libstdc++-v3/include/Makefile.am	(Revision 134490)
+++ libstdc++-v3/include/Makefile.am	(Arbeitskopie)
@@ -858,72 +858,46 @@
 # Here are the rules for building the headers
 all-local: ${allstamped} ${allcreated}
 
-# This rule is slightly different, in that we must change the name of the
-# local file from std_foo.h to foo.
+# Ignore errors from 'mkdir -p' to avoid parallel make failure on
+# systems with broken mkdir.  Call mkdir unconditionally because
+# it is just as cheap to avoid going through the shell.
+# Ignore errors from $(LN_S) because the links may already exist.
 stamp-std: ${std_headers}
-	@if [ ! -d "${std_builddir}" ]; then \
-	  mkdir -p ${std_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-std ]; then \
-	  (cd ${std_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-std
+	@-mkdir -p ${std_builddir}
+	@-cd ${std_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-std
 
 stamp-bits: ${bits_headers}
-	@if [ ! -d "${bits_builddir}" ]; then \
-	  mkdir -p ${bits_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-bits ]; then \
-	  (cd ${bits_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-bits
+	@-mkdir -p ${bits_builddir}
+	@-cd ${bits_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-bits
 
-stamp-c_base: ${c_base_headers} 
-	@if [ ! -d "${c_base_builddir}" ]; then \
-	  mkdir -p ${c_base_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-c_base ]; then \
-	  (cd ${c_base_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-c_base
+stamp-c_base: ${c_base_headers}
+	@-mkdir -p ${c_base_builddir}
+	@-cd ${c_base_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-c_base
 
 stamp-c_base_extra: ${c_base_headers_extra}
-	@if [ ! -d "${bits_builddir}" ]; then \
-	  mkdir -p ${bits_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-c_base_extra ]; then \
-	  (cd ${bits_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-c_base_extra
+	@-mkdir -p ${bits_builddir}
+	@-cd ${bits_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-c_base_extra
 
 stamp-c_compatibility: ${c_compatibility_headers_extra}
-	@if [ ! -d "${c_compatibility_builddir}" ]; then \
-	  mkdir -p ${c_compatibility_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-c_compatibility ]; then \
-	  if [ ! -z "${c_compatibility_headers_extra}" ]; then \
-	    (cd ${c_compatibility_builddir} && $(LN_S) $? . || true) ;\
-	  fi ;\
-	fi ;\
-	$(STAMP) stamp-c_compatibility
+	@-mkdir -p ${c_compatibility_builddir}
+	@-if [ ! -z "${c_compatibility_headers_extra}" ]; then \
+	  cd ${c_compatibility_builddir} && $(LN_S) $? . 2>/dev/null ;\
+	fi
+	@$(STAMP) stamp-c_compatibility
 
 stamp-backward: ${backward_headers}
-	@if [ ! -d "${backward_builddir}" ]; then \
-	  mkdir -p ${backward_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-backward ]; then \
-	  (cd ${backward_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-backward
+	@-mkdir -p ${backward_builddir}
+	@-cd ${backward_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-backward
 
 stamp-ext: ${ext_headers}
-	@if [ ! -d "${ext_builddir}" ]; then \
-	    mkdir -p ${ext_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-ext ]; then \
-	  (cd ${ext_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-ext
+	@-mkdir -p ${ext_builddir}
+	@-cd ${ext_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-ext
 
 # Have to deal with nested include directories, gah! Strip off source
 # directory before making the link.
@@ -977,52 +951,33 @@
 	$(STAMP) stamp-pb
 
 stamp-tr1: ${tr1_headers}
-	@if [ ! -d "${tr1_builddir}" ]; then \
-	    mkdir -p ${tr1_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-tr1 ]; then \
-	  (cd ${tr1_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-tr1
+	@-mkdir -p ${tr1_builddir}
+	@-cd ${tr1_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-tr1
 
 stamp-tr1-impl: ${tr1_impl_headers}
-	@if [ ! -d "${tr1_impl_builddir}" ]; then \
-	    mkdir -p ${tr1_impl_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-tr1-impl ]; then \
-	  (cd ${tr1_impl_builddir} && $(LN_S) $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-tr1-impl
+	@-mkdir -p ${tr1_impl_builddir}
+	@-cd ${tr1_impl_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-tr1-impl
 
 stamp-debug: ${debug_headers}
-	@if [ ! -d "${debug_builddir}" ]; then \
-	  mkdir -p ${debug_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-debug ]; then \
-	  (cd ${debug_builddir} && @LN_S@ $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-debug
+	@-mkdir -p ${debug_builddir}
+	@-cd ${debug_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-debug
 
 stamp-parallel: ${parallel_headers}
-	@if [ ! -d "${parallel_builddir}" ]; then \
-	  mkdir -p ${parallel_builddir} ;\
-	fi ;\
-	if [ ! -f stamp-parallel ]; then \
-	  (cd ${parallel_builddir} && @LN_S@ $? . || true) ;\
-	fi ;\
-	$(STAMP) stamp-parallel
+	@-mkdir -p ${parallel_builddir}
+	@-cd ${parallel_builddir} && $(LN_S) $? . 2>/dev/null
+	@$(STAMP) stamp-parallel
 
 stamp-${host_alias}:
-	@if [ ! -d ${host_builddir} ]; then \
-	  mkdir -p ${host_builddir} ;\
-	fi ;\
-	$(STAMP) stamp-${host_alias}
+	@-mkdir -p ${host_builddir}
+	@$(STAMP) stamp-${host_alias}
 
 # Host includes static.
 # XXX Missing dependency info for {host_headers_extra}
 stamp-host: ${host_headers} ${host_headers_noinst} stamp-${host_alias}
-	@if [ ! -f stamp-host ]; then \
-	  (cd ${host_builddir} ;\
+	@cd ${host_builddir} && {\
 	  $(LN_S) ${host_headers} . || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(BASIC_FILE_H) basic_file.h || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(ALLOCATOR_H) c++allocator.h || true ;\
@@ -1031,8 +986,8 @@
 	  $(LN_S) ${glibcxx_srcdir}/$(CLOCALE_INTERNAL_H) . || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(COMPATIBILITY_H) . || true ;\
 	  $(LN_S) ${glibcxx_srcdir}/$(CMESSAGES_H) messages_members.h || true ;\
-	  $(LN_S) ${glibcxx_srcdir}/$(CTIME_H) time_members.h || true);\
-	fi ;\
+	  $(LN_S) ${glibcxx_srcdir}/$(CTIME_H) time_members.h || true;\
+	} 2>/dev/null
 	$(STAMP) stamp-host
 
 # Host includes dynamic.

Re: [PATCH, v3] Fix stamp rules in libstdc++-v3/include

[Ralf Wildenhues]

* Ralf Wildenhues wrote on Sun, Apr 20, 2008 at 11:10:37PM CEST:
> After reviews from Benjamin and Paolo,
> <http://gcc.gnu.org/ml/libstdc++/2008-03/msg00083.html> and
> <http://gcc.gnu.org/ml/gcc-patches/2008-04/msg01426.html>,
> I've committed this.  The extra regenerated Makefile.in files stem from
> a recent commit that causes ../config/override.m4 to be added to
> distribution file list.

BTW, is this OK for 4.3 as well?  Asking because this
<http://gcc.gnu.org/ml/gcc-patches/2008-04/msg01412.html>
was OKed for 4.3, and it's kind of awkward to have one but not the
other.

Thanks,
Ralf

> libstdc++-v3:
> 2008-04-20  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>
> 
> 	* include/Makefile.am: Fix most stamp rules to regenerate
> 	the links for all sources newer than the stamp file.
> 	* include/Makefile.in: Regenerate.