| Message ID | F9C551623D2CBB4C9488801D14F864C663264BB0@ex-mb1.corp.adtran.com |
|---|---|
| State | Accepted |
| Headers | show |
On 18/11/13 21:02, ANDY KENNEDY wrote: > When building pciutils as shared, also include the static libraries for > a more rounded staging directory (useful when PREFER_STATIC_LIB is not > set, but should provide a static library for when static linking with > libpci is preferred). > > Signed-off-by: Andy Kennedy <andy.kennedy@adtran.com> > --- > diff -Naur a/package/pciutils/pciutils.mk b/package/pciutils/pciutils.mk > --- a/package/pciutils/pciutils.mk 2013-09-17 06:42:07.000000000 -0500 > +++ b/package/pciutils/pciutils.mk 2013-11-18 13:49:31.000000000 -0600 > @@ -17,7 +17,26 @@ > PCIUTILS_ZLIB=no > endif > PCIUTILS_DNS=no > +ifeq ($(BR2_PREFER_STATIC_LIB),y) > +PCIUTILS_SHARED=no > +PCIUTILS_DO_SHARED_BUILD = > +else > PCIUTILS_SHARED=yes > +define PCIUTILS_DO_SHARED_BUILD > + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" \ > + HOST="$(KERNEL_ARCH)-linux" \ > + OPT="$(TARGET_CFLAGS)" \ > + LDFLAGS="$(TARGET_LDFLAGS)" \ > + RANLIB=$(TARGET_RANLIB) \ > + AR=$(TARGET_AR) \ > + -C $(PCIUTILS_DIR) \ > + SHARED=yes \ > + ZLIB=$(PCIUTILS_ZLIB) \ > + DNS=$(PCIUTILS_DNS) \ > + LIBKMOD=$(PCIUTILS_KMOD) \ > + PREFIX=/usr This should be refactored so the long list of arguments isn't repeated. E.g. PCIUTILS_MAKE_OPTS = \ CC="$(TARGET_CC)" \ ... define PCIUTILS_DO_SHARED_BUILD $(TARGET_MAKE_ENV) $(MAKE) -C @(D) \ $(PCIUTILS_MAKE_OPTS) SHARED=yes endef > +endef > +endif > > # Build after busybox since it's got a lightweight lspci > ifeq ($(BR2_PACKAGE_BUSYBOX),y) > @@ -47,11 +66,12 @@ > RANLIB=$(TARGET_RANLIB) \ > AR=$(TARGET_AR) \ > -C $(PCIUTILS_DIR) \ > - SHARED=$(PCIUTILS_SHARED) \ > + SHARED=no \ > ZLIB=$(PCIUTILS_ZLIB) \ > DNS=$(PCIUTILS_DNS) \ > LIBKMOD=$(PCIUTILS_KMOD) \ > PREFIX=/usr > + $(PCIUTILS_DO_SHARED_BUILD) > endef > > # Ditch install-lib if SHARED is an option in the future As this comment says, for the static case, install-lib is not necessary for the target install. Wouldn't the install commands have to be done twice as well for staging? Regards, Arnout > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot >
<snip> > > +define PCIUTILS_DO_SHARED_BUILD > > + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" \ > > + HOST="$(KERNEL_ARCH)-linux" \ > > + OPT="$(TARGET_CFLAGS)" \ > > + LDFLAGS="$(TARGET_LDFLAGS)" \ > > + RANLIB=$(TARGET_RANLIB) \ > > + AR=$(TARGET_AR) \ > > + -C $(PCIUTILS_DIR) \ > > + SHARED=yes \ > > + ZLIB=$(PCIUTILS_ZLIB) \ > > + DNS=$(PCIUTILS_DNS) \ > > + LIBKMOD=$(PCIUTILS_KMOD) \ > > + PREFIX=/usr > > This should be refactored so the long list of arguments isn't repeated. > E.g. > Yeah, I had already started that one (with the intent to come back with a V2 patch). I'm in process right now of getting that a bit cleaner. <snip> > > > > # Ditch install-lib if SHARED is an option in the future > > As this comment says, for the static case, install-lib is not necessary > for the target install. Right. That is more stuff that I saw after I had done the initial work. After looking at it more, I wonder if I can refactor the whole file to make it take advantage of autotools as well. It seems that including something like PCIUTIL_MAKE_OPTS+=install-lib may be a cleaner design. > > Wouldn't the install commands have to be done twice as well for staging? No, I don't think so. I wonder if it grabs up the two files at the same time (perhaps with a wildcard of some kind) during the copy. It did pull both into my staging. > > > Regards, > Arnout Thanks for the review. I'll make the changes I've stated above in the next few days and resubmit. Andy
diff -Naur a/package/pciutils/pciutils.mk b/package/pciutils/pciutils.mk --- a/package/pciutils/pciutils.mk 2013-09-17 06:42:07.000000000 -0500 +++ b/package/pciutils/pciutils.mk 2013-11-18 13:49:31.000000000 -0600 @@ -17,7 +17,26 @@ PCIUTILS_ZLIB=no endif PCIUTILS_DNS=no +ifeq ($(BR2_PREFER_STATIC_LIB),y) +PCIUTILS_SHARED=no +PCIUTILS_DO_SHARED_BUILD = +else PCIUTILS_SHARED=yes +define PCIUTILS_DO_SHARED_BUILD + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" \ + HOST="$(KERNEL_ARCH)-linux" \ + OPT="$(TARGET_CFLAGS)" \ + LDFLAGS="$(TARGET_LDFLAGS)" \ + RANLIB=$(TARGET_RANLIB) \ + AR=$(TARGET_AR) \ + -C $(PCIUTILS_DIR) \ + SHARED=yes \ + ZLIB=$(PCIUTILS_ZLIB) \ + DNS=$(PCIUTILS_DNS) \ + LIBKMOD=$(PCIUTILS_KMOD) \ + PREFIX=/usr +endef +endif # Build after busybox since it's got a lightweight lspci ifeq ($(BR2_PACKAGE_BUSYBOX),y) @@ -47,11 +66,12 @@ RANLIB=$(TARGET_RANLIB) \ AR=$(TARGET_AR) \ -C $(PCIUTILS_DIR) \ - SHARED=$(PCIUTILS_SHARED) \ + SHARED=no \ ZLIB=$(PCIUTILS_ZLIB) \ DNS=$(PCIUTILS_DNS) \ LIBKMOD=$(PCIUTILS_KMOD) \ PREFIX=/usr + $(PCIUTILS_DO_SHARED_BUILD) endef # Ditch install-lib if SHARED is an option in the future
When building pciutils as shared, also include the static libraries for a more rounded staging directory (useful when PREFER_STATIC_LIB is not set, but should provide a static library for when static linking with libpci is preferred). Signed-off-by: Andy Kennedy <andy.kennedy@adtran.com> ---