Bug 498490

Whoops -- updated Group.

Spec URL: http://mattdm.org/misc/fedora/smem.spec
SRPM URL: http://mattdm.org/misc/fedora/smem-0.1-2.fc11.mattdm.src.rpm

rpmlint complaints a bit

smem.src:31: W: setup-not-quiet
- perhaps
%setup -q

smem.src: E: no-cleaning-of-buildroot %install
- install can become
%install
rm -rf $RPM_BUILD_ROOT
install -p -D -m 755 smem $RPM_BUILD_ROOT/%{_bindir}/smem

smem.noarch: W: spurious-executable-perm /usr/share/doc/smem-0.1/capture
- perhaps
chmod -x capture

builds OK in mock

Updated to fix those. Thanks.

Spec URL: http://mattdm.org/misc/fedora/smem.spec
SRPM URL: http://mattdm.org/misc/fedora/smem-0.1-3.fc11.mattdm.src.rpm

Matthew,

I see you submitted your package before me while I was waiting for Matt to add the license. No problem.
My spec is here http://michich.fedorapeople.org/smem/smem.spec
Take a look at it and take whatever you think is a good idea.
Some things worth considering:
- I did not add the PDF as documentation because I did not think is was very useful and it made the package much bigger.
- I have Requires: python python-matplotlib
  matplotlib is needed for graphical output of smem. Unfortunately rpmlint complains: E: explicit-lib-dependency python-matplotlib . I don't know if it's a real problem or not.
- I installed the capture script as /usr/sbin/smem-capture , but on second thought I believe that installing it as documentation should be enough (as you did it).

Michal

Thanks Michal.

I could take or leave the PDF -- I included it because documentation is scarce and it's only a hundred K. Still, 100k here, 100k there, and soon you have a megabyte. :)

I left off the matplotlib dep kinda on purpose, because the package works fine in its basic usage without it. That may need revisited as the program itself grows.

I'm considering also adding a requirement on kernel >= 2.6.27, because the program in its current state appears to work but gives incorrect results on older kernels. Thoughts on that?

F10 was released with 2.6.27. It might make sense for F9, but even there we had 2.6.27 in updates since November. And such a dependency will not have any effect when someone has more kernel versions installed and boots into the old one.

My concern is basically to act as a warning flag for EPEL.

> works fine in its basic usage without it. 

and fails gracefully if its not installed. Not to mention the ridiculous depchain difference between only python and pythin-matplotlib.


> I'm considering also adding a requirement on kernel >= 2.6.27, because the
> program in its current state appears to work but gives incorrect results on
> older kernels. Thoughts on that?  

providing incorrect results without a hint should be an upstream bug.

> providing incorrect results without a hint should be an upstream bug.  

Yeah. First two posts on the project mailing list, in fact:
http://selenic.com/pipermail/smem/2009-April/000001.html

But it seems like the right thing to do in the meantime. And when the issue is fixed, it'll still be a dependency. (Since this program pokes into kernel information, it's not unreasonable for it to actually be marked as requiring the kernel, since it requires it in a much more specific way than most programs who just happen to run on top of it do.)

Kernel version check patch submitted upstream:
http://selenic.com/pipermail/smem/2009-April/000016.html

Taking into review.

rpmlint output is clean.


MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
MUST: The spec file for the package is legible and macros are used consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK

MUST: The License field in the package spec file must match the actual license. NEEDSFIX
- There is nothing mentioned about a license in the tarball => the tarball has no license.
- Since the GPLv2+ license is in upstream svn, use a svn revision for now, until upstream releases a stable tarball with a license. (Of course, you can ask upstream to make a new stable release with the license info ASAP.)

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. OK

MUST: Optflags are used and time stamps preserved. NEEDSFIX
- Use "cp -a" instead of plain "cp" in setup phase.
- Also, use
 install -D -p -m 755 smem $RPM_BUILD_ROOT/%{_bindir}/smem
to preserve the time stamp of the python file itself. -D creates the directory if necessary, so you can drop the dir creation line:
 install -p -d $RPM_BUILD_ROOT/%{_bindir}

MUST: Packages containing shared library files must call ldconfig. OK
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. OK
MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK
MUST: Desktop files are installed properly. OK
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK

SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. NEEDSFIX
- Use the svn revision until upstream releases a new tarball with license information included.

SHOULD: The package builds in mock. OK

(In reply to comment #9)
> > works fine in its basic usage without it. 
> 
> and fails gracefully if its not installed. Not to mention the ridiculous
> depchain difference between only python and python-matplotlib.

Would be nice if upstream could separate gui stuff in a module such that we could ship smem and smem-gui, with smem-gui having a dep on python-matplotlib.

(update - can't work on this this weekend but will monday or tuesday.)

Okay, updated.

Spec URL: http://mattdm.org/misc/fedora/smem.spec
SRPM URL: http://mattdm.org/misc/fedora/smem-0.1-4.fc11.mattdm.src.rpm

Okay, looks good,


APPROVED

Michal Schmidt -- do you want to be co-owner of this package?

Matthew,
yes please. Thanks.

New Package CVS Request
=======================
Package Name: smem
Short Description: Report application memory usage in a meaningful way
Owners: mattdm mschmidt
Branches: F-10 F-11
InitialCC:

cvs done.

smem-0.1-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/smem-0.1-4.fc11

smem-0.1-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/smem-0.1-4.fc10

Package imported, built on devel, F-11 and F-10, updates submitted. Closing this review. Thanks everyone.

smem-0.1-4.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

smem-0.1-4.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: smem
New Branches: el6 epel7
Owners: michich mattdm

Git done (by process-git-requests).