| Message ID | 1395173894-4811-2-git-send-email-zinosat@tiscali.it |
|---|---|
| State | Superseded |
| Headers | show |
Dear Davide Viti, On Tue, 18 Mar 2014 21:18:14 +0100, Davide Viti wrote: > From: Davide Viti <d.viti@infosolution.it> > > > Signed-off-by: Davide Viti <zinosat@tiscali.it> We probably want the title of the patch to be: eigen: add an option to install unsupported modules > > +if BR2_PACKAGE_EIGEN > + > +config BR2_PACKAGE_EIGEN_UNSUPPORTED_MODULES > + bool "unsupported modules" Indentation is wrong here: there should be one tab before the "bool" statement. > + help > + Install eigen unsupported modules > +endif > + > comment "eigen needs a toolchain w/ C++" > depends on !BR2_INSTALL_LIBSTDCPP > diff --git a/package/eigen/eigen.mk b/package/eigen/eigen.mk > index 5abd464..388dd63 100644 > --- a/package/eigen/eigen.mk > +++ b/package/eigen/eigen.mk > @@ -13,6 +13,14 @@ EIGEN_LICENSE_FILES = COPYING.MPL2 COPYING.BSD COPYING.LGPL COPYING.README > EIGEN_INSTALL_STAGING = YES > EIGEN_INSTALL_TARGET = NO > > +define EIGEN_INSTALL_UNSUPPORTED_MODULES > + cp -a $(@D)/unsupported $(STAGING_DIR)/usr/include/ I'm a bit worried about having a directory called $(STAGING_DIR)/usr/include/unsupported. Shouldn't this directory be installed *below* the $(STAGING_DIR)/usr/include/Eigen directory created by the normal installation of Eigen? > +endef > + > +ifeq ($(BR2_PACKAGE_EIGEN_UNSUPPORTED_MODULES),y) > + EIGEN_POST_INSTALL_STAGING_HOOKS += EIGEN_INSTALL_UNSUPPORTED_MODULES > +endif I believe we normally don't really use post install hooks when there is a definition of the staging install commands. Instead, we use the following idiom: ifeq ($(BR2_PACKAGE_EIGEN_UNSUPPORTED_MODULES),y) define EIGEN_INSTALL_UNSUPPORTED_MODULES_CMDS cp -a ... endef endif define EIGEN_INSTALL_STAGING_CMDS ... existing installation steps ... $(EIGEN_INSTALL_UNSUPPORTED_MODULES_CMDS) endef Thomas
Thomas, Davide, all, On Tue, Mar 18, 2014 at 11:55 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Davide Viti, > > On Tue, 18 Mar 2014 21:18:14 +0100, Davide Viti wrote: >> From: Davide Viti <d.viti@infosolution.it> [...] >> >> +define EIGEN_INSTALL_UNSUPPORTED_MODULES >> + cp -a $(@D)/unsupported $(STAGING_DIR)/usr/include/ > > I'm a bit worried about having a directory called > $(STAGING_DIR)/usr/include/unsupported. Shouldn't this directory be > installed *below* the $(STAGING_DIR)/usr/include/Eigen directory > created by the normal installation of Eigen? This goes a bit against what eigen's build-system (cmake) does by default. The "Eigen" and "unsupported" directories are installed in the same include directory (defaults: /usr/include/eigen3). I agree such a change may break some code in Buildroot users' code, but I'm also worried about not following default install scheme done by upstream projects. Regards,
Hi Samuel and Thomas, I can provide a patch that installs the files under /usr/include/eigen3 and modify my latest patch to install unsupported under that directory too. I guess separate patches are preferred. I can do it later on today. Regards, Davide > Il giorno 19/mar/2014, alle ore 08:25, Samuel Martin <s.martin49@gmail.com> ha scritto: > > Thomas, Davide, all, > > On Tue, Mar 18, 2014 at 11:55 PM, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: >> Dear Davide Viti, >> >>> On Tue, 18 Mar 2014 21:18:14 +0100, Davide Viti wrote: >>> From: Davide Viti <d.viti@infosolution.it> > [...] >>> >>> +define EIGEN_INSTALL_UNSUPPORTED_MODULES >>> + cp -a $(@D)/unsupported $(STAGING_DIR)/usr/include/ >> >> I'm a bit worried about having a directory called >> $(STAGING_DIR)/usr/include/unsupported. Shouldn't this directory be >> installed *below* the $(STAGING_DIR)/usr/include/Eigen directory >> created by the normal installation of Eigen? > > This goes a bit against what eigen's build-system (cmake) does by default. > The "Eigen" and "unsupported" directories are installed in the same > include directory > (defaults: /usr/include/eigen3). > > I agree such a change may break some code in Buildroot users' code, > but I'm also worried about not following default install scheme done > by upstream projects. > > > Regards, > > -- > Samuel
Dear Davide Viti, On Wed, 19 Mar 2014 09:07:10 +0100, Davide Viti wrote: > I can provide a patch that installs the files under > /usr/include/eigen3 and modify my latest patch to install unsupported > under that directory too. Seems like a good choice to me. > I guess separate patches are preferred. Great, thanks! Thomas
diff --git a/package/eigen/Config.in b/package/eigen/Config.in index e94f9a3..a653e37 100644 --- a/package/eigen/Config.in +++ b/package/eigen/Config.in @@ -13,5 +13,13 @@ config BR2_PACKAGE_EIGEN http://eigen.tuxfamily.org/ +if BR2_PACKAGE_EIGEN + +config BR2_PACKAGE_EIGEN_UNSUPPORTED_MODULES + bool "unsupported modules" + help + Install eigen unsupported modules +endif + comment "eigen needs a toolchain w/ C++" depends on !BR2_INSTALL_LIBSTDCPP diff --git a/package/eigen/eigen.mk b/package/eigen/eigen.mk index 5abd464..388dd63 100644 --- a/package/eigen/eigen.mk +++ b/package/eigen/eigen.mk @@ -13,6 +13,14 @@ EIGEN_LICENSE_FILES = COPYING.MPL2 COPYING.BSD COPYING.LGPL COPYING.README EIGEN_INSTALL_STAGING = YES EIGEN_INSTALL_TARGET = NO +define EIGEN_INSTALL_UNSUPPORTED_MODULES + cp -a $(@D)/unsupported $(STAGING_DIR)/usr/include/ +endef + +ifeq ($(BR2_PACKAGE_EIGEN_UNSUPPORTED_MODULES),y) + EIGEN_POST_INSTALL_STAGING_HOOKS += EIGEN_INSTALL_UNSUPPORTED_MODULES +endif + # This package only consists of headers that need to be # copied over to the sysroot for compile time use define EIGEN_INSTALL_STAGING_CMDS