public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two
@ 2017-08-11 15:26 Michał Górny
  2017-08-11 15:26 ` [gentoo-dev] [PATCH 1/3] flag-o-matic.eclass: test-flag-PROG, refactor to reduce duplication Michał Górny
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Michał Górny @ 2017-08-11 15:26 UTC (permalink / raw)
  To: gentoo-dev

Hi, everyone.

I've just reverted the LDFLAGS stripping code I've committed earlier
because it failed hard with clang. Here's an updated patch set that
ensures that clang is going to work fine. Please review.

--
Best regards,
Michał Górny



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [gentoo-dev] [PATCH 1/3] flag-o-matic.eclass: test-flag-PROG, refactor to reduce duplication
  2017-08-11 15:26 [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny
@ 2017-08-11 15:26 ` Michał Górny
  2017-08-11 15:26 ` [gentoo-dev] [PATCH 2/3] flag-o-matic.eclass: test-flag-PROG, ignore unused args in clang Michał Górny
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Michał Górny @ 2017-08-11 15:26 UTC (permalink / raw)
  To: gentoo-dev; +Cc: Michał Górny

---
 eclass/flag-o-matic.eclass | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass
index b2f3742b3ecf..0393a30b74c3 100644
--- a/eclass/flag-o-matic.eclass
+++ b/eclass/flag-o-matic.eclass
@@ -433,11 +433,15 @@ test-flag-PROG() {
 		# Use -c so we can test the assembler as well.
 		-c -o /dev/null
 	)
-	if "${cmdline[@]}" -x${lang} - </dev/null >/dev/null 2>&1 ; then
-		"${cmdline[@]}" "${flag}" -x${lang} - </dev/null >/dev/null 2>&1
+	if "${cmdline[@]}" -x${lang} - </dev/null &>/dev/null ; then
+		cmdline+=( "${flag}" -x${lang} - )
 	else
-		"${cmdline[@]}" "${flag}" -c -o /dev/null /dev/null >/dev/null 2>&1
+		# XXX: what's the purpose of this? does it even work with
+		# any compiler?
+		cmdline+=( "${flag}" -c -o /dev/null /dev/null )
 	fi
+
+	"${cmdline[@]}" </dev/null &>/dev/null
 }
 
 # @FUNCTION: test-flag-CC
-- 
2.14.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [gentoo-dev] [PATCH 2/3] flag-o-matic.eclass: test-flag-PROG, ignore unused args in clang
  2017-08-11 15:26 [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny
  2017-08-11 15:26 ` [gentoo-dev] [PATCH 1/3] flag-o-matic.eclass: test-flag-PROG, refactor to reduce duplication Michał Górny
@ 2017-08-11 15:26 ` Michał Górny
  2017-08-11 15:26 ` [gentoo-dev] [PATCH 3/3] flag-o-matic.eclass: Strip LDFLAGS unsupported by the C compiler, #621274 Michał Górny
  2017-08-25 13:53 ` [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny
  3 siblings, 0 replies; 5+ messages in thread
From: Michał Górny @ 2017-08-11 15:26 UTC (permalink / raw)
  To: gentoo-dev; +Cc: Michał Górny

By default, clang considers unused arguments as error when -Werror is
used. Since flag tests are performed without linking, this causes all
tests for linker flags to fail inadvertently and all those flags
are stripped as a result.

While the correctness of passing unused flags is doubtful, silently
stripping them in a few random packages is certainly not the solution
to the problem, and also makes the results differ between gcc and clang.
To account for that, use clang's -Qunused-arguments option to silence
unused argument warnings.

To avoid wasting time on testing the compiler, just try passing
-Qunused-arguments every time a flag check fails. If clang is not used,
the additional call will fail just the same as the previous one (either
because of the original flag or because of -Qunused-arguments), so
the result will be the same.
---
 eclass/flag-o-matic.eclass   | 9 ++++++++-
 eclass/tests/flag-o-matic.sh | 5 +++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass
index 0393a30b74c3..79866e04a483 100644
--- a/eclass/flag-o-matic.eclass
+++ b/eclass/flag-o-matic.eclass
@@ -441,7 +441,14 @@ test-flag-PROG() {
 		cmdline+=( "${flag}" -c -o /dev/null /dev/null )
 	fi
 
-	"${cmdline[@]}" </dev/null &>/dev/null
+	if ! "${cmdline[@]}" </dev/null &>/dev/null; then
+		# -Werror makes clang bail out on unused arguments as well;
+		# try to add -Qunused-arguments to work-around that
+		# other compilers don't support it but then, it's failure like
+		# any other
+		cmdline+=( -Qunused-arguments )
+		"${cmdline[@]}" </dev/null &>/dev/null
+	fi
 }
 
 # @FUNCTION: test-flag-CC
diff --git a/eclass/tests/flag-o-matic.sh b/eclass/tests/flag-o-matic.sh
index 92c68b82c3c9..5e7ee354bf33 100755
--- a/eclass/tests/flag-o-matic.sh
+++ b/eclass/tests/flag-o-matic.sh
@@ -143,6 +143,11 @@ tbegin "test-flags-CC (gcc-valid but clang-invalid flags)"
 out=$(CC=clang test-flags-CC -finline-limit=1200)
 [[ $? -ne 0 && -z ${out} ]]
 ftend
+
+tbegin "test-flags-CC (unused flags w/clang)"
+out=$(CC=clang test-flags-CC -Wl,-O1)
+[[ $? -eq 0 && ${out} == "-Wl,-O1" ]]
+ftend
 fi
 
 texit
-- 
2.14.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [gentoo-dev] [PATCH 3/3] flag-o-matic.eclass: Strip LDFLAGS unsupported by the C compiler, #621274
  2017-08-11 15:26 [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny
  2017-08-11 15:26 ` [gentoo-dev] [PATCH 1/3] flag-o-matic.eclass: test-flag-PROG, refactor to reduce duplication Michał Górny
  2017-08-11 15:26 ` [gentoo-dev] [PATCH 2/3] flag-o-matic.eclass: test-flag-PROG, ignore unused args in clang Michał Górny
@ 2017-08-11 15:26 ` Michał Górny
  2017-08-25 13:53 ` [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny
  3 siblings, 0 replies; 5+ messages in thread
From: Michał Górny @ 2017-08-11 15:26 UTC (permalink / raw)
  To: gentoo-dev; +Cc: Michał Górny

Include LDFLAGS in the variables stripped by strip-unsupported-flags.
The code reuses the current functions for testing CC, and so only remove
LDFLAGS that are rejected by the C compiler and not the linker. This
solves the case of bug #621274 where LDFLAGS contained GCC-specific
-flto flag.
---
 eclass/flag-o-matic.eclass   | 3 +++
 eclass/tests/flag-o-matic.sh | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass
index 79866e04a483..4e3cfff5afd5 100644
--- a/eclass/flag-o-matic.eclass
+++ b/eclass/flag-o-matic.eclass
@@ -546,6 +546,9 @@ strip-unsupported-flags() {
 	export CXXFLAGS=$(test-flags-CXX ${CXXFLAGS})
 	export FFLAGS=$(test-flags-F77 ${FFLAGS})
 	export FCFLAGS=$(test-flags-FC ${FCFLAGS})
+	# note: this does not verify the linker flags but it is enough
+	# to strip invalid C flags which are much more likely, #621274
+	export LDFLAGS=$(test-flags-CC ${LDFLAGS})
 }
 
 # @FUNCTION: get-flag
diff --git a/eclass/tests/flag-o-matic.sh b/eclass/tests/flag-o-matic.sh
index 5e7ee354bf33..53af9f862c41 100755
--- a/eclass/tests/flag-o-matic.sh
+++ b/eclass/tests/flag-o-matic.sh
@@ -55,7 +55,7 @@ done <<<"
 
 tbegin "strip-unsupported-flags"
 strip-unsupported-flags
-[[ ${CFLAGS} == "" ]] && [[ ${CXXFLAGS} == "-z=2" ]]
+[[ ${CFLAGS} == "" ]] && [[ ${CXXFLAGS} == "-z=2" ]] && [[ ${LDFLAGS} == "" ]]
 ftend
 
 for var in $(all-flag-vars) ; do
-- 
2.14.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two
  2017-08-11 15:26 [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny
                   ` (2 preceding siblings ...)
  2017-08-11 15:26 ` [gentoo-dev] [PATCH 3/3] flag-o-matic.eclass: Strip LDFLAGS unsupported by the C compiler, #621274 Michał Górny
@ 2017-08-25 13:53 ` Michał Górny
  3 siblings, 0 replies; 5+ messages in thread
From: Michał Górny @ 2017-08-25 13:53 UTC (permalink / raw)
  To: gentoo-dev

W dniu pią, 11.08.2017 o godzinie 17∶26 +0200, użytkownik Michał Górny
napisał:
> Hi, everyone.
> 
> I've just reverted the LDFLAGS stripping code I've committed earlier
> because it failed hard with clang. Here's an updated patch set that
> ensures that clang is going to work fine. Please review.
> 

Merged now.

-- 
Best regards,
Michał Górny



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-08-25 13:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-11 15:26 [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny
2017-08-11 15:26 ` [gentoo-dev] [PATCH 1/3] flag-o-matic.eclass: test-flag-PROG, refactor to reduce duplication Michał Górny
2017-08-11 15:26 ` [gentoo-dev] [PATCH 2/3] flag-o-matic.eclass: test-flag-PROG, ignore unused args in clang Michał Górny
2017-08-11 15:26 ` [gentoo-dev] [PATCH 3/3] flag-o-matic.eclass: Strip LDFLAGS unsupported by the C compiler, #621274 Michał Górny
2017-08-25 13:53 ` [gentoo-dev] [PATCH] flag-o-matic.eclass: LDFLAGS stripping, take two Michał Górny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox