Message ID | 1426742959-1220-1-git-send-email-steven@uplinklabs.net |
---|---|
State | Accepted |
Headers | show |
Dear Steven Noonan, On Wed, 18 Mar 2015 22:29:19 -0700, Steven Noonan wrote: > diff --git a/package/hwloc/Config.in b/package/hwloc/Config.in > new file mode 100644 > index 0000000..332536b > --- /dev/null > +++ b/package/hwloc/Config.in > @@ -0,0 +1,18 @@ > +config BR2_PACKAGE_HWLOC > + bool "hwloc" > + # libpciaccess dependency > + depends on BR2_LARGEFILE > + # numactl dependency > + depends on BR2_i386 || BR2_mips || BR2_mipsel || \ > + BR2_mips64 || BR2_mips64el || BR2_powerpc || BR2_x86_64 > + select BR2_PACKAGE_LIBPCIACCESS > + select BR2_PACKAGE_NUMACTL The dependencies on libpciaccess and numactl are optional, so I removed them. However, there was a missing dependency on BR2_TOOLCHAIN_HAS_THREADS, since hwloc uses pthread. > + help > + Portable Hardware Locality > + > + Provides a portable abstraction (across OS, versions, architectures, ...) > + of the hierarchical topology of modern architectures, including NUMA > + memory nodes, sockets, shared caches, cores and simultaneous > + multithreading. Lines slightly too long, so I rewrapped. > +################################################################################ > +# > +# hwloc > +# > +################################################################################ > + > +HWLOC_VERSION = 1.10.1 I declared HWLOC_VERSION_MAJOR here.. > +HWLOC_SOURCE = hwloc-$(HWLOC_VERSION).tar.bz2 > +HWLOC_SITE = http://www.open-mpi.org/software/hwloc/v1.10/downloads ... so it can be re-used here. > +HWLOC_DEPENDENCIES = libpciaccess numactl As I said, those dependencies are optional, so I made them as such, and also used explicit --enable/--disable options. This allowed me to discover that the numa support was never enabled with your submission: the numactl package was not installing its library/header file in $(STAGING_DIR). With an explicit --enable-libnuma, hwloc configure script bailed out since it couldn't find the libnuma library. So I committed a patch that installs libnuma to the staging directory as well. Also, I added some explicit --disable options for the things that we don't support for now. Committed with those issues fixed. Thanks, Thomas
On Fri, Mar 20, 2015 at 1:37 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Steven Noonan, > > On Wed, 18 Mar 2015 22:29:19 -0700, Steven Noonan wrote: > >> diff --git a/package/hwloc/Config.in b/package/hwloc/Config.in >> new file mode 100644 >> index 0000000..332536b >> --- /dev/null >> +++ b/package/hwloc/Config.in >> @@ -0,0 +1,18 @@ >> +config BR2_PACKAGE_HWLOC >> + bool "hwloc" >> + # libpciaccess dependency >> + depends on BR2_LARGEFILE >> + # numactl dependency >> + depends on BR2_i386 || BR2_mips || BR2_mipsel || \ >> + BR2_mips64 || BR2_mips64el || BR2_powerpc || BR2_x86_64 >> + select BR2_PACKAGE_LIBPCIACCESS >> + select BR2_PACKAGE_NUMACTL > > The dependencies on libpciaccess and numactl are optional, so I removed > them. > > However, there was a missing dependency on BR2_TOOLCHAIN_HAS_THREADS, > since hwloc uses pthread. Again, only tested with glibc and such, so I'm not seeing lots of these dependencies. Glad I held off on my other contributions until I could get these through review. Now I've got some ideas of things to look for before submission. >> + help >> + Portable Hardware Locality >> + >> + Provides a portable abstraction (across OS, versions, architectures, ...) >> + of the hierarchical topology of modern architectures, including NUMA >> + memory nodes, sockets, shared caches, cores and simultaneous >> + multithreading. > > Lines slightly too long, so I rewrapped. What column wrapping is preferred for the Config.in files? I could throw it into my vim config to enforce it in the future. > >> +################################################################################ >> +# >> +# hwloc >> +# >> +################################################################################ >> + >> +HWLOC_VERSION = 1.10.1 > > I declared HWLOC_VERSION_MAJOR here.. > >> +HWLOC_SOURCE = hwloc-$(HWLOC_VERSION).tar.bz2 >> +HWLOC_SITE = http://www.open-mpi.org/software/hwloc/v1.10/downloads > > ... so it can be re-used here. > >> +HWLOC_DEPENDENCIES = libpciaccess numactl > > As I said, those dependencies are optional, so I made them as such, and > also used explicit --enable/--disable options. This allowed me to > discover that the numa support was never enabled with your submission: > the numactl package was not installing its library/header file in > $(STAGING_DIR). With an explicit --enable-libnuma, hwloc configure > script bailed out since it couldn't find the libnuma library. This would probably explain why the 'perf' build wasn't finding the optional libnuma dependency as well -- hadn't figured out what was wrong there. > So I committed a patch that installs libnuma to the staging directory > as well. Excellent. I'm not familiar with how the staging directory works. Does that change work for any package depending on it (e.g. perf)? > Also, I added some explicit --disable options for the things that we > don't support for now. > Nice.
Dear Steven Noonan, On Fri, 20 Mar 2015 15:33:41 -0700, Steven Noonan wrote: > > However, there was a missing dependency on BR2_TOOLCHAIN_HAS_THREADS, > > since hwloc uses pthread. > > Again, only tested with glibc and such, so I'm not seeing lots of > these dependencies. Glad I held off on my other contributions until I > could get these through review. Now I've got some ideas of things to > look for before submission. Specifically for thread dependency, I generally don't build with a no-thread toolchain (even though one is available pre-built at http://autobuild.buildroot.org/toolchains/configs/br-arm-full-nothread.config). Instead, I grep for pthread in the source code, and try to decide whether it needs thread support or not (sometimes packages use thread when available, but it is optional). > > Lines slightly too long, so I rewrapped. > > What column wrapping is preferred for the Config.in files? I could > throw it into my vim config to enforce it in the future. I think 72 ? > > As I said, those dependencies are optional, so I made them as such, and > > also used explicit --enable/--disable options. This allowed me to > > discover that the numa support was never enabled with your submission: > > the numactl package was not installing its library/header file in > > $(STAGING_DIR). With an explicit --enable-libnuma, hwloc configure > > script bailed out since it couldn't find the libnuma library. > > This would probably explain why the 'perf' build wasn't finding the > optional libnuma dependency as well -- hadn't figured out what was > wrong there. Yes, that would explain it. > > So I committed a patch that installs libnuma to the staging directory > > as well. > > Excellent. I'm not familiar with how the staging directory works. It's actually quite simple. The staging directory, $(STAGING_DIR), is the "sysroot" of the toolchain. This is where the toolchain looks to find libraries and headers. By default, packages only get installed to $(TARGET_DIR), because <pkg>_INSTALL_TARGET defaults to "YES". In this case <pkg>_INSTALL_TARGET_CMDS is called and does it job (of course, for autotools packages, you don't implement this variable manually in your package .mk file, since the autotools-package infra already has it implemented for you). However, when your package is a library, or generally needs to install something in $(STAGING_DIR), you need to explicitly set <pkg>_INSTALL_STAGING to YES, because it defaults to NO. When set to YES, the top-level package infrastructure will call <pkg>_INSTALL_STAGING_CMDS (which, just like <pkg>_INSTALL_TARGET_CMDS, is already implemented for you for autotools packages, CMake packages and al.). > Does that change work for any package depending on it (e.g. perf)? Yes: from now on, whenever a package has "numactl" in its <pkg>_DEPENDENCIES variable, numactl will be installed to both $(TARGET_DIR) and $(STAGING_DIR), which means that such packages will be able to "see" the libnuma library and header files. Thomas
diff --git a/package/Config.in b/package/Config.in index 96e373a..4ee489c 100644 --- a/package/Config.in +++ b/package/Config.in @@ -331,6 +331,7 @@ if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS source "package/hdparm/Config.in" endif source "package/hwdata/Config.in" + source "package/hwloc/Config.in" source "package/i2c-tools/Config.in" source "package/input-event-daemon/Config.in" source "package/input-tools/Config.in" diff --git a/package/hwloc/Config.in b/package/hwloc/Config.in new file mode 100644 index 0000000..332536b --- /dev/null +++ b/package/hwloc/Config.in @@ -0,0 +1,18 @@ +config BR2_PACKAGE_HWLOC + bool "hwloc" + # libpciaccess dependency + depends on BR2_LARGEFILE + # numactl dependency + depends on BR2_i386 || BR2_mips || BR2_mipsel || \ + BR2_mips64 || BR2_mips64el || BR2_powerpc || BR2_x86_64 + select BR2_PACKAGE_LIBPCIACCESS + select BR2_PACKAGE_NUMACTL + help + Portable Hardware Locality + + Provides a portable abstraction (across OS, versions, architectures, ...) + of the hierarchical topology of modern architectures, including NUMA + memory nodes, sockets, shared caches, cores and simultaneous + multithreading. + + http://www.open-mpi.org/projects/hwloc/ diff --git a/package/hwloc/hwloc.hash b/package/hwloc/hwloc.hash new file mode 100644 index 0000000..53563dd --- /dev/null +++ b/package/hwloc/hwloc.hash @@ -0,0 +1,2 @@ +# From http://www.open-mpi.org/software/hwloc/v1.10/ +sha1 76291124e4638b2fbd4deb4cc3cd680e153077b5 hwloc-1.10.1.tar.bz2 diff --git a/package/hwloc/hwloc.mk b/package/hwloc/hwloc.mk new file mode 100644 index 0000000..ced3627 --- /dev/null +++ b/package/hwloc/hwloc.mk @@ -0,0 +1,14 @@ +################################################################################ +# +# hwloc +# +################################################################################ + +HWLOC_VERSION = 1.10.1 +HWLOC_SOURCE = hwloc-$(HWLOC_VERSION).tar.bz2 +HWLOC_SITE = http://www.open-mpi.org/software/hwloc/v1.10/downloads +HWLOC_DEPENDENCIES = libpciaccess numactl +HWLOC_LICENSE = BSD-3c +HWLOC_LICENSE_FILES = COPYING + +$(eval $(autotools-package))
Signed-off-by: Steven Noonan <steven@uplinklabs.net> --- v2 - Fixed description in Config.in. v3 - Fixed source URL for hwloc.hash entries. - Copied dependencies from libpciaccess/numactl. package/Config.in | 1 + package/hwloc/Config.in | 18 ++++++++++++++++++ package/hwloc/hwloc.hash | 2 ++ package/hwloc/hwloc.mk | 14 ++++++++++++++ 4 files changed, 35 insertions(+) create mode 100644 package/hwloc/Config.in create mode 100644 package/hwloc/hwloc.hash create mode 100644 package/hwloc/hwloc.mk