[Bug 606498] Review Request: hwloc - portable abstraction of hierarchical architectures

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=606498

Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp

--- Comment #4 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 2010-07-01 15:46:28 EDT ---
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.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.