diff mbox

Fix up libstdc++ build breakage with ldbl-extra.ver

Message ID 20120228085045.GX18768@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 28, 2012, 8:50 a.m. UTC 
Hi!

On Mon, Feb 27, 2012 at 03:28:32PM -0800, Benjamin Kosnik wrote:
> On Tue, 28 Feb 2012 00:12:33 +0100
> Jakub Jelinek <jakub@redhat.com> wrote:
> 
> > and $(EGREP) -v '#(#| |$$)' just throws away the whole
> > };# Appended to version file.
> > line.  I wonder if
> > sed -e 's/#[# $].*$//'
> > wouldn't be better, or alternative add ^ before the first #
> > in the egrep regex.  Of course we can add a newline to gnu.ver, but
> > the next time somebody forgots to add a newline at the end of the file
> > we'll have the same problem again.
> > 
> > Benjamin, what do you prefer?
> 
> I would prefer not having gnu.ver have to end in a newline. Slight
> preference for ^ before first # but I don't really care.

Here is a patch.  Just ^ wasn't enough.  Ok for trunk?

2012-02-28  Jakub Jelinek  <jakub@redhat.com>

	* src/Makefile.am (libstdc++-symbols.ver): Only remove comment lines
	if they are at the beginning of lines (with optional whitespace before
	#).
	* src/Makefile.in: Regenerated.


	Jakub

Comments

Paolo Bonzini Feb. 28, 2012, 8:57 a.m. UTC | #1 
Il 28/02/2012 09:50, Jakub Jelinek ha scritto:
> -	$(EGREP) -v '#(#| |$$)' $@.tmp | \
> +	$(EGREP) -v '^[ 	]*#(#| |$$)' $@.tmp | \

I don't know this part very well, so I wonder why you have to remove
comments at all...  hence I wonder if sed 's/##.*//;s/# .*//;s/#$//'
(alternation is not portable in sed) would be closer to the original
intentions.

Anyhow, if the above change works it is surely ok for 4.7.0.

Paolo
Jakub Jelinek Feb. 28, 2012, 9:19 a.m. UTC | #2 
On Tue, Feb 28, 2012 at 09:57:38AM +0100, Paolo Bonzini wrote:
> Il 28/02/2012 09:50, Jakub Jelinek ha scritto:
> > -	$(EGREP) -v '#(#| |$$)' $@.tmp | \
> > +	$(EGREP) -v '^[ 	]*#(#| |$$)' $@.tmp | \
> 
> I don't know this part very well, so I wonder why you have to remove
> comments at all...  hence I wonder if sed 's/##.*//;s/# .*//;s/#$//'
> (alternation is not portable in sed) would be closer to the original
> intentions.

The reason for comment removal is that we pipe this into the preprocessor,
which without the removal spits hundreds of
<stdin>:25:7: error: invalid preprocessing directive #Names
<stdin>:33:7: error: invalid preprocessing directive #std
etc. errors.  Preprocessing directives are only recognized at the
start of the lines, after optional whitespace, so other comments are
just fine.

	Jakub
Paolo Bonzini Feb. 28, 2012, 9:23 a.m. UTC | #3 
Il 28/02/2012 10:19, Jakub Jelinek ha scritto:
>>> > > -	$(EGREP) -v '#(#| |$$)' $@.tmp | \
>>> > > +	$(EGREP) -v '^[ 	]*#(#| |$$)' $@.tmp | \
>> > 
>> > I don't know this part very well, so I wonder why you have to remove
>> > comments at all...  hence I wonder if sed 's/##.*//;s/# .*//;s/#$//'
>> > (alternation is not portable in sed) would be closer to the original
>> > intentions.
> The reason for comment removal is that we pipe this into the preprocessor,
> which without the removal spits hundreds of
> <stdin>:25:7: error: invalid preprocessing directive #Names
> <stdin>:33:7: error: invalid preprocessing directive #std
> etc. errors.  Preprocessing directives are only recognized at the
> start of the lines, after optional whitespace, so other comments are
> just fine.

Ok then!

Paolo
Paolo Carlini Feb. 28, 2012, 10:46 a.m. UTC | #4 
Thanks Jakub for taking care of this.

Paolo.
Rainer Orth March 1, 2012, 6:01 p.m. UTC | #5 
Paolo Carlini <paolo.carlini@oracle.com> writes:

> Thanks Jakub for taking care of this.

Indeed, and sorry for the breakage.  Nothing of the sort turned up on
x86_64-linux testing.

	Rainer
diff mbox

Patch

--- libstdc++-v3/src/Makefile.am	2012-02-27 16:03:11.269648298 +0100
+++ libstdc++-v3/src/Makefile.am	2012-02-28 09:42:02.179585742 +0100
@@ -115,7 +115,7 @@  libstdc++-symbols.ver:  ${glibcxx_srcdir
 	    rm tmp.top tmp.bottom; \
 	  fi; \
 	fi
-	$(EGREP) -v '#(#| |$$)' $@.tmp | \
+	$(EGREP) -v '^[ 	]*#(#| |$$)' $@.tmp | \
 	  $(COMPILE) -E -P -include config.h - > $@ || (rm -f $@ ; exit 1)
 	rm -f $@.tmp
 
--- libstdc++-v3/src/Makefile.in	2012-02-27 16:03:11.269648298 +0100
+++ libstdc++-v3/src/Makefile.in	2012-02-28 09:42:30.126598881 +0100
@@ -776,7 +776,7 @@  vpath % $(top_srcdir)
 @ENABLE_SYMVERS_TRUE@	    rm tmp.top tmp.bottom; \
 @ENABLE_SYMVERS_TRUE@	  fi; \
 @ENABLE_SYMVERS_TRUE@	fi
-@ENABLE_SYMVERS_TRUE@	$(EGREP) -v '#(#| |$$)' $@.tmp | \
+@ENABLE_SYMVERS_TRUE@	$(EGREP) -v '^[ 	]*#(#| |$$)' $@.tmp | \
 @ENABLE_SYMVERS_TRUE@	  $(COMPILE) -E -P -include config.h - > $@ || (rm -f $@ ; exit 1)
 @ENABLE_SYMVERS_TRUE@	rm -f $@.tmp
 @ENABLE_SYMVERS_SUN_TRUE@@ENABLE_SYMVERS_TRUE@libstdc++-symbols.ver-sun : libstdc++-symbols.ver \