Bug 1174290 - Review Request: scalasca - Toolset for scalable performance analysis of large-scale parallel applications
Summary: Review Request: scalasca - Toolset for scalable performance analysis of large...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1174292
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-12-15 15:03 UTC by Dave Love
Modified: 2017-02-07 04:33 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-09-26 17:34:08 UTC
Type: ---
Embargoed:
zbyszek: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Dave Love 2014-12-15 15:03:08 UTC
Spec URL: https://loveshack.fedorapeople.org/review/scalasca.spec
SRPM URL: https://loveshack.fedorapeople.org/review/scalasca-2.1-4.el6.src.rpm
Description: 
Scalasca is a software tool that supports the performance optimization
of parallel programs by measuring and analyzing their runtime
behavior. The analysis identifies potential performance bottlenecks -
in particular those concerning communication and synchronization - and
offers guidance in exploring their causes.

Scalasca targets mainly scientific and engineering applications based
on the programming interfaces MPI and OpenMP, including hybrid
applications based on a combination of the two. The tool has been
specifically designed for use on large-scale systems, but is also well
suited for small- and medium-scale HPC platforms.
Fedora Account System Username: loveshack

copr builds:
http://copr.fedoraproject.org/coprs/loveshack/livhpc/build/63651/
http://copr.fedoraproject.org/coprs/loveshack/livhpc/build/63654/

This needs scorep <https://bugzilla.redhat.com/show_bug.cgi?id=1013836>
to acquire data, though it's not a dependency.  It also requires an updated otf2
for EPEL6.

Comment 1 Vladimir Stackov 2014-12-16 07:22:44 UTC
Greetings,

why do you need "rm -rf $RPM_BUILD_ROOT" in %install and %clean ?
It seems that you are not packaging this for EL5/F12 and below so you could easily remove it.

You should also use %license for COPYING instead of %doc.

Please note that this is informal review.

Comment 2 Dave Love 2014-12-16 13:18:28 UTC
The rm -rf is what the template installs.  I wouldn't rule it out on EPEL5,
though I'm unlikely to want it on our EPEL5 cluster, and the dependencies
aren't currently available.

What is %license?  I treated COPYING the same way as the example in the
guidelines, and like every other package I've seen, as far as I can tell.

Comment 3 Vladimir Stackov 2014-12-16 14:24:53 UTC
(In reply to Dave Love from comment #2)
> The rm -rf is what the template installs.  I wouldn't rule it out on EPEL5,
> though I'm unlikely to want it on our EPEL5 cluster, and the dependencies
> aren't currently available.
https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

> 
> What is %license?  I treated COPYING the same way as the example in the
> guidelines, and like every other package I've seen, as far as I can tell.

https://fedorahosted.org/fpc/ticket/411
https://fedoraproject.org/wiki/Changes/Use_license_macro_in_RPMs_for_packages_in_Cloud_Image

Comment 4 Dave Love 2014-12-17 15:43:52 UTC
(In reply to Vladimir Stackov from comment #3)

> https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

Yes, but I'm doing what the packaging guidelines say by using a template and
rpmlint.  This seems harmless and trivial in comparison to feedback I was
hoping for before doing similar tools.

> https://fedorahosted.org/fpc/ticket/411
> https://fedoraproject.org/wiki/Changes/
> Use_license_macro_in_RPMs_for_packages_in_Cloud_Image

I can't tell what the status of that is, even.  It's not in the guidelines,
I haven't seen anything use it, and I don't understand what you gain by
substituting the contents of the license tag somewhere.  I want to include
actual documents, which aren't necessarily just the GPL, or whatever.

Comment 5 Dave Love 2015-02-22 20:13:14 UTC
I updated this, mainly to add %check.  It isn't the latest upstream now; that depends on otf2 1.5.

Spec URL: https://loveshack.fedorapeople.org/review/scalasca.spec
SRPM URL: https://loveshack.fedorapeople.org/review/scalasca-2.1-6.el6.src.rpm

Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=9030284

Comment 6 Zbigniew Jędrzejewski-Szmek 2015-05-29 19:03:24 UTC
(In reply to Dave Love from comment #4)
> (In reply to Vladimir Stackov from comment #3)
> 
> > https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean
> 
> Yes, but I'm doing what the packaging guidelines say by using a template and
> rpmlint.  This seems harmless and trivial in comparison to feedback I was
> hoping for before doing similar tools.
Less is better. It is easier to read (or review) the spec file if it doesn't contain unnecessary elements. The guidelines are normative, and rpmlint and templates are often out of date.

Similarly for BuildRoot tag: please remove it.

> > https://fedorahosted.org/fpc/ticket/411
> > https://fedoraproject.org/wiki/Changes/
> > Use_license_macro_in_RPMs_for_packages_in_Cloud_Image
> 
> I can't tell what the status of that is, even.  It's not in the guidelines,
> I haven't seen anything use it, and I don't understand what you gain by
> substituting the contents of the license tag somewhere.  I want to include
> actual documents, which aren't necessarily just the GPL, or whatever.
Please see https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text. Using %license is now mandatory. It does not substitute the file, just changes the location and makes rpm install it even if docs are excluded.

If you want to retain compatibility with older releases, you can use:
%files
%{!?_licensedir:%global license %%doc}
%license COPYING

It's a question of preference, but I'd change stuff like:
for d in openmpi \
%if %{with mpich}
mpich
%endif
do
  pushd $d
  make install DESTDIR=$RPM_BUILD_ROOT
  popd
done

to
%makeinstall -C openmpi
%if %{with mpich}
%makeinstall -C mpich
%endif

... just to be easier on the eyes :)

Comment 7 Zbigniew Jędrzejewski-Szmek 2015-06-27 16:58:45 UTC
Ping?

Comment 8 Dave Love 2015-07-14 16:07:25 UTC
Sorry I let this drop.  I'm working on getting support for the latest version
into EPEL and making sure I can test that, since I don't run fedora.
Also I've failed to run fedora-review on it, either in RHEL6 or an F22
vagrant box.

Comment 9 Zbigniew Jędrzejewski-Szmek 2015-07-14 16:48:24 UTC
It fails to build in rawhide:
configure: error: provided interface version '7' of cube not sufficient for Scalasca, provide '5' or compatible.
cube-config --interface-version says 7:2:0.
I don't know if it backwards compatible.

Comment 10 Dave Love 2015-07-16 16:41:48 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #9)
> It fails to build in rawhide:
> configure: error: provided interface version '7' of cube not sufficient for
> Scalasca, provide '5' or compatible.
> cube-config --interface-version says 7:2:0.
> I don't know if it backwards compatible.

That's fixed by the current version, built in copr, but I haven't actually
tested it, and I want to get the updated cube into EPEL.

Comment 11 Dave Love 2015-07-18 17:15:30 UTC
This is the latest upstream.  The buildrequires are now in epel6-testing. 

Spec URL: https://loveshack.fedorapeople.org/review/scalasca.spec
SRPM URL: https://loveshack.fedorapeople.org/review/scalasca-2.2.2-2.el6.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=10401154

Comment 12 Zbigniew Jędrzejewski-Szmek 2015-07-18 19:03:00 UTC
As usual, I'd suggest adding %global _docdir_fmt %{name}, to avoid having a separate doc and license dir for each subpackage. (Although you'd probably have to take some extra steps so that the html documentation does not end up in the packages, so it might not be worth the effort.)

Is the comment about the license of the spec file really necessary? By default all spec files in Fedora are MIT licensed [https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#License_of_Fedora_SPEC_Files], so there's little difference with the BSD license that is currently specified, and having an explicit (and different) license requires extra thought from whomever would e.g. want to copy part of the spec file to incorporate into a different spec file.

- license is OK
- license file is present and %license is used
- build flags and parallel build are used
- spec file is nice and clean
- name is OK
- filesystem layout is OK
- latest version is packaged
- %check is present
- installs and runs without problem

Rpmlint
-------
Checking: scalasca-openmpi-2.2.2-2.fc23.x86_64.rpm
          scalasca-mpich-2.2.2-2.fc23.x86_64.rpm
          scalasca-doc-2.2.2-2.fc23.noarch.rpm
          scalasca-2.2.2-2.fc23.src.rpm
scalasca-openmpi.x86_64: W: spelling-error Summary(en_US) Toolset -> Tool set, Tool-set, Togolese
scalasca-openmpi.x86_64: W: spelling-error %description -l en_US runtime -> run time, run-time, rudiment
scalasca-openmpi.x86_64: W: shared-lib-calls-exit /usr/lib64/openmpi/lib/libpearl.base.so.0.0.0 exit.5
scalasca-openmpi.x86_64: E: library-without-ldconfig-postin /usr/lib64/openmpi/lib/libpearl.base.so.0.0.0
...

This is not in ld path anyway, so can be ignored.

scalasca-mpich.x86_64: W: undefined-non-weak-symbol /usr/lib64/mpich/lib/libpearl.thread.omp.so.0.0.0 vtable for pearl::MemoryError
...

OK.

scalasca-mpich.x86_64: W: unused-direct-shlib-dependency /usr/lib64/mpich/lib/libpearl.thread.omp.so.0.0.0 /lib64/libm.so.6
scalasca-mpich.x86_64: W: unused-direct-shlib-dependency /usr/lib64/mpich/lib/libpearl.ipc.mockup.so.0.0.0 /lib64/libstdc++.so.6
scalasca-mpich.x86_64: W: unused-direct-shlib-dependency /usr/lib64/mpich/lib/libpearl.ipc.mockup.so.0.0.0 /lib64/libm.so.6
scalasca-mpich.x86_64: W: unused-direct-shlib-dependency /usr/lib64/mpich/lib/libpearl.ipc.mockup.so.0.0.0 /lib64/libgcc_s.so.1

OK too. Those libraries are always installed anyway, so even if they could be removed from the dependency list, nothing would be gained.

Requires
--------
scalasca-mpich (rpmlib, GLIBC filtered):
    /bin/sh
    ld-linux-x86-64.so.2()(64bit)
    libc.so.6()(64bit)
    libcube4w.so.7()(64bit)
    libcubewriter4.so.7()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgomp.so.1()(64bit)
    libgomp.so.1(GOMP_1.0)(64bit)
    libgomp.so.1(GOMP_4.0)(64bit)
    libgomp.so.1(OMP_1.0)(64bit)
    libgomp.so.1(OMP_2.0)(64bit)
    libm.so.6()(64bit)
    libmpi.so.12()(64bit)
    libmpicxx.so.12()(64bit)
    libotf2.so.5()(64bit)
    libpearl.base.so.0()(64bit)
    libpearl.ipc.mockup.so.0()(64bit)
    libpearl.ipc.mpi.so.0()(64bit)
    libpearl.mpi.so.0()(64bit)
    libpearl.replay.so.0()(64bit)
    libpearl.thread.omp.so.0()(64bit)
    libpearl.thread.ser.so.0()(64bit)
    libpearl.writer.hyb.so.0()(64bit)
    libpearl.writer.mpi.so.0()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libz.so.1()(64bit)
    mpich(x86-64)
    rtld(GNU_HASH)

scalasca-openmpi (rpmlib, GLIBC filtered):
    /bin/sh
    ld-linux-x86-64.so.2()(64bit)
    libc.so.6()(64bit)
    libcube4w.so.7()(64bit)
    libcubewriter4.so.7()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgomp.so.1()(64bit)
    libgomp.so.1(GOMP_1.0)(64bit)
    libgomp.so.1(GOMP_4.0)(64bit)
    libgomp.so.1(OMP_1.0)(64bit)
    libgomp.so.1(OMP_2.0)(64bit)
    libm.so.6()(64bit)
    libmpi.so.1()(64bit)
    libmpi_cxx.so.1()(64bit)
    libotf2.so.5()(64bit)
    libpearl.base.so.0()(64bit)
    libpearl.ipc.mockup.so.0()(64bit)
    libpearl.ipc.mpi.so.0()(64bit)
    libpearl.mpi.so.0()(64bit)
    libpearl.replay.so.0()(64bit)
    libpearl.thread.omp.so.0()(64bit)
    libpearl.thread.ser.so.0()(64bit)
    libpearl.writer.hyb.so.0()(64bit)
    libpearl.writer.mpi.so.0()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libz.so.1()(64bit)
    openmpi(x86-64)
    rtld(GNU_HASH)

scalasca-doc (rpmlib, GLIBC filtered):

Provides
--------
scalasca-mpich:
    libpearl.base.so.0()(64bit)
    libpearl.ipc.mockup.so.0()(64bit)
    libpearl.ipc.mpi.so.0()(64bit)
    libpearl.mpi.so.0()(64bit)
    libpearl.replay.so.0()(64bit)
    libpearl.thread.omp.so.0()(64bit)
    libpearl.thread.ser.so.0()(64bit)
    libpearl.writer.hyb.so.0()(64bit)
    libpearl.writer.mpi.so.0()(64bit)
    scalasca-mpich
    scalasca-mpich(x86-64)

scalasca-openmpi:
    libpearl.base.so.0()(64bit)
    libpearl.ipc.mockup.so.0()(64bit)
    libpearl.ipc.mpi.so.0()(64bit)
    libpearl.mpi.so.0()(64bit)
    libpearl.replay.so.0()(64bit)
    libpearl.thread.omp.so.0()(64bit)
    libpearl.thread.ser.so.0()(64bit)
    libpearl.writer.hyb.so.0()(64bit)
    libpearl.writer.mpi.so.0()(64bit)
    scalasca-openmpi
    scalasca-openmpi(x86-64)

scalasca-doc:
    scalasca-doc

Requires and Provides are sane, but will be much imporoved when https://bugzilla.redhat.com/show_bug.cgi?id=1241282 goes through. You should probably then rebuild this package.

Everything seems fine. Package is APPROVED.

Comment 13 Dave Love 2015-07-30 11:08:16 UTC
Thanks for the approval.  I didn't see a notification about it for some reason.
I'll submit the package when I get back from holiday and have understood the MPI
changes and checked the docdir comment.  Somehow I'd missed the implicit
licensing of spec files, and I know to make sure everything has a licence.

Comment 14 Zbigniew Jędrzejewski-Szmek 2015-07-30 12:13:35 UTC
Re licensing of spec files: I think it is pretty rare for license files to include any licensing information at all.

Comment 15 Dave Love 2015-08-17 11:27:00 UTC
New Package SCM Request
=======================
Package Name: scalasca
Short Description: Toolset for scalable performance analysis of large-scale parallel applications
Upstream URL: http://www.scalasca.org/
Owners: loveshack
Branches: f21 f22 f23 el6 epel7
InitialCC:

Comment 16 Gwyn Ciesla 2015-08-17 13:24:53 UTC
Git done (by process-git-requests).

Comment 17 Zbigniew Jędrzejewski-Szmek 2015-08-17 23:41:08 UTC
OK, changing the status so this does not clutter up my search results.

Comment 18 Fedora Update System 2015-08-18 16:08:34 UTC
scalasca-2.2.2-2.fc23 has been submitted as an update for Fedora 23.
https://admin.fedoraproject.org/updates/scalasca-2.2.2-2.fc23

Comment 19 Fedora Update System 2015-08-19 08:04:24 UTC
scalasca-2.2.2-2.fc23 has been pushed to the Fedora 23 testing repository.

Comment 20 Fedora Update System 2015-08-31 18:53:54 UTC
scalasca-2.2.2-2.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update scalasca'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-13671

Comment 21 Fedora Update System 2015-09-08 09:17:27 UTC
scalasca-2.2.2-2.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-7965

Comment 22 Fedora Update System 2015-09-10 15:03:35 UTC
scalasca-2.2.2-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15536

Comment 23 Fedora Update System 2015-09-11 03:49:35 UTC
scalasca-2.2.2-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update scalasca'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15536

Comment 24 Fedora Update System 2015-09-26 17:34:06 UTC
scalasca-2.2.2-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 25 Fedora Update System 2015-09-27 15:57:15 UTC
scalasca-2.2.2-2.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.