diff mbox series

[ovs-dev,v2] ci: Get rid of OVS_CFLAGS in CI scripts.

Message ID 20191004143738.6887-1-i.maximets@ovn.org
State Accepted
Headers show
Series [ovs-dev,v2] ci: Get rid of OVS_CFLAGS in CI scripts. | expand

Commit Message

Ilya Maximets Oct. 4, 2019, 2:37 p.m. UTC
Our CI scripts uses OVS_CFLAGS variable that is intended for internal
usage by 'configure' script only.  Usual CFLAGS should be used instead
to avoid giving bad example to users.

Additionally, '-m32' flag passed directly to CC variable to avoid
splitting it from the compiler invocations and force same API/ABI for
invocations of 'configure' and 'make'.
'BUILD_ENV' dropped as not needed anymore.

Before this patch 'configure' always checked for 64bit libraries
regardless of fact that we're going to build 32bit binary.  This
caused issues if only 64bit version of desired library was available.

Suggested-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---

Version 2:
  * CFLAGS passed to './configure' where possible (except 'distcheck' case).

 .cirrus.yml            |  3 ++-
 .travis.yml            |  2 +-
 .travis/linux-build.sh | 22 +++++++++++++---------
 3 files changed, 16 insertions(+), 11 deletions(-)

Comments

Ben Pfaff Oct. 4, 2019, 5:31 p.m. UTC | #1
On Fri, Oct 04, 2019 at 04:37:38PM +0200, Ilya Maximets wrote:
> Our CI scripts uses OVS_CFLAGS variable that is intended for internal
> usage by 'configure' script only.  Usual CFLAGS should be used instead
> to avoid giving bad example to users.
> 
> Additionally, '-m32' flag passed directly to CC variable to avoid
> splitting it from the compiler invocations and force same API/ABI for
> invocations of 'configure' and 'make'.
> 'BUILD_ENV' dropped as not needed anymore.
> 
> Before this patch 'configure' always checked for 64bit libraries
> regardless of fact that we're going to build 32bit binary.  This
> caused issues if only 64bit version of desired library was available.
> 
> Suggested-by: Ben Pfaff <blp@ovn.org>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
> 
> Version 2:
>   * CFLAGS passed to './configure' where possible (except 'distcheck' case).

Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets Oct. 4, 2019, 6:43 p.m. UTC | #2
On 04.10.2019 19:31, Ben Pfaff wrote:
> On Fri, Oct 04, 2019 at 04:37:38PM +0200, Ilya Maximets wrote:
>> Our CI scripts uses OVS_CFLAGS variable that is intended for internal
>> usage by 'configure' script only.  Usual CFLAGS should be used instead
>> to avoid giving bad example to users.
>>
>> Additionally, '-m32' flag passed directly to CC variable to avoid
>> splitting it from the compiler invocations and force same API/ABI for
>> invocations of 'configure' and 'make'.
>> 'BUILD_ENV' dropped as not needed anymore.
>>
>> Before this patch 'configure' always checked for 64bit libraries
>> regardless of fact that we're going to build 32bit binary.  This
>> caused issues if only 64bit version of desired library was available.
>>
>> Suggested-by: Ben Pfaff <blp@ovn.org>
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>> ---
>>
>> Version 2:
>>    * CFLAGS passed to './configure' where possible (except 'distcheck' case).
> 
> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks! I gave it one more run and applied to master.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/.cirrus.yml b/.cirrus.yml
index 654e63ee0..251f6734d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -20,7 +20,8 @@  freebsd_build_task:
 
   configure_script:
     - ./boot.sh
-    - ./configure CC=${COMPILER} MAKE=gmake OVS_CFLAGS='-Wall' --enable-Werror
+    - ./configure CC=${COMPILER} CFLAGS="-g -O2 -Wall"
+                  MAKE=gmake --enable-Werror
                   || { cat config.log; exit 1; }
 
   build_script:
diff --git a/.travis.yml b/.travis.yml
index b547eb041..eabbad92d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -39,7 +39,7 @@  env:
   - TESTSUITE=1 LIBS=-ljemalloc
   - KERNEL_LIST="5.0  4.20 4.19 4.18 4.17 4.16"
   - KERNEL_LIST="4.15 4.14 4.9  4.4  3.19 3.16"
-  - BUILD_ENV="-m32" OPTS="--disable-ssl"
+  - M32=1 OPTS="--disable-ssl"
   - DPDK=1 OPTS="--enable-shared"
   - DPDK_SHARED=1
   - DPDK_SHARED=1 OPTS="--enable-shared"
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 89496e2a2..758c8235c 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -3,7 +3,7 @@ 
 set -o errexit
 set -x
 
-CFLAGS=""
+CFLAGS_FOR_OVS="-g -O2"
 SPARSE_FLAGS=""
 EXTRA_OPTS="--enable-Werror"
 TARGET="x86_64-native-linuxapp-gcc"
@@ -116,7 +116,8 @@  function install_dpdk()
 
 function configure_ovs()
 {
-    ./boot.sh && ./configure $* || { cat config.log; exit 1; }
+    ./boot.sh
+    ./configure CFLAGS="${CFLAGS_FOR_OVS}" $* || { cat config.log; exit 1; }
 }
 
 function build_ovs()
@@ -147,18 +148,20 @@  if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
     install_dpdk $DPDK_VER
     if [ "$CC" = "clang" ]; then
         # Disregard cast alignment errors until DPDK is fixed
-        CFLAGS="$CFLAGS -Wno-cast-align"
+        CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align"
     fi
 fi
 
 if [ "$CC" = "clang" ]; then
-    export OVS_CFLAGS="$CFLAGS -Wno-error=unused-command-line-argument"
-elif [[ $BUILD_ENV =~ "-m32" ]]; then
-    # Disable sparse for 32bit builds on 64bit machine
-    export OVS_CFLAGS="$CFLAGS $BUILD_ENV"
+    CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-error=unused-command-line-argument"
+elif [ "$M32" ]; then
+    # Not using sparse for 32bit builds on 64bit machine.
+    # Adding m32 flag directly to CC to avoid any posiible issues with API/ABI
+    # difference on 'configure' and 'make' stages.
+    export CC="$CC -m32"
 else
     OPTS="--enable-sparse"
-    export OVS_CFLAGS="$CFLAGS $BUILD_ENV $SPARSE_FLAGS"
+    CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${SPARSE_FLAGS}"
 fi
 
 save_OPTS="${OPTS} $*"
@@ -170,7 +173,8 @@  if [ "$TESTSUITE" ]; then
     configure_ovs
 
     export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
-    if ! make distcheck TESTSUITEFLAGS=-j4 RECHECK=yes; then
+    if ! make distcheck CFLAGS="${CFLAGS_FOR_OVS}" \
+         TESTSUITEFLAGS=-j4 RECHECK=yes; then
         # testsuite.log is necessary for debugging.
         cat */_build/sub/tests/testsuite.log
         exit 1