On Thu, Feb 2, 2017 at 8:17 AM, Andre Przywara wrote: > On 19/01/17 18:46, Chirantan Ekbote wrote: > > The makefile currently automatically detects if certain optional > > libraries are present on the file system and links with them if they > > are. > > > > This isn't always desired though: users may not want to link against > > those libraries even if they do exist. Add new variables that when set > > to 0 will force the corresponding optional library off. > > thanks for the patch, and I support the idea of being able to turn off > libraries. But shouldn't that actually cover all of them? I am thinking > about gtk3 in particular here. > > ... > > > > > Signed-off-by: Chirantan Ekbote <chirantan@xxxxxxxxxxxx> > > --- > > Makefile | 43 +++++++++++++++++++++++++------------------ > > 1 file changed, 25 insertions(+), 18 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index eeb54a4..69daa1e 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -250,26 +250,33 @@ ifeq (y,$(ARCH_HAS_FRAMEBUFFER)) > > endif > > endif > > > > -ifeq ($(call try-build,$(SOURCE_ZLIB),$(CFLAGS),$(LDFLAGS) -lz),y) > > - CFLAGS_DYNOPT += -DCONFIG_HAS_ZLIB > > - LIBS_DYNOPT += -lz > > -else > > - NOTFOUND += zlib > > -endif > > -ifeq ($(call try-build,$(SOURCE_ZLIB),$(CFLAGS),$(LDFLAGS) -lz -static),y) > > - CFLAGS_STATOPT += -DCONFIG_HAS_ZLIB > > - LIBS_STATOPT += -lz > > -endif > > +# Define USE_ZLIB=0 to disable zlib. > > +ifneq ($(USE_ZLIB),0) > > Mmh, that sounds a bit backwards to me, since we don't really have a > USE_ZLIB=1 case. Wouldn't it be more intuitive to either use > DISABLE_ZLIB or WITHOUT_ZLIB? personally, i find negative variables to be bad juju. i spend more time reasoning about things like "if (without_foo)" and "if (!without_foo)" than i would with "if (foo)" and "if (!foo)". so i'd prefer to keep all the variable names positive. if we want to retain the defaults, that's easy with something like: USE_ZLIB ?= y > Or what about using the variable instead of the "y" at the end of that > ifeq? Define USE_ZLIB=y by default, allow overriding on the cmd line, > then: ifeq ($(call try-build..., $(USE_ZLIB)) > Wouldn't that work? That avoids the extra indentation and makes the > patch smaller. i think we were just going with something simple/straightforward due to lack of precedent in this file. if you have a preferred form, switching to that shouldn't be a problem. > Also: I know that I am getting a bit cheeky here, but this Makefile part > is a bit messy already and doesn't really get better with that patch. > Can't we use this opportunity to make this easier and more readable? > Maybe use some wrappers to consolidate the quite similar parts? Or/and > to cover static and non-static with one case instead of two separate calls? or convert it to autotools :D -mike
