Bug 498490 - Review Request: smem - Reports application memory usage in a meaningful way
Summary: Review Request: smem - Reports application memory usage in a meaningful way
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-04-30 17:42 UTC by Matthew Miller
Modified: 2014-03-07 19:30 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-05-21 21:32:34 UTC
Type: ---
Embargoed:
susi.lehtola: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Matthew Miller 2009-04-30 17:42:36 UTC
Spec URL: http://mattdm.org/misc/fedora/smem.spec
SRPM URL: http://mattdm.org/misc/fedora/smem-0.1-1.fc11.mattdm.src.rpm
Description: 

smem is a tool that can give numerous reports on memory usage on Linux
systems. Unlike existing tools, smem can report proportional set size (PSS),
which is a more meaningful representation of the amount of memory used by
libraries and applications in a virtual memory system.

Because large portions of physical memory are typically shared among
multiple applications, the standard measure of memory usage known as
resident set size (RSS) will significantly overestimate memory usage. PSS
instead measures each application's "fair share" of each shared area to give
a realistic measure.

Comment 1 Matthew Miller 2009-04-30 18:00:48 UTC
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

Comment 2 Yanko Kaneti 2009-04-30 19:43:59 UTC
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

Comment 3 Matthew Miller 2009-04-30 19:50:43 UTC
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

Comment 4 Michal Schmidt 2009-04-30 20:15:08 UTC
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

Comment 5 Matthew Miller 2009-04-30 20:25:25 UTC
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.

Comment 6 Matthew Miller 2009-04-30 20:33:14 UTC
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?

Comment 7 Michal Schmidt 2009-04-30 20:40:31 UTC
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.

Comment 8 Matthew Miller 2009-04-30 20:46:20 UTC
My concern is basically to act as a warning flag for EPEL.

Comment 9 Yanko Kaneti 2009-04-30 20:47:42 UTC
> 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.

Comment 10 Matthew Miller 2009-04-30 20:52:41 UTC
> 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.)

Comment 11 Jeff Schroeder 2009-05-01 01:04:24 UTC
Kernel version check patch submitted upstream:
http://selenic.com/pipermail/smem/2009-April/000016.html

Comment 12 Susi Lehtola 2009-05-01 06:58:30 UTC
Taking into review.

Comment 13 Susi Lehtola 2009-05-01 07:26:26 UTC
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

Comment 14 Terje Røsten 2009-05-01 21:14:36 UTC
(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.

Comment 15 Matthew Miller 2009-05-03 20:46:05 UTC
(update - can't work on this this weekend but will monday or tuesday.)

Comment 16 Matthew Miller 2009-05-06 13:58:44 UTC
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

Comment 17 Susi Lehtola 2009-05-06 14:52:01 UTC
Okay, looks good,


APPROVED

Comment 18 Matthew Miller 2009-05-12 15:33:01 UTC
Michal Schmidt -- do you want to be co-owner of this package?

Comment 19 Michal Schmidt 2009-05-12 16:19:22 UTC
Matthew,
yes please. Thanks.

Comment 20 Matthew Miller 2009-05-12 17:51:29 UTC
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:

Comment 21 Kevin Fenzi 2009-05-13 05:13:16 UTC
cvs done.

Comment 22 Fedora Update System 2009-05-21 21:20:54 UTC
smem-0.1-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/smem-0.1-4.fc11

Comment 23 Fedora Update System 2009-05-21 21:29:51 UTC
smem-0.1-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/smem-0.1-4.fc10

Comment 24 Michal Schmidt 2009-05-21 21:32:34 UTC
Package imported, built on devel, F-11 and F-10, updates submitted. Closing this review. Thanks everyone.

Comment 25 Fedora Update System 2009-05-25 21:22:15 UTC
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.

Comment 26 Fedora Update System 2009-05-25 21:24:59 UTC
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.

Comment 27 Michal Schmidt 2014-03-07 18:14:17 UTC
Package Change Request
======================
Package Name: smem
New Branches: el6 epel7
Owners: michich mattdm

Comment 28 Gwyn Ciesla 2014-03-07 19:30:02 UTC
Git done (by process-git-requests).


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