Hi Olivier, thank you for your feedback , more comments below :) Thanks. Mario. ________________________________________ From: Olivier MATZ [olivier.m...@6wind.com] Sent: Friday, October 16, 2015 12:29 PM To: Arevalo, Mario Alfredo C; dev at dpdk.org Cc: Panu Matilainen Subject: Re: [PATCH v4 1/7] mk: Add rule for installing headers
Hi Mario, Thank you for this patch series, and thank you Panu for the good comments on this series. Please see some comments below. On 10/05/2015 10:20 PM, Mario Carrillo wrote: > Add hierarchy-file support to the DPDK headers. > > When invoking "make install-headers" headers will > be installed in: $(DESTDIR)/$(INCLUDE_DIR) > where INCLUDE_DIR=/usr/include/dpdk by default. > > You can override INCLUDE_DIR var. > This hierarchy is based on: > http://www.freedesktop.org/software/systemd/man/file-hierarchy.html > > Signed-off-by: Mario Carrillo <mario.alfredo.c.arevalo at intel.com> > --- > mk/rte.sdkinstall.mk | 18 +++++++++++++++++- > mk/rte.sdkroot.mk | 4 ++-- > 2 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk > index 86c98a5..f016171 100644 > --- a/mk/rte.sdkinstall.mk > +++ b/mk/rte.sdkinstall.mk > @@ -41,6 +41,10 @@ endif > # x86_64-native-*-gcc > ifndef T > T=* > +ifneq (,$(wildcard $(BUILD_DIR)/build/.config)) What about using $(RTE_OUTPUT) here instead of $(BUILD_DIR)/build ? With you patch, the following commands work: make config T=x86_64-native-linuxapp-gcc make -j32 make install-headers DESTDIR=install Replacing all the occurences of $(BUILD_DIR)/build by $(RTE_OUTPUT) would also make the following commands work: make config T=x86_64-native-linuxapp-gcc O=build2 make -j32 O=build2 make install-headers O=build2 DESTDIR=install2 Note: the default value of RTE_OUTPUT is set to $(RTE_SRCDIR)/build in mk/rte.sdkroot.mk We also need to to replace other occurences of $(BUILD_DIR), please see my comment on patch 2/7. [Mario]: Yes, you right, I can replace it :) > +INCLUDE_DIR ?= /usr/include/dpdk > +HSLINKS := $(wildcard $(RTE_OUTPUT)/include/*) > +endif > endif > > # > @@ -72,7 +76,19 @@ install: $(INSTALL_TARGETS) > echo "Using local configuration"; \ > fi > $(Q)$(MAKE) all O=$(BUILD_DIR)/$* > - > +# > +# install headers in /usr/include/dpdk by default > +# INCLUDE_DIR can be overridden. > +# > +.PHONY: install-headers > +install-headers: > + @echo ================== Installing headers; > + @[ -d $(DESTDIR)/$(INCLUDE_DIR) ] || mkdir -p $(DESTDIR)/$(INCLUDE_DIR) I think it's useless to do the [ -d $(DESTDIR)/$(INCLUDE_DIR) ] as the 'mkdir -p' will do it as well. But maybe it could be useful to check: [ "$${HSLINKS}" != "" ] This would solve the issue described by Panu about the directories created even if they are empty. [Mario]: Sounds good, that can solve the problem. > + @for HSLINK in ${HSLINKS}; do \ Not sure to understand what is the meaning of HSLINK? HS = headers? [Mario]: HSLINKS= headers links, in dpdk installation, it creates links to headers, however for installation I would like to get the headers paths and copy them to installation path, not create links :) . Regards, Olivier
