Spec URL: http://rapidshare.com/files/401428321/hwloc.spec.html MD5: 785DCF1783644BCD37AD0AEEF8ADA46F SRPM URL: http://rapidshare.com/files/401428011/hwloc-1.0.1-17.fc12.src.rpm.html MD5: 785DCF1783644BCD37AD0AEEF8ADA46F Review Description: Hello! I work at Red Hat and we are using hwloc to report CPU and NUMA topology and to start process with given CPU affinity. I believe it's very useful package and therefore I have decided to create a package for it. This is my first package and I'm seeking a sponsor. I have followed guidelines at http://fedoraproject.org/wiki/Packaging:ReviewGuidelines and it has passed all MUST and SHOULD haves. I have uploaded SRPM and SPEC file on rapidshare. It's just a temporary solution, I hope to get my public URL on http://fedorapeople.org/ to share SPEC files and SRPMs. Thanks a lot! Jirka Description of hwloc package: The Portable Hardware Locality (hwloc) software package provides a portable abstraction (across OS, versions, architectures, ...) of the hierarchical topology of modern architectures, including NUMA memory nodes, shared caches, processor sockets, processor cores and processing units (logical processors or "threads"). It also gathers various system attributes such as cache and memory information. It primarily aims at helping applications with gathering information about modern computing hardware so as to exploit it accordingly and efficiently. hwloc may display the topology in multiple convenient formats. It also offers a powerful programming interface (C API) to gather information about the hardware, bind processes, and much more.
Hi, I was dealing with rpath. Final solution was to add sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool %{__make} %{?_smp_mflags} into %build field. I don't like this. Libraries are getting installed into /usr/lib64 but libtool is still using rpath. I was working with upstream to resolve this and their recommendation was to put "/usr/lib64" directory into /etc/ld.so.conf. It's indeed working (tested on Fedora12 and 13, both x86_64) It seems to me like bug libtool - IMHo, it should recognize /usr/lib64 as default directory for shared libraries on 64 bit system. Thanks Jirka
New Package CVS Request ======================= Package Name: hwloc Short Description: portable abstraction of hierarchical architectures Owners: jhladky Branches: F-11 F-12 F-13 EL-5 EL-6 InitialCC: jhladky
Spec URL: http://jhladky.fedorapeople.org/hwloc.spec MD5: 785DCF1783644BCD37AD0AEEF8ADA46F SRPM URL: http://jhladky.fedorapeople.org/hwloc-1.0.1-17.fc12.src.rpm MD5: 785DCF1783644BCD37AD0AEEF8ADA46F
Some initial comments: ? Release number - By the way, why does the release number of your spec file begin with 17 (instead of 1)? ! EPEL4 handling - Well, as I don't have RHEL product, I don't care for EPEL handling. However some notes: * Maybe it is better that you create another spec file for EPEL4 only and remove all %{?rhel} handling, because (Build)Requires are so different, however this is not a blocker (however see below) * Would you explain why - You have disabled AutoReq - And why main package (hwloc) has some Requires for development-purpose packages (i.e. has "Requires: foo-devel")? This seems strange because on Fedora side you don't write such Requires. * make build.log more verbose - Currently build.log does not show how linkage between binaries is done during compiling. - Also for EPEL4 build, build.log shows like: ------------------------------------------------------------ + /usr/bin/make -j8 Making all in src make[1]: Entering directory `/builddir/build/BUILD/hwloc-1.0.1/src' CC topology.lo CC traversal.lo CC topology-synthetic.lo CC cpuset.lo CC misc.lo CC bind.lo ------------------------------------------------------------ From this we cannot check if compilation flags are honored correctly or not. Please add "V=1" to "make %{?_smp_mflags}" to make build.log more verbose. * Requires for -devel subpackage - Would you check if Requires on -devel subpackage is needed, other than libxml2-devel? * Installed header files don't seem to require any of these, and the installed pkgconfig .pc file only requires libxml-2.0 (as Requires.private) ! Note - On Fedora 12+, rpmbuild checks the dependencies on pkgconfig file and "Requires: pkgconfig(libxml-2.0)" is automatically added to -devel subpackage, so (on Fedora) "Requires: libxml2-devel" for -devel subpackage is not (explicitly) needed. * Macros - Please use macros for standard directories, %{_prefix} for /usr, %{_bindir} for /usr/bin: https://fedoraproject.org/wiki/Packaging/RPMMacros - and %{_prefix}/share should be %{_datadir} - And please use macros consistently: - If you use %{__rm}, %{__make} or so, please also use %{__sed}. and both %{__rm} and rm are used, please choose one. * Timestamp - When using cp or install command, also add "-p" option to keep timestamps on installed files: https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps * Rpath handling - So are you going to remove by this way instead of the following one? (Note: not a blocker) http://lists.fedoraproject.org/pipermail/packaging/2010-June/007187.html * Directory ownership issue - %{_datadir}/%{name} itself is not owned by any packages. - -devel subpackage need now own the directory %{_defaultdocdir}/%{name}-%{version}, this is already owned by main package and -devel subpackage depends on main package.
Hi Mamoru, thanks a lot for looking into it! I have followed your advises and fixed the issues you have pointed out. Updated version can be downloaded here: Spec URL: http://jhladky.fedorapeople.org/hwloc.spec-18 MD5: 4c9fbaa104dd6106cbbb7386baa5dfec Spec URL: http://jhladky.fedorapeople.org/hwloc-1.0.1-18.fc12.src.rpm MD5: 16cfa71f386a3b27539d2ef25e047702 See my comments below: > Some initial comments: > > ? Release number > - By the way, why does the release number of your spec file > begin with 17 (instead of 1)? > I'm already using this package on RHEL systems. I had lots of building issues on RHEL4 because of faulty evolution28-cairo-devel rpm package. That's why Release number is so high. Sorry for it. > ! EPEL4 handling > - Well, as I don't have RHEL product, I don't care for EPEL handling. > However some notes: > * Maybe it is better that you create another spec file for EPEL4 only and > remove all %{?rhel} handling, because (Build)Requires are so different, > however this is not a blocker (however see below) Done. I have now standalone SPEC for RHEL4. I have dropped vector graphic support on RHEL4. Posted spec file is for Fedora, RHEL5 and RHEL6. > * Would you explain why > - You have disabled AutoReq That was only for RHEL4. I have removed it. (reason: broken evolution28-cairo-devel rpm package.) > - And why main package (hwloc) has some Requires for development-purpose > packages (i.e. has "Requires: foo-devel")? This seems strange because > on Fedora side you don't write such Requires. That was only for RHEL4. I have removed it. > * make build.log more verbose > - Currently build.log does not show how linkage between binaries is done > during compiling. > - Also for EPEL4 build, build.log shows like: > ------------------------------------------------------------ > + /usr/bin/make -j8 > Making all in src > make[1]: Entering directory `/builddir/build/BUILD/hwloc-1.0.1/src' > CC topology.lo > CC traversal.lo > CC topology-synthetic.lo > CC cpuset.lo > CC misc.lo > CC bind.lo > ------------------------------------------------------------ > From this we cannot check if compilation flags are honored correctly > or not. > Please add "V=1" to "make %{?_smp_mflags}" to make build.log more verbose. Done. >* Requires for -devel subpackage > - Would you check if Requires on -devel subpackage is needed, other than > libxml2-devel? > * Installed header files don't seem to require any of these, and the >installed > pkgconfig .pc file only requires libxml-2.0 (as Requires.private) > > ! Note > - On Fedora 12+, rpmbuild checks the dependencies on pkgconfig file and > "Requires: pkgconfig(libxml-2.0)" is automatically added to -devel >subpackage, > so (on Fedora) "Requires: libxml2-devel" for -devel subpackage is not > (explicitly) needed. I let now rpmbuild to decide about Requires. > * Macros > - Please use macros for standard directories, %{_prefix} for /usr, %{_bindir} > for /usr/bin: > https://fedoraproject.org/wiki/Packaging/RPMMacros > - and %{_prefix}/share should be %{_datadir} > - And please use macros consistently: > - If you use %{__rm}, %{__make} or so, please also use %{__sed}. > and both %{__rm} and rm are used, please choose one. Done. > * Timestamp > - When using cp or install command, also add "-p" option to keep timestamps > on installed files: > https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps I have -p already in the spec file: %{__make} install DESTDIR=%{buildroot} INSTALL="%{__install} -p" Is anything wrong with the line above? > * Rpath handling > - So are you going to remove by this way instead of the following one? > (Note: not a blocker) > http://lists.fedoraproject.org/pipermail/packaging/2010-June/007187.html I have tried to use the solution proposed in the mailing thread above. However, there are some issues: 1) I'm not sure if I can do it for all archs - should I add conditional to do it only on x86_64? To be honest I don't know about ARM, MIPS, Parisc - are libraries on these archs stored at /usr/lib64 ?? https://fedoraproject.org/wiki/Architectures Therefore I would prefer to have a solution common for all archs. 2) rpmlint --verbose --info SPECS/hwloc.spec SPECS/hwloc.spec:42: E: hardcoded-library-path in /lib It's complaining about this line: %{__sed} -i.libdir_syssearch -e '/sys_lib_dlsearch_path_spec/s|/usr/lib |/usr/lib /usr/lib64 /lib /lib64|' configure Because of these issues I have stuck with solutions proposed at http://fedoraproject.org/wiki/RPath_Packaging_Draft > * Directory ownership issue > - %{_datadir}/%{name} itself is not owned by any packages. > - -devel subpackage need now own the directory I have fixed it by adding %dir %{_datadir}/%{name} to %files for hwloc package. > %{_defaultdocdir}/%{name}-%{version}, > this is already owned by main package and -devel subpackage depends on > main package. I have removed %dir %{_defaultdocdir}/%{name}-%{version} from %files section of hwloc-devel package. Please have a look at new spec file. Thanks a lot! Jirka
For -18: Well, now after removing RHEL4 specific stuff, the spec file is much easier to read, thank you. * Timestamp (In reply to comment #5) > > * Timestamp > > - When using cp or install command, also add "-p" option to keep timestamps > > on installed files: > > https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps > > I have -p already in the spec file: > %{__make} install DESTDIR=%{buildroot} INSTALL="%{__install} -p" > Is anything wrong with the line above? - I am mentioning about the following lines (INSTALL="%{__install} -p" is correct) ------------------------------------------------------------- 61 %{__cp} AUTHORS COPYING NEWS README VERSION %{buildroot}%{_defaultdocdir}/%{name}-%{version} 62 %{__cp} doc/hwloc-hello.c %{buildroot}%{_defaultdocdir}/%{name}-%{version} ------------------------------------------------------------- Please use %{__cp} -p ! rpath > > * Rpath handling > > - So are you going to remove by this way instead of the following one? > > (Note: not a blocker) > > http://lists.fedoraproject.org/pipermail/packaging/2010-June/007187.html > > I have tried to use the solution proposed in the mailing thread above. However, > there are some issues: > > 1) I'm not sure if I can do it for all archs - should I add conditional to do > it only on x86_64? To be honest I don't know about ARM, MIPS, Parisc - are > libraries on these archs stored at /usr/lib64 ?? > https://fedoraproject.org/wiki/Architectures > Therefore I would prefer to have a solution common for all archs. - I am interested only in what is supported on Fedora. > 2) rpmlint --verbose --info SPECS/hwloc.spec > SPECS/hwloc.spec:42: E: hardcoded-library-path in /lib > > It's complaining about this line: > %{__sed} -i.libdir_syssearch -e '/sys_lib_dlsearch_path_spec/s|/usr/lib > |/usr/lib /usr/lib64 /lib /lib64|' configure > > Because of these issues I have stuck with solutions proposed at > http://fedoraproject.org/wiki/RPath_Packaging_Draft - Well, we must always investigate if rpmlint warnings or errors can be ignored or not on each specific package. There are also some other cases in which we ignore rpmlint warnings or errors. However this is not a blocker. Other things * Arch-specific header files in /usr/include --------------------------------------------------------------------- diff -ur Binary/hwloc-devel-1.0.1-18.fc14.i686/usr/include/hwloc/config.h x86_64/hwloc-devel-1.0.1-18.fc14.x86_64/usr/include/hwloc/config.h --- Binary/hwloc-devel-1.0.1-18.fc14.i686/usr/include/hwloc/config.h 2010-07-10 00:41:44.000000000 +0900 +++ x86_64/hwloc-devel-1.0.1-18.fc14.x86_64/usr/include/hwloc/config.h 2010-07-10 00:41:45.000000000 +0900 @@ -99,7 +99,7 @@ #define HWLOC_NBMAXCPUS 1024 /* The size of `unsigned long', as computed by sizeof */ -#define HWLOC_SIZEOF_UNSIGNED_LONG 4 +#define HWLOC_SIZEOF_UNSIGNED_LONG 8 /* The size of `unsigned int', as computed by sizeof */ #define HWLOC_SIZEOF_UNSIGNED_INT 4 --------------------------------------------------------------------- - Installed hwloc/config.h differs between i686 vs x86_64, and this breaks multilib installation. This is (perhaps) not a blocker, however it is highly preferred that -devel package supports multilib installation. ! You can use what python-devel handles this issue. python-devel has "/usr/include/python2.6/pyconfig.h", which says: --------------------------------------------------------------------- #include <bits/wordsize.h> #if __WORDSIZE == 32 #include "pyconfig-32.h" #elif __WORDSIZE == 64 #include "pyconfig-64.h" #else #error "Unknown word size" #endif --------------------------------------------------------------------- and python-devel renames the "original pyconfig.h" to pyconfig-{32,64}.h.
Now: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few (or no) work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora package collection review requests which are waiting for someone to review can be checked on my wiki page: http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets (Check "No one is reviewing") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
Hello, I am one of the upstream devs of hwloc. I am looking at fixing the arch-specific-header-in-/usr/include problem. Looks easy, so will likely be fixed in 1.1 and maybe even in 1.0.2. Brice
Hi Brice, it would be great if you can fix arch-specific-header-in-/usr/include problem in the upstream. I have checked rpm spec file for python-devel. They deal with this issue in rpm spec file like this: =============================================================== # Make python-devel multilib-ready (bug #192747, #139911) %global _pyconfig32_h pyconfig-32.h %global _pyconfig64_h pyconfig-64.h %ifarch ppc64 s390x x86_64 ia64 alpha sparc64 %global _pyconfig_h %{_pyconfig64_h} %else %global _pyconfig_h %{_pyconfig32_h} %endif mv %{buildroot}%{_includedir}/python%{pybasever}/pyconfig.h \ %{buildroot}%{_includedir}/python%{pybasever}/%{_pyconfig_h} cat > %{buildroot}%{_includedir}/python%{pybasever}/pyconfig.h << EOF #include <bits/wordsize.h> #if __WORDSIZE == 32 #include "%{_pyconfig32_h}" #elif __WORDSIZE == 64 #include "%{_pyconfig64_h}" #else #error "Unknown word size" #endif EOF ============================================================ Steps are: 1) mv pyconfig.h pyconfig-32.h (or pyconfig-64.h on 64-bit platform) 2) Create new pyconfig.h with following content: #include <bits/wordsize.h> #if __WORDSIZE == 32 #include "%{_pyconfig32_h}" #elif __WORDSIZE == 64 #include "%{_pyconfig64_h}" #else #error "Unknown word size" #endif EOF Please let me know if you can implement this in upstream. If yes, I will wait for 1.0.2 or 1.1 release. Should you run into trouble, let me just know and I will implement python's like solution in the hwloc SPEC file. Thanks a lot! Jirka
Created attachment 430871 [details] python.spec file from Fedora12
Jiri, I just removed all definitions and uses of HWLOC_SIZEOF_* in the public headers in trunk. See: https://svn.open-mpi.org/trac/hwloc/changeset/2300 https://svn.open-mpi.org/trac/hwloc/changeset/2298 It's only needed and used during internal build now. So I should be enough I guess? I am waiting for feedback from other developers before backporting to the 1.0 branch.
Hi Brice, thanks a lot for fixing this! Yes, removing HWLOC_SIZEOF_* from public headers should be enough. Thanks Jirka
Hi Mamoru, thanks a lot for your comments! https://bugzilla.redhat.com/show_bug.cgi?id=606498#c6 * Timestamp : done * RPATH: done %ifarch ppc64 s390x x86_64 ia64 alpha sparc64 %{__sed} -i.libdir_syssearch -e '/sys_lib_dlsearch_path_spec/s|/usr/lib |/usr/lib /usr/lib64 /lib /lib64|' configure %endif * Multilib support: implemented in upstream. Thanks Brice! I will now wait for either hwloc 1.0.2 or hwloc 1.1 (whatever comes first) to finalize the packing. The current version of SPEC file and srpms are located here (Multilib not yet fixed): Spec URL: http://jhladky.fedorapeople.org/hwloc.spec-19 MD5: 47043f440e69ff3af04afeef45c03113 Spec URL: http://jhladky.fedorapeople.org/hwloc-1.0.1-19.fc12.src.rpm MD5: 2940ef9ee267985dd33254babdee6198 Thanks Jirka
Hi Mamoru, ad https://bugzilla.redhat.com/show_bug.cgi?id=606498#c7 Radek Vokal has already sponsored me. Radek is my colleague at Red Hat, we work in the same office. Nevertheless, I do plan to review some R-languages when I return from vacation in two weeks. Thanks Jirka
(In reply to comment #13) > * Multilib support: implemented in upstream. Thanks Brice! > I will now wait for either hwloc 1.0.2 or hwloc 1.1 (whatever comes first) to > finalize the packing. > > The current version of SPEC file and srpms are located here (Multilib not yet > fixed): Would you apply the upstream patch for now?
FYI, upstream patch backported to the 1.0 branch in commit r2304 https://svn.open-mpi.org/trac/hwloc/changeset/2304
Thanks Brice! Mamoru, I had discussion with Brice regarding creating a patch file to hwloc 1.0.1. However, there is quite a lot of changes and it includes some binaries files (.png, .pdf) which has been added to hwloc source tree. Nevertheless I have tried to create a patch file but it does not work: ================================================================ svn diff http://svn.open-mpi.org/svn/hwloc/tags/hwloc-1.0.1 http://svn.open-mpi.org/svn/hwloc/branches/v1.0 > my.patch tar zxvf ~/tests/performance/hwloc/hwloc-1.0.1.tar.gz cd hwloc-1.0.1/ $patch -p0 -i ../my.patch patching file include/private/private.h ..........stripped......... up-to this error message: patching file utils/Makefile.am can't find file to patch at input line 822 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: tests/embedded/Makefile.am |=================================================================== |--- tests/embedded/Makefile.am (.../tags/hwloc-1.0.1) (revision 2305) |+++ tests/embedded/Makefile.am (.../branches/v1.0) (revision 2305) ===================================================================== Since we are close to hwloc 1.0.2 release we have agreed to post here 1.0.2 alpha version. Brice will prepare a source tree tarball for me. Would it be OK with you? Thanks Jirka
Well, if applying the patch seems difficult for now, please rename config.h to something arch-specific (as written in comment 6) until next version is released. (if 1.0.2 alpha is to be released soon, it is of course appreciated)
Hi Mamoru, I'm back from vacation. Brice has prepared release candidate for version 1.0.2. Please find updated SPEC file and SRPM below: Spec URL: http://jhladky.fedorapeople.org/hwloc-1.0.2-0.1.rc1r2330.spec MD5: a53baaacf2efcfc1a7f845bedeb4778b SRPM URL: http://jhladky.fedorapeople.org/hwloc-1.0.2-0.1.rc1r2330.fc12.src.rpm MD5: acc4e6a57c20c833550126695a060691 Thanks Jirka
Okay, then * %check - Now as source tarball contains tests/ directory, please add %check section and execute some test program (like $ make check V=1) there.
Hi Mamoru, version 1.0.2 has been just released:-) I have created new SPEC file and SRPM with following new section: %check %{__make} check Spec URL: http://jhladky.fedorapeople.org/hwloc-1.0.2-1.spec MD5: cb1f10eeb71913ee75f7bdb853582c17 SRPM URL: http://jhladky.fedorapeople.org/hwloc-1.0.2-1.fc12.src.rpm MD5: ca7f40565210140ef7b2e2da8669d1e0 Please have a look. Thanks Jirka
Okay. ------------------------------------------------------- This package (hwloc) is APPROVED by mtasaka -------------------------------------------------------
Hi Mamoru, thanks a lot for reviewing the package and helping me to resolve the issues! I'm going to ask for CVS access. Thanks Jirka
New Package CVS Request ======================= Package Name: hwloc Short Description: portable abstraction of hierarchical architectures Owners: jhladky Branches: F-12 F-13 EL-5 EL-6 InitialCC: jhladky
CVS done (by process-cvs-requests.py).
hwloc-1.0.2-1.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/hwloc-1.0.2-1.fc12
hwloc-1.0.2-1.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/hwloc-1.0.2-1.el5
hwloc-1.0.2-1.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/hwloc-1.0.2-1.fc13
Closing.
hwloc-1.0.2-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
hwloc-1.0.2-1.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
hwloc-1.0.2-1.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.
hwloc-1.1-0.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/hwloc-1.1-0.fc13
hwloc-1.1-0.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/hwloc-1.1-0.fc14
hwloc-1.1-0.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/hwloc-1.1-0.el5
hwloc-1.1-0.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
hwloc-1.1-0.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.
hwloc-1.1-0.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.