| Message ID | 20170119184622.11461-1-chirantan@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Acked-by: Mike Frysinger <vapier@gentoo.org>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Chirantan, 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@chromium.org> > --- > 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? 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. 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? Cheers, Andre. > + 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_AIO),$(CFLAGS),$(LDFLAGS) -laio),y) > - CFLAGS_DYNOPT += -DCONFIG_HAS_AIO > - LIBS_DYNOPT += -laio > -else > - NOTFOUND += aio > + ifeq ($(call try-build,$(SOURCE_ZLIB),$(CFLAGS),$(LDFLAGS) -lz -static),y) > + CFLAGS_STATOPT += -DCONFIG_HAS_ZLIB > + LIBS_STATOPT += -lz > + endif > endif > -ifeq ($(call try-build,$(SOURCE_AIO),$(CFLAGS),$(LDFLAGS) -laio -static),y) > - CFLAGS_STATOPT += -DCONFIG_HAS_AIO > - LIBS_STATOPT += -laio > + > +# Define USE_AIO=0 to disable libaio. > +ifneq ($(USE_AIO),0) > + ifeq ($(call try-build,$(SOURCE_AIO),$(CFLAGS),$(LDFLAGS) -laio),y) > + CFLAGS_DYNOPT += -DCONFIG_HAS_AIO > + LIBS_DYNOPT += -laio > + else > + NOTFOUND += aio > + endif > + ifeq ($(call try-build,$(SOURCE_AIO),$(CFLAGS),$(LDFLAGS) -laio -static),y) > + CFLAGS_STATOPT += -DCONFIG_HAS_AIO > + LIBS_STATOPT += -laio > + endif > endif > > ifeq ($(LTO),1) >
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@chromium.org> > > --- > > 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
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) + 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_AIO),$(CFLAGS),$(LDFLAGS) -laio),y) - CFLAGS_DYNOPT += -DCONFIG_HAS_AIO - LIBS_DYNOPT += -laio -else - NOTFOUND += aio + ifeq ($(call try-build,$(SOURCE_ZLIB),$(CFLAGS),$(LDFLAGS) -lz -static),y) + CFLAGS_STATOPT += -DCONFIG_HAS_ZLIB + LIBS_STATOPT += -lz + endif endif -ifeq ($(call try-build,$(SOURCE_AIO),$(CFLAGS),$(LDFLAGS) -laio -static),y) - CFLAGS_STATOPT += -DCONFIG_HAS_AIO - LIBS_STATOPT += -laio + +# Define USE_AIO=0 to disable libaio. +ifneq ($(USE_AIO),0) + ifeq ($(call try-build,$(SOURCE_AIO),$(CFLAGS),$(LDFLAGS) -laio),y) + CFLAGS_DYNOPT += -DCONFIG_HAS_AIO + LIBS_DYNOPT += -laio + else + NOTFOUND += aio + endif + ifeq ($(call try-build,$(SOURCE_AIO),$(CFLAGS),$(LDFLAGS) -laio -static),y) + CFLAGS_STATOPT += -DCONFIG_HAS_AIO + LIBS_STATOPT += -laio + endif endif ifeq ($(LTO),1)
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. Signed-off-by: Chirantan Ekbote <chirantan@chromium.org> --- Makefile | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-)