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

Ilya Maximets i.maximets at ovn.org
Thu Oct 3 17:22:56 UTC 2019


On 03.10.2019 15:36, Ben Pfaff wrote:
> On Thu, Oct 03, 2019 at 05:47:29PM +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 at ovn.org>
>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> 
> Hi Ilya!  Thank you for writing this up!
> 
> When I want, e.g., CFLAGS to be set to a particular value by default
> during "make", I usually pass that on the "configure" command line
> directly, like this: "../configure CFLAGS='-Wall".  The configure script
> is smart enough to save the value and spit it out in the Makefile that
> it produces.  You can still override it at "make" time, but the default
> means that you don't have to.
> 
> In this case, it doesn't really matter whether we specify CFLAGS on
> configure or on make, since we're only running make once, but I tend to
> think of this script as setting some kind of example for others.
> 
> So, instead of this kind of change:
> 
>>     configure_script:
>>       - ./boot.sh
>> -    - ./configure CC=${COMPILER} MAKE=gmake OVS_CFLAGS='-Wall' --enable-Werror
>> +    - ./configure CC=${COMPILER} MAKE=gmake --enable-Werror
>>                     || { cat config.log; exit 1; }
>>   
>>     build_script:
>> -    - gmake -j8
>> +    - gmake -j8 CFLAGS="-g -O2 -Wall"
> 
> I would encourage just doing s/OVS_CFLAGS/CFLAGS/ in the "configure"
> line.

OK.  I'll update the cirrus.yml with that change.
Do you think we need to do the same for travis script?
One issue here that we can't normally pass CFLAGS to distcheck via
configure flags. Trying to push them via DISTCHECK_CONFIGURE_FLAGS
might be tricky.

Best regards, Ilya Maximets.


More information about the dev mailing list