* [patch] Remove superfluous /dev/null on grep line
@ 2016-04-06 7:39 Eric Botcazou
2016-04-06 8:50 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2016-04-06 7:39 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++
[-- Attachment #1: Type: text/plain, Size: 1509 bytes --]
Hi,
we recently ran into build failures on Windows systems using a somewhat old
grep, coming from a syntax error in the libstdc++-symbols.ver version file:
# Symbol versioning for shared libraries.
if ENABLE_SYMVERS
libstdc++-symbols.ver: ${glibcxx_srcdir}/$(SYMVER_FILE) \
$(port_specific_symbol_files)
cp ${glibcxx_srcdir}/$(SYMVER_FILE) $@.tmp
chmod +w $@.tmp
if test "x$(port_specific_symbol_files)" != x; then \
if grep '^# Appended to version file.' \
$(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then
\
cat $(port_specific_symbol_files) >> $@.tmp; \
else \
sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
sed -n '/DO NOT DELETE/,$$p' $@.tmp > tmp.bottom; \
cat tmp.top $(port_specific_symbol_files) tmp.bottom > $@.tmp; \
rm tmp.top tmp.bottom; \
fi; \
fi
Note the double /dev/null on the grep command line. The first one causes the
grep to fail when the command is invoked on these systems. That's old code,
but it is now invoked for config/abi/pre/float128.ver on the mainline and 5
branch and this breaks the build on these systems (4.9 builds fine).
This first /dev/null doesn't serve any useful purpose and seems to be a typo,
so the attached patch gets rid of it.
Tested on x86/Windows and x86-64/Linux, OK for mainline and 5 branch?
2016-04-06 Eric Botcazou <ebotcazou@adacore.com>
libstdc++-v3/
* src/Makefile.am (libstdc++-symbols.ver): Remove useless /dev/null.
* src/Makefile.in: Regenerate.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 600 bytes --]
Index: src/Makefile.am
===================================================================
--- src/Makefile.am (revision 234695)
+++ src/Makefile.am (working copy)
@@ -228,7 +228,7 @@ libstdc++-symbols.ver: ${glibcxx_srcdir
chmod +w $@.tmp
if test "x$(port_specific_symbol_files)" != x; then \
if grep '^# Appended to version file.' \
- $(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then \
+ $(port_specific_symbol_files) > /dev/null 2>&1; then \
cat $(port_specific_symbol_files) >> $@.tmp; \
else \
sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Remove superfluous /dev/null on grep line
2016-04-06 7:39 [patch] Remove superfluous /dev/null on grep line Eric Botcazou
@ 2016-04-06 8:50 ` Jonathan Wakely
2016-04-06 9:01 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2016-04-06 8:50 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, libstdc++
On 06/04/16 09:39 +0200, Eric Botcazou wrote:
>Hi,
>
>we recently ran into build failures on Windows systems using a somewhat old
>grep, coming from a syntax error in the libstdc++-symbols.ver version file:
>
># Symbol versioning for shared libraries.
>if ENABLE_SYMVERS
>libstdc++-symbols.ver: ${glibcxx_srcdir}/$(SYMVER_FILE) \
> $(port_specific_symbol_files)
> cp ${glibcxx_srcdir}/$(SYMVER_FILE) $@.tmp
> chmod +w $@.tmp
> if test "x$(port_specific_symbol_files)" != x; then \
> if grep '^# Appended to version file.' \
> $(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then
>\
> cat $(port_specific_symbol_files) >> $@.tmp; \
> else \
> sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
> sed -n '/DO NOT DELETE/,$$p' $@.tmp > tmp.bottom; \
> cat tmp.top $(port_specific_symbol_files) tmp.bottom > $@.tmp; \
> rm tmp.top tmp.bottom; \
> fi; \
> fi
>
>Note the double /dev/null on the grep command line. The first one causes the
>grep to fail when the command is invoked on these systems. That's old code,
>but it is now invoked for config/abi/pre/float128.ver on the mainline and 5
>branch and this breaks the build on these systems (4.9 builds fine).
>
>This first /dev/null doesn't serve any useful purpose and seems to be a typo,
Doesn't it mean that if $port_specific_symbol_files contains only
whitespace we don't hang waiting for input from stdin? The 'if' above
it will be true when "x$port_specific_symbol_files" = "x " or similar.
I don't see any way for that to happen in the FSF tree, so it should
be safe. I'm a bit concerned about making that change this late in
stage 4 though. There isn't much time to find out if it breaks an
obscure target.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Remove superfluous /dev/null on grep line
2016-04-06 8:50 ` Jonathan Wakely
@ 2016-04-06 9:01 ` Jakub Jelinek
2016-04-06 9:12 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2016-04-06 9:01 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Eric Botcazou, gcc-patches, libstdc++
On Wed, Apr 06, 2016 at 09:50:48AM +0100, Jonathan Wakely wrote:
> On 06/04/16 09:39 +0200, Eric Botcazou wrote:
> >we recently ran into build failures on Windows systems using a somewhat old
> >grep, coming from a syntax error in the libstdc++-symbols.ver version file:
> >
> ># Symbol versioning for shared libraries.
> >if ENABLE_SYMVERS
> >libstdc++-symbols.ver: ${glibcxx_srcdir}/$(SYMVER_FILE) \
> > $(port_specific_symbol_files)
> > cp ${glibcxx_srcdir}/$(SYMVER_FILE) $@.tmp
> > chmod +w $@.tmp
> > if test "x$(port_specific_symbol_files)" != x; then \
> > if grep '^# Appended to version file.' \
> > $(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then
> >\
> > cat $(port_specific_symbol_files) >> $@.tmp; \
> > else \
> > sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
> > sed -n '/DO NOT DELETE/,$$p' $@.tmp > tmp.bottom; \
> > cat tmp.top $(port_specific_symbol_files) tmp.bottom > $@.tmp; \
> > rm tmp.top tmp.bottom; \
> > fi; \
> > fi
> >
> >Note the double /dev/null on the grep command line. The first one causes the
> >grep to fail when the command is invoked on these systems. That's old code,
> >but it is now invoked for config/abi/pre/float128.ver on the mainline and 5
> >branch and this breaks the build on these systems (4.9 builds fine).
> >
> >This first /dev/null doesn't serve any useful purpose and seems to be a typo,
>
> Doesn't it mean that if $port_specific_symbol_files contains only
> whitespace we don't hang waiting for input from stdin? The 'if' above
> it will be true when "x$port_specific_symbol_files" = "x " or similar.
>
> I don't see any way for that to happen in the FSF tree, so it should
> be safe. I'm a bit concerned about making that change this late in
> stage 4 though. There isn't much time to find out if it breaks an
> obscure target.
As it is a make variable, can't make be used to test this?
So perhaps
chmod +w $@.tmp
ifneq ($(port_specific_symbol_files),)
if grep '^# Appended to version file.' \
$(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then \
cat $(port_specific_symbol_files) >> $@.tmp; \
else \
sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
sed -n '/DO NOT DELETE/,$$p' $@.tmp > tmp.bottom; \
cat tmp.top $(port_specific_symbol_files) tmp.bottom > $@.tmp; \
rm tmp.top tmp.bottom; \
fi;
endif
? Though, I think the initial and trailing whitespace is removed during
expansion (or already parsing of the vars), so even the
test "x$(port_specific_symbol_files)" != x
check should work right.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Remove superfluous /dev/null on grep line
2016-04-06 9:01 ` Jakub Jelinek
@ 2016-04-06 9:12 ` Jonathan Wakely
2016-04-06 9:17 ` Jakub Jelinek
2016-04-06 15:08 ` Eric Botcazou
0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Wakely @ 2016-04-06 9:12 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Eric Botcazou, gcc-patches, libstdc++
On 06/04/16 11:01 +0200, Jakub Jelinek wrote:
>On Wed, Apr 06, 2016 at 09:50:48AM +0100, Jonathan Wakely wrote:
>> On 06/04/16 09:39 +0200, Eric Botcazou wrote:
>> >we recently ran into build failures on Windows systems using a somewhat old
>> >grep, coming from a syntax error in the libstdc++-symbols.ver version file:
>> >
>> ># Symbol versioning for shared libraries.
>> >if ENABLE_SYMVERS
>> >libstdc++-symbols.ver: ${glibcxx_srcdir}/$(SYMVER_FILE) \
>> > $(port_specific_symbol_files)
>> > cp ${glibcxx_srcdir}/$(SYMVER_FILE) $@.tmp
>> > chmod +w $@.tmp
>> > if test "x$(port_specific_symbol_files)" != x; then \
>> > if grep '^# Appended to version file.' \
>> > $(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then
>> >\
>> > cat $(port_specific_symbol_files) >> $@.tmp; \
>> > else \
>> > sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
>> > sed -n '/DO NOT DELETE/,$$p' $@.tmp > tmp.bottom; \
>> > cat tmp.top $(port_specific_symbol_files) tmp.bottom > $@.tmp; \
>> > rm tmp.top tmp.bottom; \
>> > fi; \
>> > fi
>> >
>> >Note the double /dev/null on the grep command line. The first one causes the
>> >grep to fail when the command is invoked on these systems. That's old code,
>> >but it is now invoked for config/abi/pre/float128.ver on the mainline and 5
>> >branch and this breaks the build on these systems (4.9 builds fine).
>> >
>> >This first /dev/null doesn't serve any useful purpose and seems to be a typo,
>>
>> Doesn't it mean that if $port_specific_symbol_files contains only
>> whitespace we don't hang waiting for input from stdin? The 'if' above
>> it will be true when "x$port_specific_symbol_files" = "x " or similar.
>>
>> I don't see any way for that to happen in the FSF tree, so it should
>> be safe. I'm a bit concerned about making that change this late in
>> stage 4 though. There isn't much time to find out if it breaks an
>> obscure target.
>
>As it is a make variable, can't make be used to test this?
>So perhaps
> chmod +w $@.tmp
>ifneq ($(port_specific_symbol_files),)
> if grep '^# Appended to version file.' \
> $(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then \
> cat $(port_specific_symbol_files) >> $@.tmp; \
> else \
> sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
> sed -n '/DO NOT DELETE/,$$p' $@.tmp > tmp.bottom; \
> cat tmp.top $(port_specific_symbol_files) tmp.bottom > $@.tmp; \
> rm tmp.top tmp.bottom; \
> fi;
>endif
>? Though, I think the initial and trailing whitespace is removed during
>expansion (or already parsing of the vars), so even the
>test "x$(port_specific_symbol_files)" != x
>check should work right.
OK, I have no objection to the original patch then.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Remove superfluous /dev/null on grep line
2016-04-06 9:12 ` Jonathan Wakely
@ 2016-04-06 9:17 ` Jakub Jelinek
2016-04-06 15:08 ` Eric Botcazou
1 sibling, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2016-04-06 9:17 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Eric Botcazou, gcc-patches, libstdc++
On Wed, Apr 06, 2016 at 10:12:18AM +0100, Jonathan Wakely wrote:
> >As it is a make variable, can't make be used to test this?
> >So perhaps
> > chmod +w $@.tmp
> >ifneq ($(port_specific_symbol_files),)
> > if grep '^# Appended to version file.' \
> > $(port_specific_symbol_files) /dev/null > /dev/null 2>&1; then \
> > cat $(port_specific_symbol_files) >> $@.tmp; \
> > else \
> > sed -n '1,/DO NOT DELETE/p' $@.tmp > tmp.top; \
> > sed -n '/DO NOT DELETE/,$$p' $@.tmp > tmp.bottom; \
> > cat tmp.top $(port_specific_symbol_files) tmp.bottom > $@.tmp; \
> > rm tmp.top tmp.bottom; \
> > fi;
> >endif
> >? Though, I think the initial and trailing whitespace is removed during
> >expansion (or already parsing of the vars), so even the
> >test "x$(port_specific_symbol_files)" != x
> >check should work right.
>
> OK, I have no objection to the original patch then.
To correct myself, only leading whitespace is removed, trailing is not,
but when we do care about vars containing only whitespace, that means
removing everything.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Remove superfluous /dev/null on grep line
2016-04-06 9:12 ` Jonathan Wakely
2016-04-06 9:17 ` Jakub Jelinek
@ 2016-04-06 15:08 ` Eric Botcazou
1 sibling, 0 replies; 6+ messages in thread
From: Eric Botcazou @ 2016-04-06 15:08 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: gcc-patches, Jakub Jelinek, libstdc++
> OK, I have no objection to the original patch then.
Thanks, applied. FWIW I verified that the library still builds after the
change with an empty port_specific_symbol_files variable.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-06 15:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-06 7:39 [patch] Remove superfluous /dev/null on grep line Eric Botcazou
2016-04-06 8:50 ` Jonathan Wakely
2016-04-06 9:01 ` Jakub Jelinek
2016-04-06 9:12 ` Jonathan Wakely
2016-04-06 9:17 ` Jakub Jelinek
2016-04-06 15:08 ` Eric Botcazou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).