dimkr / loksh

A Linux port of OpenBSD's ksh
115 stars 6 forks source link

Add 'override' keyword before adding to CFLAGS #6

Closed jsarenik closed 7 years ago

jsarenik commented 7 years ago

There is an important command line argument adding directory '.' to be searched for header files.

$ make -n | grep -m1 ^cc
cc -O2 -I. -D_GNU_SOURCE -std=gnu99 -Wall -pedantic -Wno-format-security \
   -Wno-pointer-sign   -c -o alloc.o alloc.c

Without this patch, whenever CFLAGS are set by a command line argument, the include directory (along with other options) is missing:

$ make CFLAGS="-s" -n | grep -m1 ^cc
cc -s   -c -o alloc.o alloc.c

After this patch:

$ make CFLAGS="-s" -n | grep -m1 ^cc
cc -s -I. -D_GNU_SOURCE -std=gnu99 -Wall -pedantic -Wno-format-security \
   -Wno-pointer-sign   -c -o alloc.o alloc.c

The rationale is that when someone wants to add custom CFLAGS and is compiling loksh with musl libc, it is much easier to just add those CFLAGS to the existing set. When someone wants to debug and redefine CFLAGS completely, they will have to delete word "override" in the Makefile now. The logic before applying this patch was reversed and the change required to be done in Makefile was not as obvious as now.

https://www.gnu.org/software/make/manual/html_node/Override-Directive.html#Override-Directive

dimkr commented 7 years ago

I learned something new today. Thanks!