On Tue, Jul 26, 2022 at 11:57:04PM +0600, NRK wrote: > On Mon, Jul 04, 2022 at 03:01:35PM +0600, NRK wrote: > > currently, doing something like the following would fail to build: > > > > $ make CFLAGS="-O3 -march=native" > > > > this is because CPPFLAGS get mangled with CFLAGS and so changing CFLAGS > > kills the build. > > > > similarly it's conventional to have library flags in LDLIBS, so that > > user can change LDFLAGS to add "-flto" or other flags they might want > > without the build unnecessarily failing.
This is not the reason the two variables exist; they have a different meaning. LDLIBS is for libraries (eg. -lfoo /path/to/bar.a), which are input files to be linked like objects, while LDFLAGS are for flags/options (eg. -Lpath -Wl,--as-needed). Options goes before input files; order matters for input files but not for (most) flags. So the traditional link command is $(LD) $(LDFLAGS) $(OBJECTS) $(LDLIBS) Users may want to override LDFLAGS, or LDLIBS, or both in some situations. So its unrelated to user overrides. And I think it's the same for CPPFLAGS vs CFLAGS: the distinction is there for makefiles that run the preprocessor separately. (What I like to do in my makefiles, is to first define default options that are just preferences with "?=" (eg. CFLAGS ?= -O2 -march=native) and then add the important options with "+=" (eg. CFLAGS += -I. -std=c89), so a user can override only the defaults with "CFLAGS=... make" or override the whole thing with "make CFLAGS=...". Here, it does not matter much I think, since the user is supposed to edit config.mk to his liking.) > > --- > > > > P.S: Could also drop `-std=c99` from CFLAGS and set CC to c99. But > > didn't do that since it wasn't needed for my goal of being able to do > > something like the following: > > > > $ make CFLAGS="-O3 -flto" LDFLAGS="-O3 -flto" > > > > Makefile | 8 +++++--- > > config.mk | 6 +++--- > > 2 files changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index a03a95c..8d25ad0 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -11,11 +11,13 @@ all: options dmenu stest > > options: > > @echo dmenu build options: > > @echo "CFLAGS = $(CFLAGS)" > > + @echo "CPPFLAGS = $(CPPFLAGS)" > > @echo "LDFLAGS = $(LDFLAGS)" > > + @echo "LDLIBS = $(LDLIBS)" > > @echo "CC = $(CC)" > > > > .c.o: > > - $(CC) -c $(CFLAGS) $< > > + $(CC) -c $(CPPFLAGS) $(CFLAGS) $< > > > > config.h: > > cp config.def.h $@ > > @@ -23,10 +25,10 @@ config.h: > > $(OBJ): arg.h config.h config.mk drw.h > > > > dmenu: dmenu.o drw.o util.o > > - $(CC) -o $@ dmenu.o drw.o util.o $(LDFLAGS) > > + $(CC) -o $@ dmenu.o drw.o util.o $(LDFLAGS) $(LDLIBS) > > I would put LDFLAGS before the object files. > > stest: stest.o > > - $(CC) -o $@ stest.o $(LDFLAGS) > > + $(CC) -o $@ stest.o $(LDFLAGS) $(LDLIBS) > > (same here) > > clean: > > rm -f dmenu stest $(OBJ) dmenu-$(VERSION).tar.gz > > diff --git a/config.mk b/config.mk > > index b0bd246..a513b74 100644 > > --- a/config.mk > > +++ b/config.mk > > @@ -24,9 +24,9 @@ INCS = -I$(X11INC) -I$(FREETYPEINC) > > LIBS = -L$(X11LIB) -lX11 $(XINERAMALIBS) $(FREETYPELIBS) > > > > # flags > > -CPPFLAGS = -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE=700 > > -D_POSIX_C_SOURCE=200809L -DVERSION=\"$(VERSION)\" $(XINERAMAFLAGS) > > -CFLAGS = -std=c99 -pedantic -Wall -Os $(INCS) $(CPPFLAGS) > > -LDFLAGS = $(LIBS) > > +CPPFLAGS = $(INCS) -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE=700 > > -D_POSIX_C_SOURCE=200809L -DVERSION=\"$(VERSION)\" $(XINERAMAFLAGS) > > +CFLAGS = -std=c99 -pedantic -Wall -Os > > +LDLIBS = $(LIBS) > > Technically, only the -l flags should go there, the -L ones should go to LDFLAGS. > > # compiler and linker > > CC = cc > > -- > > 2.35.1 > > > > > > Any comment on this? Or any specific reason for deviating from the > conventional approach and mangling `CFLAGS` and `CPPFLAGS`? > > - NRK > Other than my comments on link flags, the patch looks good.
