From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli To: passt-dev@passt.top Subject: Re: [PATCH] Makefile: Allow define overrides by prepending, not appending, CFLAGS Date: Tue, 20 Sep 2022 22:51:49 +0200 Message-ID: In-Reply-To: <20220914134544.2868226-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1415488027092279811==" --===============1415488027092279811== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Hello everyone, And thanks Stefano for the patch! On Wed, 2022-09-14 at 15:45 +0200, Stefano Brivio wrote: > If we append CFLAGS to the ones passed via command line (if any), > -D options we append will override -D options passed on command line > (if any). > > For example, OpenSUSE build flags include -D_FORTIFY_SOURCE=3, and we > want to have -D_FORTIFY_SOURCE=2, if and only if not overridden. The > current behaviour implies we redefine _FORTIFY_SOURCE as 2, though. > > Instead of appending CFLAGS, prepend them by adding all the default > build flags to another variable, a simply expanded one (defined with > :=), named FLAGS, and pass that *before* CFLAGS in targets, so that > defines from command line can override default flags. > Right. In fact, in openSUSE, we try to use _FORTIFY_SOURCE=3 for all the packages, although opting out is possible, if that causes problem or is undesirable for whatever reason. Point is though, that we would like for the CFLAGS that we set from the project configuration in OBS, to be the ones that are actually used. With this patch, this is exactly what happens, as we can see here: [ 31s] + CFLAGS='-O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 - fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables - fstack-clash-protection -Werror=return-type -flto=auto -g' ... [ 32s] cc -Wall -Wextra -pedantic -std=c99 -D_XOPEN_SOURCE=700 - D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -O2 -pie -fPIE -DPAGE_SIZE=4096 - DNETNS_RUN_DIR=\"/run/netns\" -DPASST_AUDIT_ARCH=AUDIT_ARCH_X86_64 - DRLIMIT_STACK_VAL=8192 -DARCH=\"X86_64\" -DHAS_SND_WND - DHAS_BYTES_ACKED -DHAS_MIN_RTT -DHAS_GETRANDOM -fstack-protector-strong -O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector- strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash- protection -Werror=return-type -flto=auto -g qrap.c -o qrap -flto=auto Which is in fact what I want. So I guess the patch can have... > Reported-by: Dario Faggioli > Signed-off-by: Stefano Brivio > ... Tested-by: Dario Faggioli (If that's useful :-). AFAICS, the patch is not yet committed. I've therefore added it as a downstream one in my passt package on OBS: https://build.opensuse.org/package/show/home:dfaggioli:devel/passt I've also submitted the package to the Virtualization:containers Devel Project: https://build.opensuse.org/request/show/1005013 https://en.opensuse.org/openSUSE:Factory_development_model https://en.opensuse.org/openSUSE:How_to_contribute_to_Factory If/When it's accepted there, I'll proceed and file a request for putting it in "Factory", which will then mean that it will be available in openSUSE Tumbleweed's official repository. Thanks again and Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ ------------------------------------------------------------------- <> (Raistlin Majere) --===============1415488027092279811== Content-Type: application/pgp-signature Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="signature.asc" MIME-Version: 1.0 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUFCQ0FBZEZpRUVTNXNzT2ozVmhy MFdQbk9MRmtKNGlhVzRjKzRGQW1NcUorVUFDZ2tRRmtKNGlhVzQKYys2UkJCQUF4WUppUEdHQUps Nk5TOENicFNrRWpZNHZtdm0yeTAvRGt1ZzBlL1BXU3RCTVFRM0hlaU9LcXBzdQp1cEZ6NXZjdmlk OHhyTUY5M0lrbnEyeVFmdHRvbUpmNnJyWkpXQUpHOHFBWlV2UjYrM0FFNXN0d1FzNmZPZXF3Ck1p NmwrMVJ6MVN5VU9DOHVrSUVldCtlS0JCZnYvYi9odGNiSWJ5Z1BpMWhIb2h0OWhsU1BISytJTmhI TFc4eEgKd3p3ZWJETnBlejhIY2duWFFQeU5wUjJwT2h5bkIwTEM1RmlsZzdzbnJ0M0ZxZnBydzBh NTcyYlloblVxcjhYUApid2NHRG9HRW9iOE4rYjhabVFaaFZkbURYSEF2OTk2cEJqQ3AwR0Nmbzk1 djRJRG9zMTdzd3BwQy9KQXhJOVBsCmdKdEVQbkcrTFFsUnh0cW4ydUtxUVpnWThCT2RFaTVSdUpP bGw5T3BnSjNnMWZ0bi82TXgvS2s1a3lGRDNJVTIKQ2U4OFFnS1FUWkl0UG50aUJuZzJHbmlNMjZq aHB4N3RXWE5jeDVHWTJkQTJONEZENDU5UWl5eEZnY2IrRlozagpldDR0RzB4ZUJHcFN1OFVjRUF6 VEkvZWs2ekhYOEgrdk9lZHJYRmQvbE1CQVM2SFpiem5LWHJWUGFLQVVVQUZtCkl3RzhiLzZieEdh Sy9ldXlUZWUyQkprMXdFR1Y1K3dDaHZJNzF3UXFRQWpZaHE4MUFYWFB3MFhsRE04ZmI3cEwKNGpj OVIvdCtMdEVzLzBzUCtOTHEzTVlBeHVtWXJ1cVFITFFFMEdZVnJNWWJTQXRNNzh3TzBGZXBEMlFw QzRiSQpKWmNySTZLSUlXZmhsVndFa0RjdC9JSUlQZkp5WHZOa3NXR1IrS3BsT1hQS0kvMlhuSTA9 Cj13Q1RJCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo= --===============1415488027092279811==--