-
Notifications
You must be signed in to change notification settings - Fork 53
Add hwloc to core kit
#672
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
| %{_cross_bindir}/hwloc-annotate | ||
| %{_cross_bindir}/hwloc-compress-dir | ||
| %{_cross_bindir}/hwloc-gather-topology | ||
| %{_cross_bindir}/hwloc-ls | ||
| %{_cross_bindir}/hwloc-ps | ||
| %{_cross_bindir}/hwloc-bind | ||
| %{_cross_bindir}/hwloc-calc | ||
| %{_cross_bindir}/hwloc-diff | ||
| %{_cross_bindir}/hwloc-distrib | ||
| %{_cross_bindir}/hwloc-info | ||
| %{_cross_bindir}/hwloc-patch | ||
| %{_cross_bindir}/lstopo | ||
| %{_cross_bindir}/lstopo-no-graphics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all binaries, not helper scripts?
Can we omit any of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The primary use case I've seen is using hwloc-ls for confirming topology. I think hwloc-info might be similar. I added a reply to another comment but I'm not able to find the specific invocation we need for any specific binary so I think we may just keep this as a package that can be included in -dev variants. Either way, I think we can drop lstopo which is graphically oriented. Outside of the ls/info, the rest are for manipulating the topology files which would be useful if trying to debug this manually but honestly I think this would not be generally how things are done for containers.
packages/release/release.spec
Outdated
| Requires: %{_cross_os}glibc | ||
| Requires: %{_cross_os}grep | ||
| Requires: %{_cross_os}grub | ||
| Requires: %{_cross_os}hwloc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the binary package to release to include in all variants since EFA can also use hwloc for finding colocation-relevant data.
You're saying that EFA (the kernel driver) does an upcall to hwloc in the host OS? Or what exactly does that dependency look like?
I'd prefer to make this NVIDIA-only if there's nothing in the host that actually uses this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went to find the specific references to these to trim them down and I'm not finding direct usage of hwloc-ls. The errors I first encountered are referencing this code which has a library dependency on hwloc. In light of not finding any specifics, I think we should avoid adding this to images by default, and use this more as a debugging package when needed.
|
^ Updated to no longer include hwloc in |
hwloc is needed both as a binary dependency like hwloc-ls but also can be used as a build dependency so this package vends a devel both a primary package and a development package. Signed-off-by: Matthew Yeazel <[email protected]>
| --enable-static \ | ||
| --disable-shared \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why enabling static libraries but not shared libraries? Is it because hwloc will be used within a container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was the intention. If we don't need them mounted into containers we can use shared instead and save of space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention was to allow these to be mounted into a container. If we don't expect that to happen, we can switch back to dynamic. It will save space on the filesystem so I'm ok to do that.
| sha512 = "949d6c9d7b858ee58e477b15e6c06f57812872142fa1c7f3ef20aae2e4ef954135f839e8604404bfd0637fde99729c7d00211c8aee860dfde9ac60bba0e78aef" | ||
|
|
||
| [build-dependencies] | ||
| glibc = { path = "../glibc" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we begin skipping glibc as a dependency of the packages @bcressey ? Now that glibc-devel is provided by the SDK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't have use cases in mind for the tools, I'd prefer to invert the packaging and have libhwloc, libhwloc-devel, and libhwloc-tools - so that the tools are optional when the shared library is needed.
Compare to libcrypto-tools and libcryptsetup-tools.
| %{_cross_bindir}/hwloc-compress-dir | ||
| %{_cross_bindir}/hwloc-gather-topology |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are both shell scripts, at least on Fedora.
Issue number:
Closes # #670
Description of changes:
Adds
hwlocas package to the core kit. Also vend a devel package in case variants want to bind against hwloc as a build dependency rather than use the binaries.Testing done:
Built images with the binaries included.
hwloc-lsis usable fromsheltie:Built an image and confirmed hwloc-ls will work from a container.
Specfile:
Output:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.