Bug 226222

Going through the list on:

http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

Results of rpmlint
$ rpmlint oprofile-0.9.2-8.fc8.src.rpm 
W: oprofile buildprereq-use qt-devel
W: oprofile buildprereq-use libxslt
W: oprofile buildprereq-use docbook-style-xsl
W: oprofile buildprereq-use docbook-utils
W: oprofile buildprereq-use elinks
W: oprofile buildprereq-use gtk2-devel
W: oprofile buildprereq-use automake
W: oprofile buildprereq-use binutils-devel
W: oprofile mixed-use-of-spaces-and-tabs (spaces: line 76, tab: line 83)

Need static package.
- MUST: Static libraries must be in a -static package.


There is a GUI application for OProfile which doesn't follow the following

- MUST: Packages containing GUI applications must include a
%{name}.desktop file, and that file must be properly installed with
desktop-file-install in the %install section. This is described in
detail in the desktop files section of Packaging Guidelines. If you
feel that your packaged GUI application does not need a .desktop file,
you must put a comment in the spec file with your explanation.

This was merged long ago, closing.

Hi Jeff,
  Can you please fix following issues and build new oprofile in rawhide?

1) Use macros as per https://fedoraproject.org/wiki/Packaging:RPMMacros
Replace /etc occurrences with %{_sysconfdir} 
Replace /usr/share occurrences with %{_datadir} 

2) mixing of macro style should be avoided
you have used %{buildroot} and $RPM_BUILD_ROOT. See https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

3) Good if you will change
make DESTDIR=${RPM_BUILD_ROOT} install
to
make install DESTDIR=${RPM_BUILD_ROOT} INSTALL="install -p"

also add -p to all install commands 

This will make sure keeping upstream timestamps.

4)Buildroot is not required,%clean is not needed, remove cleaning of buildroot in %install.

5) see https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net and add source url as
Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz

6) Drop 
BuildRequires: binutils-devel
as this will get pulled by binutils-static

7) see http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Shared_libraries
and drop 

Requires(post): /sbin/ldconfig
Requires(postun): /sbin/ldconfig
Requires: /etc/ld.so.conf.d

8)Good to fix following rpmlint output messages (Included only messages that need to be fixed)

oprofile.src:21: W: macro-in-comment %{ix86}
oprofile.src:21: W: macro-in-comment %{arm}
oprofile.src:136: W: macro-in-comment %doc
oprofile.src:144: W: macro-in-comment %{_includedir}
oprofile.src:157: W: macro-in-comment %{_libdir}
oprofile.src: W: patch-not-applied Patch83: oprofile-0.9.3-xen.patch
oprofile.src: W: invalid-url Source0: oprofile-0.9.6.tar.gz
oprofile-devel.x86_64: W: summary-ended-with-dot C Header files and libraries for developing apps which will use oprofile.
oprofile-devel.x86_64: W: spurious-executable-perm /usr/lib64/libopabi.a
oprofile-devel.x86_64: W: spurious-executable-perm /usr/include/op_list.h
oprofile-devel.x86_64: W: spurious-executable-perm /usr/include/op_sample_file.h
oprofile-devel.x86_64: W: spurious-executable-perm /usr/include/op_config.h
oprofile-devel.x86_64: W: spurious-executable-perm /usr/lib64/liboputil.a
oprofile-devel.x86_64: W: spurious-executable-perm /usr/include/odb.h
oprofile-devel.x86_64: W: spurious-executable-perm /usr/include/op_types.h
oprofile-devel.x86_64: W: spurious-executable-perm /usr/lib64/libop.a
oprofile-devel.x86_64: W: spurious-executable-perm /usr/include/op_cpu_type.h
oprofile-devel.x86_64: W: spurious-executable-perm /usr/lib64/liboputil++.a
oprofile-devel.x86_64: W: spurious-executable-perm /usr/lib64/libodb.a
oprofile-devel.x86_64: W: spurious-executable-perm /usr/include/op_events.h
oprofile-gui.x86_64: W: summary-ended-with-dot C GUI for oprofile.

oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libopagent.a
oprofile-jit.x86_64: W: non-conffile-in-etc /etc/ld.so.conf.d/oprofile-x86_64.conf
oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libjvmti_oprofile.a
oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libjvmti_oprofile.so
oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libopagent.so

also, this should add -static package

Unfortunately no time in future to work on this. Removing myself.

Fresh review.

Good:

- rpmlint checks return:

oprofile.spec:129: W: macro-in-comment %doc
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

oprofile.spec:198: W: macro-in-%changelog %pre
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

Trivial to fix.

oprofile.src: W: spelling-error Summary(en_US) profiler -> profile, profiles, profiled
The value of this tag appears to be misspelled. Please double-check.

Ignore.

oprofile.x86_64: E: incorrect-fsf-address /usr/share/doc/oprofile-0.9.7/COPYING
oprofile-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/oprofile-0.9.7/daemon/liblegacy/p_module.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

Ignore, fixable upstream.

oprofile.x86_64: W: no-manual-page-for-binary opjitconv
Each executable in standard binary directories should have a man page.

oprofile.x86_64: W: no-manual-page-for-binary oprofiled
Each executable in standard binary directories should have a man page.

Fix if feasible.

oprofile.x86_64: W: dangerous-command-in-%postun userdel

We shouldn't remove created users or groups.
https://fedoraproject.org/wiki/Packaging:UsersAndGroups

oprofile-devel.x86_64: W: summary-ended-with-dot C Header files and libraries for developing apps which will use oprofile.
Summary ends with a dot.

Trivial fix.

oprofile-devel.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

Fix if feasible.

oprofile-gui.x86_64: W: summary-ended-with-dot C GUI for oprofile.
Summary ends with a dot.

Trivial to fix.

oprofile-gui.x86_64: W: spelling-error %description -l en_US oprof -> prof, proof, o prof
The value of this tag appears to be misspelled. Please double-check.

Ignore.

oprofile-gui.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

oprofile-gui.x86_64: W: no-manual-page-for-binary oprof_start
Each executable in standard binary directories should have a man page.

oprofile-jit.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

Fix if feasible.

oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libopagent.a
oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libjvmti_oprofile.a
oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libjvmti_oprofile.so
oprofile-jit.x86_64: W: devel-file-in-non-devel-package /usr/lib64/oprofile/libopagent.so
A development file (usually source code) is located in a non-devel package. If
you want to include source code in your package, be sure to create a
development package.

Should the .a be included, and should the .so files be in -devel?

oprofile-jit.x86_64: W: non-conffile-in-etc /etc/ld.so.conf.d/oprofile-x86_64.conf
A non-executable file in your package is being installed in /etc, but is not a
configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

Fix.

- package meets naming guidelines
- package meets packaging guidelines
- license ( ) OK, text in %doc, matches source

Says GPLv2, should be GPLv2+ and LGPLv2+.

- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files

I see two, remove or move to -static, or Provide -static in -devel.

- post/postun ldconfig ok
- devel requires base package n-v-r 

Let me know if you'd like me to commit fixes.

Ignore -static, I see that that was done, and I missed it.  

Also, I tried building on rawhide, and it wants java-1.6.0-openjdk, and so fails.

oprofile-0.9.7-3 going through the build system addresses most of the points in comment #3 and #6.

profiler is listed as a noun in:
http://www.merriam-webster.com/dictionary/profiler

Probably should remove the .a and .la files from oprofile-jit. However, oprofile-jit really does need the .so libraries.

Would like to remove the oprofile-devel sub-package entirely. the eclipse tools shouldn't require it any more.

Fantastic, thanks for the quick response!  APPROVED, closing.

Wow! this review is finished. Few years back I tried to review this but failed due to lack of package maintainer response.

Thanks Jon!

No problem. :)  A lot of packages sort of eventually fall into compliance anyway.