Message ID | 20180416193953.19924-5-ps.report@gmx.net |
---|---|
State | Accepted |
Headers | show |
Series | [v3,1/6] squashfs: add license hash | expand |
Peter, All, On 2018-04-16 21:39 +0200, Peter Seiderer spake thusly: > Add patch to split libzstd install target into pc, static, shared > and includes target. Call only the needed ones for the buildroot > staging/target install steps (respect the static/shared configuration). > > Signed-off-by: Peter Seiderer <ps.report@gmx.net> [--SNIP--] > diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk > index 98f8f779aa..6be36cf398 100644 > --- a/package/zstd/zstd.mk > +++ b/package/zstd/zstd.mk [--SNIP--] > @@ -36,15 +37,60 @@ else > ZSTD_OPTS += HAVE_LZ4=0 > endif > > +ifeq ($(BR2_STATIC_LIBS),y) > define ZSTD_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + -C $(@D)/lib libzstd.a > $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > -C $(@D) zstd > endef > +else ifeq ($(BR2_SHARED_LIBS),y) > +define ZSTD_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + -C $(@D)/lib libzstd > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + -C $(@D) zstd > +endef > +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) > +define ZSTD_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + -C $(@D) lib zstd > +endef > +endif I don't like much that we redefine the BUILD_CMDS, INSTALL_TARGET_CMDS, and INSTALL_STAGING_CMDS multiple times under various conditions. When we have that situation, we tend to define additional macros, like so: ifeq ($(BR2_STATIC_LIBS),y) ZSTD_BUILD_LIBS = libzstd.a ZSTD_INSTALL_LIBS = install-static else ifeq ($(BR2_SHARED_LIBS),y) ZSTD_BUILD_LIBS = libzstd ZSTD_INSTALL_LIBS = install-shared else ZSTD_BUILD_LIBS = libzstd.a libzstd ZSTD_INSTALL_LIBS = install-static install-shared endif define ZSTD_BUILD_CMDS $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ -C $(@D)/lib $(ZSTD_BUILD_LIBS) $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ -C $(@D) zstd endef define ZSTD_INSTALL_STAGING_CMDS $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ DESTDIR=$(STAGING_DIR) PREFIX=/usr -C $(@D)/lib \ install-pc install-includes $(ZSTD_INSTALL_LIBS) endef define ZSTD_INSTALL_TARGET_CMDS $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/programs install $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/lib $(ZSTD_INSTALL_LIBS) endef Note: yes, the target install would also install the static libs, but they will be removed in target-finalize anyway. So, given this matches our usual practice, and that it makes the code smaller and easier to read, I'd suggest we go this route. Regards, Yann E. MORIN. > +ifeq ($(BR2_STATIC_LIBS),y) > +define ZSTD_INSTALL_STAGING_CMDS > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + DESTDIR=$(STAGING_DIR) PREFIX=/usr -C $(@D)/lib \ > + install-pc install-static install-includes > +endef > +else ifeq ($(BR2_SHARED_LIBS),y) > +define ZSTD_INSTALL_STAGING_CMDS > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + DESTDIR=$(STAGING_DIR) PREFIX=/usr -C $(@D)/lib \ > + install-pc install-shared install-includes > +endef > +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) > +define ZSTD_INSTALL_STAGING_CMDS > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + DESTDIR=$(STAGING_DIR) PREFIX=/usr -C $(@D)/lib \ > + install > +endef > +endif > + > +ifeq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y) > +define ZSTD_INSTALL_TARGET_CMDS > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/lib install-shared > + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > + DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/programs install > +endef > +else > define ZSTD_INSTALL_TARGET_CMDS > $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ > DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/programs install > endef > +endif > > # note: no 'HAVE_...' options for host library build only > define HOST_ZSTD_BUILD_CMDS > -- > 2.16.3 >
Hello, On Mon, 16 Apr 2018 21:39:52 +0200, Peter Seiderer wrote: > Add patch to split libzstd install target into pc, static, shared > and includes target. Call only the needed ones for the buildroot > staging/target install steps (respect the static/shared configuration). > > Signed-off-by: Peter Seiderer <ps.report@gmx.net> > --- > Changes v2 -> v3: > - split libray install targets for static/shared > (suggested by Yann E. MORIN) Applied to master after implementing the suggestion from Yann: [Thomas: as suggested by Yann E. Morin, refactor the build/install commands to use only one invocation, with intermediate ZSTD_BUILD_LIBS and ZSTD_INSTALL_LIBS variables, defined depending on whether static/shared/static+shared is selected.] Also, did you submit the patch upstream ? Thanks! Thomas
Hello Yann, Thomas, On Wed, 25 Apr 2018 23:48:31 +0200, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello, > > On Mon, 16 Apr 2018 21:39:52 +0200, Peter Seiderer wrote: > > Add patch to split libzstd install target into pc, static, shared > > and includes target. Call only the needed ones for the buildroot > > staging/target install steps (respect the static/shared configuration). > > > > Signed-off-by: Peter Seiderer <ps.report@gmx.net> > > --- > > Changes v2 -> v3: > > - split libray install targets for static/shared > > (suggested by Yann E. MORIN) > > Applied to master after implementing the suggestion from Yann: Thanks for review, suggestions and fixing it on the fly.... > > [Thomas: as suggested by Yann E. Morin, refactor the build/install > commands to use only one invocation, with intermediate ZSTD_BUILD_LIBS > and ZSTD_INSTALL_LIBS variables, defined depending on whether > static/shared/static+shared is selected.] > > Also, did you submit the patch upstream ? Just done, see [1]... Regards, Peter [1] https://github.com/facebook/zstd/pull/1114 > > Thanks! > > Thomas
diff --git a/package/zstd/0001-Split-library-install-target-into-pc-static-shared-a.patch b/package/zstd/0001-Split-library-install-target-into-pc-static-shared-a.patch new file mode 100644 index 0000000000..af9b2bf3f9 --- /dev/null +++ b/package/zstd/0001-Split-library-install-target-into-pc-static-shared-a.patch @@ -0,0 +1,51 @@ +From 2623a12bff19049b6ad5bc066e3ef9c6259d415c Mon Sep 17 00:00:00 2001 +From: Peter Seiderer <ps.report@gmx.net> +Date: Mon, 16 Apr 2018 20:44:49 +0200 +Subject: [PATCH] Split library install target into pc, static, shared and + include only target + +Signed-off-by: Peter Seiderer <ps.report@gmx.net> +--- + lib/Makefile | 15 ++++++++++++--- + 1 file changed, 12 insertions(+), 3 deletions(-) + +diff --git a/lib/Makefile b/lib/Makefile +index cdfdc5c..b592aa6 100644 +--- a/lib/Makefile ++++ b/lib/Makefile +@@ -159,20 +159,29 @@ libzstd.pc: libzstd.pc.in + -e 's|@VERSION@|$(VERSION)|' \ + $< >$@ + +-install: libzstd.a libzstd libzstd.pc ++install: install-pc install-static install-shared install-includes ++ @echo zstd static and shared library installed ++ ++install-pc: libzstd.pc + @$(INSTALL) -d -m 755 $(DESTDIR)$(PKGCONFIGDIR)/ $(DESTDIR)$(INCLUDEDIR)/ + @$(INSTALL_DATA) libzstd.pc $(DESTDIR)$(PKGCONFIGDIR)/ +- @echo Installing libraries ++ ++install-static: libzstd.a ++ @echo Installing static library + @$(INSTALL_DATA) libzstd.a $(DESTDIR)$(LIBDIR) ++ ++install-shared: libzstd ++ @echo Installing shared library + @$(INSTALL_PROGRAM) $(LIBZSTD) $(DESTDIR)$(LIBDIR) + @ln -sf $(LIBZSTD) $(DESTDIR)$(LIBDIR)/libzstd.$(SHARED_EXT_MAJOR) + @ln -sf $(LIBZSTD) $(DESTDIR)$(LIBDIR)/libzstd.$(SHARED_EXT) ++ ++install-includes: + @echo Installing includes + @$(INSTALL_DATA) zstd.h $(DESTDIR)$(INCLUDEDIR) + @$(INSTALL_DATA) common/zstd_errors.h $(DESTDIR)$(INCLUDEDIR) + @$(INSTALL_DATA) deprecated/zbuff.h $(DESTDIR)$(INCLUDEDIR) # prototypes generate deprecation warnings + @$(INSTALL_DATA) dictBuilder/zdict.h $(DESTDIR)$(INCLUDEDIR) +- @echo zstd static and shared library installed + + uninstall: + @$(RM) $(DESTDIR)$(LIBDIR)/libzstd.a +-- +2.16.3 + diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk index 98f8f779aa..6be36cf398 100644 --- a/package/zstd/zstd.mk +++ b/package/zstd/zstd.mk @@ -6,6 +6,7 @@ ZSTD_VERSION = v1.3.3 ZSTD_SITE = $(call github,facebook,zstd,$(ZSTD_VERSION)) +ZSTD_INSTALL_STAGING = YES ZSTD_LICENSE = BSD-3-Clause or GPL-2.0 ZSTD_LICENSE_FILES = LICENSE COPYING @@ -36,15 +37,60 @@ else ZSTD_OPTS += HAVE_LZ4=0 endif +ifeq ($(BR2_STATIC_LIBS),y) define ZSTD_BUILD_CMDS + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + -C $(@D)/lib libzstd.a $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ -C $(@D) zstd endef +else ifeq ($(BR2_SHARED_LIBS),y) +define ZSTD_BUILD_CMDS + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + -C $(@D)/lib libzstd + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + -C $(@D) zstd +endef +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) +define ZSTD_BUILD_CMDS + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + -C $(@D) lib zstd +endef +endif +ifeq ($(BR2_STATIC_LIBS),y) +define ZSTD_INSTALL_STAGING_CMDS + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + DESTDIR=$(STAGING_DIR) PREFIX=/usr -C $(@D)/lib \ + install-pc install-static install-includes +endef +else ifeq ($(BR2_SHARED_LIBS),y) +define ZSTD_INSTALL_STAGING_CMDS + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + DESTDIR=$(STAGING_DIR) PREFIX=/usr -C $(@D)/lib \ + install-pc install-shared install-includes +endef +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) +define ZSTD_INSTALL_STAGING_CMDS + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + DESTDIR=$(STAGING_DIR) PREFIX=/usr -C $(@D)/lib \ + install +endef +endif + +ifeq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y) +define ZSTD_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/lib install-shared + $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ + DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/programs install +endef +else define ZSTD_INSTALL_TARGET_CMDS $(TARGET_MAKE_ENV) $(TARGET_CONFIGURE_OPTS) $(MAKE) $(ZSTD_OPTS) \ DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D)/programs install endef +endif # note: no 'HAVE_...' options for host library build only define HOST_ZSTD_BUILD_CMDS
Add patch to split libzstd install target into pc, static, shared and includes target. Call only the needed ones for the buildroot staging/target install steps (respect the static/shared configuration). Signed-off-by: Peter Seiderer <ps.report@gmx.net> --- Changes v2 -> v3: - split libray install targets for static/shared (suggested by Yann E. MORIN) Changes v1 -> v2: - split off target libzstd support (suggested by Yann E. MORIN) --- ...ry-install-target-into-pc-static-shared-a.patch | 51 ++++++++++++++++++++++ package/zstd/zstd.mk | 46 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 package/zstd/0001-Split-library-install-target-into-pc-static-shared-a.patch