Bug 285571 - Review Request: seekwatcher - IO visualization with blktrace
Review Request: seekwatcher - IO visualization with blktrace
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-10 23:27 EDT by Eric Sandeen
Modified: 2008-04-10 13:26 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-12-04 07:33:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Eric Sandeen 2007-09-10 23:27:12 EDT
Spec URL:
 http://people.redhat.com/esandeen/seekwatcher-rpm/seekwatcher.spec

SRPM URL:
 http://people.redhat.com/esandeen/seekwatcher-rpm/seekwatcher-0.7-1.src.rpm

Description:
Seekwatcher generates graphs from blktrace runs to help visualize IO patterns
and performance. It can plot multiple blktrace runs together, making it easy
to compare the differences between different benchmark runs.

You should install the seekwatcher package if you need to visualize detailed
information about IO patterns.
Comment 1 Parag AN(पराग) 2007-09-24 05:55:16 EDT
rpmlint on SRPM gave me
seekwatcher.src: W: no-%build-section
The spec file does not contain a %build section.  Even if some packages
don't directly need it, section markers may be overridden in rpm's
configuration to provide additional "under the hood" functionality, such as
injection of automatic -debuginfo subpackages.  Add the section, even if
empty.


Where can I find mencoder?

package installation failed with error: Failed dependencies:
        mencoder is needed by seekwatcher-0.7-1.fc8.noarch

Comment 2 Parag AN(पराग) 2007-09-24 06:12:13 EDT
Just confirmed on #fedora-devel and this package cannot make into Fedora because
of missing package dependency mencoder.

If by any chance you can use any alternate to mencoder then we can think this
for inclusion in Fedora.
Comment 3 Eric Sandeen 2007-09-24 09:08:14 EDT
Thanks for the review; I'll add a %build.

Sorry about mencoder, that was a thinko on my part.  seekwatcher can still make
still graphs, just not movies.  So I think we can get it in with limited
functionality.  I'll see about patching it up to remove the option, or something
along those lines.

-Eric
Comment 4 manuel wolfshant 2007-09-24 09:21:47 EDT
Eric how about either trying to build a modular version and submit the mencoder
dependent part to rpmfusion or maybe submit the whole package to rpmfusion ?
Comment 5 Eric Sandeen 2007-09-24 10:48:32 EDT
hmm apparently the beta1 theora release has png2theora in theora-tools... pity
F8 doesn't have that release, yet... maybe I can ask for just that to be
included in the current package.  Chris Mason would be willing to use those free
tools rather than the mpg stuff.  I'll take a look.

-Eric
Comment 6 Parag AN(पराग) 2007-09-24 13:38:10 EDT
(In reply to comment #3)
> Thanks for the review; I'll add a %build.
ok
> 
> Sorry about mencoder, that was a thinko on my part.  seekwatcher can still make
> still graphs, just not movies.  So I think we can get it in with limited
> functionality.  I'll see about patching it up to remove the option, or something
> along those lines.
> 
  Will be happy to review this then :)
Comment 7 Eric Sandeen 2007-09-24 17:46:34 EDT
Well, spent too much time on this today :)  png2theora is already in the
libtheora src.rpm we have, just not installed.  I have it making a "movie" but
something is odd; it looks pretty wrong.

-Eric
Comment 8 Parag AN(पराग) 2007-10-15 10:14:21 EDT
any progress here?
Comment 9 Eric Sandeen 2007-10-15 13:14:02 EDT
Nope.  The theora guys are stumped too, but the movie comes out wrong.

The static graph side of things works, though.  It's just the animated movies
that don't have a solution at this point.

-Eric
Comment 10 Eric Sandeen 2007-11-06 12:05:09 EST
I may just patch out the movie-making part, or maybe better yet make it dynamic
so it's available *if* mencoder or other suitable app is found.

Sorry for the delay on this, it's a low priority for me even though I'd like to
get it in.

--Eric
Comment 11 Eric Sandeen 2007-11-21 00:09:30 EST
very latest seekwatcher repo has a patch which simply makes the movie options
"go away" if mencoder is not found.  When chris makes that a point release, I'll
make a new package for review.

Thanks,
-Eric
Comment 12 Eric Sandeen 2007-11-29 15:49:12 EST
See also bug #401681 : Add filtering flags to png_read_png ... the (now) very
latest seekwatcher can use png2theora, and with the fix in the aforementioned
bug, it works.  So, I'll try to wrap this one up soon.

Thanks,
-Eric
Comment 14 Parag AN(पराग) 2007-11-29 23:43:01 EST
looks missing python-devel
In build.log I got
/usr/lib/rpm/pythondeps.sh: line 8: python: command not found
Comment 15 Eric Sandeen 2007-11-30 00:06:56 EST
argh, mock produced the rpm so didn't check the logs.

... fix fix fix ...
Comment 16 Eric Sandeen 2007-11-30 00:24:31 EST
Added python to reqs/buildreqs

http://people.redhat.com/esandeen/seekwatcher-rpm/seekwatcher-0.8-2.fc8.src.rpm

No complaints from mock this time.

Thanks,
-Eric
Comment 17 Eric Sandeen 2007-11-30 17:45:29 EST
And keeping up with chris....

http://people.redhat.com/esandeen/seekwatcher-rpm/seekwatcher-0.9-1.fc8.src.rpm
Comment 18 Parag AN(पराग) 2007-11-30 22:15:22 EST
(In reply to comment #17)
> And keeping up with chris....
> 
> http://people.redhat.com/esandeen/seekwatcher-rpm/seekwatcher-0.9-1.fc8.src.rpm

with this SRPM I got,

seekwatcher.noarch: I: checking
seekwatcher.noarch: W: incoherent-version-in-changelog 0.9 0.9-1.fc8
The last entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

seekwatcher.src: I: checking
seekwatcher.src: W: no-%build-section
The spec file does not contain a %build section.  Even if some packages
don't directly need it, section markers may be overridden in rpm's
configuration to provide additional "under the hood" functionality, such as
injection of automatic -debuginfo subpackages.  Add the section, even if
empty.

Also,
  make sure to keep timestamps
http://fedoraproject.org/wiki/Packaging/Guidelines#head-0239576e441f9ef53d175c4aec8c12868dffb5ab

And if you like you can use defattr as
defattr(-,root,root,-)
Comment 19 Eric Sandeen 2007-11-30 23:26:30 EST
*sigh* I guess maybe I'm just really really bad at this ;-)

Is that rpmlint output?  Ok, now my build log looks clean and rpmlint too.

Ok... once more with feeling:

http://people.redhat.com/esandeen/seekwatcher-rpm/seekwatcher-0.9-2.fc8.src.rpm

Thanks for puttting up with me :)  Hope I've got it now.

-Eric
Comment 20 Parag AN(पराग) 2007-12-01 01:36:10 EST
Thanks for updates.
Comment 21 Parag AN(पराग) 2007-12-01 01:43:10 EST
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream url
e4c554577e9298fc70fd162bdc647746  seekwatcher-0.9.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ BuildRequires are proper.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no static libraries.
+ no .pc file present.
+ -devel subpackage does exist.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no scriptlets present.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Not a GUI App.
APPROVED.
Comment 22 Eric Sandeen 2007-12-01 10:46:26 EST
Thanks for the review(s)!
Comment 23 Kevin Fenzi 2007-12-01 12:55:22 EST
Can you add a template here with what branches you want and such?
See: 
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

Reset the fedora-cvs flag when you are ready. 
Comment 24 Eric Sandeen 2007-12-01 15:04:42 EST
Sorry 'bout that:

New Package CVS Request
=======================
Package Name: seekwatcher
Short Description: Utility for visualizing block layer IO patterns and performance
Owners: sandeen
Branches: F-8
InitialCC:
Cvsextras Commits: yes

Thanks,
-Eric
Comment 25 Eric Sandeen 2007-12-02 00:28:32 EST
Imported & built... thanks everyone!
Comment 26 Eric Sandeen 2008-04-10 11:11:16 EDT
Package Change Request
======================
Package Name: seekwatcher
New Branches: EL-5
Comment 27 Kevin Fenzi 2008-04-10 13:26:15 EDT
cvs done.

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