[v3,4/4] build: Move -Werror-implicit-function-declaration from make to build.sh
diff mbox series

Message ID 20190806164523.557-1-pvorel@suse.cz
State Accepted
Headers show
Series
  • Untitled series #123618
Related show

Commit Message

Petr Vorel Aug. 6, 2019, 4:45 p.m. UTC
Previously it was passed only to Android build. Generally Werror flags
should be on for development but disabled for releases and production.
We don't have any configure flag stating development build, so using it
in build.sh should be sufficient as it can be used in both Travis CI
builds and local development.

+ respect CC variable in build.sh (for these lazy to pass it via -c flag)

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Change v2->v3:
* move flag to build.sh, so it can be reused for local development
* respect CC

 build.sh               | 4 +++-
 include/mk/env_post.mk | 4 ----
 2 files changed, 3 insertions(+), 5 deletions(-)

Comments

Cyril Hrubis Aug. 9, 2019, 9:33 a.m. UTC | #1
Hi!
> Previously it was passed only to Android build. Generally Werror flags
> should be on for development but disabled for releases and production.
> We don't have any configure flag stating development build, so using it
> in build.sh should be sufficient as it can be used in both Travis CI
> builds and local development.
> 
> + respect CC variable in build.sh (for these lazy to pass it via -c flag)
> 
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Change v2->v3:
> * move flag to build.sh, so it can be reused for local development
> * respect CC
> 
>  build.sh               | 4 +++-
>  include/mk/env_post.mk | 4 ----
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/build.sh b/build.sh
> index 3da2adf55..78845bf95 100755
> --- a/build.sh
> +++ b/build.sh
> @@ -9,6 +9,9 @@
>  
>  set -e
>  
> +CFLAGS="${CFLAGS:--Werror-implicit-function-declaration}"

Shouldn't this be -Werror=implicit-function-declaration ?

Hmm, looks like both variants actually work, but the gcc manual speaks
only about -Werror= one.


Otherwise it looks good, acked.
Petr Vorel Aug. 9, 2019, 10:07 a.m. UTC | #2
Hi Cyril,

> > +CFLAGS="${CFLAGS:--Werror-implicit-function-declaration}"

> Shouldn't this be -Werror=implicit-function-declaration ?
Yes, it is. '${CFLAGS:-' part is POSIX shell substitute (${parameter:-[word]}
from [1]), the result is '-Werror=implicit-function-declaration' when CFLAGS is
empty or not set.
I might change it as

I might change it to more readable form:
DEFAULT_CFLAGS="-Werror-implicit-function-declaration"
CFLAGS="${CFLAGS:-$DEFAULT_CFLAGS}"

> Hmm, looks like both variants actually work, but the gcc manual speaks
> only about -Werror= one.


> Otherwise it looks good, acked.
Thx! Although I plan to send some RFC for moving ffsb as a subproject, it has a
low priority. So I'll merge this with your ack.

Kind regards,
Petr

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
Cyril Hrubis Aug. 9, 2019, 10:35 a.m. UTC | #3
Hi!
> > Shouldn't this be -Werror=implicit-function-declaration ?
> Yes, it is. '${CFLAGS:-' part is POSIX shell substitute (${parameter:-[word]}
> from [1]), the result is '-Werror=implicit-function-declaration' when CFLAGS is
> empty or not set.
> I might change it as

I meant the = after -Werror vs -.
Petr Vorel Aug. 9, 2019, 1:57 p.m. UTC | #4
Hi Cyril,

> > > Shouldn't this be -Werror=implicit-function-declaration ?
> > Yes, it is. '${CFLAGS:-' part is POSIX shell substitute (${parameter:-[word]}
> > from [1]), the result is '-Werror=implicit-function-declaration' when CFLAGS is
> > empty or not set.
> > I might change it as

> I meant the = after -Werror vs -.
Sure (I'm blind today). Changed to more common
-Werror=implicit-function-declaration and merged.

Kind regards,
Petr

Patch
diff mbox series

diff --git a/build.sh b/build.sh
index 3da2adf55..78845bf95 100755
--- a/build.sh
+++ b/build.sh
@@ -9,6 +9,9 @@ 
 
 set -e
 
+CFLAGS="${CFLAGS:--Werror-implicit-function-declaration}"
+CC="${CC:-gcc}"
+
 DEFAULT_PREFIX="$HOME/ltp-install"
 DEFAULT_BUILD="native"
 DEFAULT_TREE="in"
@@ -16,7 +19,6 @@  CONFIGURE_OPTS_IN_TREE="--with-open-posix-testsuite --with-realtime-testsuite"
 # TODO: open posix testsuite is currently broken in out-tree-build. Enable it once it's fixed.
 CONFIGURE_OPTS_OUT_TREE="--with-realtime-testsuite"
 MAKE_OPTS="-j$(getconf _NPROCESSORS_ONLN)"
-CC=gcc
 
 build_32()
 {
diff --git a/include/mk/env_post.mk b/include/mk/env_post.mk
index 913bdf5d1..f4169ad66 100644
--- a/include/mk/env_post.mk
+++ b/include/mk/env_post.mk
@@ -42,10 +42,6 @@  CPPFLAGS			+= -D__UCLIBC__ -DUCLINUX
 endif
 
 ifeq ($(ANDROID),1)
-# There are many undeclared functions, it's best not to accidentally overlook
-# them.
-CFLAGS				+= -Werror-implicit-function-declaration
-
 LDFLAGS				+= -L$(top_builddir)/lib/android_libpthread
 LDFLAGS				+= -L$(top_builddir)/lib/android_librt
 endif