Bug 250504 - Review Request: blktrace - block IO tracer
Summary: Review Request: blktrace - block IO tracer
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 472898
TreeView+ depends on / blocked
 
Reported: 2007-08-01 22:23 UTC by Eric Sandeen
Modified: 2009-02-16 19:48 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2007-09-04 21:42:39 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Eric Sandeen 2007-08-01 22:23:02 UTC
Spec URL: http://people.redhat.com/esandeen/blktrace-rpm/blktrace.spec
SRPM URL: http://people.redhat.com/esandeen/blktrace-rpm/blktrace-0.0-0.1.20070730162628git.src.rpm
Description: blktrace is a block layer IO tracing mechanism which provides detailed
information about request queue operations to user space.  This package
includes both blktrace, a utility which gathers event traces from the kernel;
and blkparse, a utility which formats trace data collected by blktrace.

Comment 1 Jason Tibbitts 2007-08-01 22:40:09 UTC
This fails to build for me.

There are errors about not finding ghostscript while building the documentation:

Transcript written on btt.log.
[1][2][3][4][5][6][7][8][9][10(./activity.eps<PS>sh: gs: command not found

and then install fails (sorry if this wraps badly):

+ rm -rf /var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild
+ make dest=/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild prefix=/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild//usr mandir=/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man install
install -m755 -d /var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild//usr/bin
install -m755 -d /var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man/man1
install -m755 -d /var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man/man8
install blkparse blktrace verify_blkparse blkrawverify btrace btt/btt
/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild//usr/bin install doc/*.1
/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man/man1 install doc/*.8
/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man/man8
+ chmod -R -x /var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man/
chmod: cannot access `/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man/man1': Permission denied
chmod: cannot access `/var/tmp/blktrace-0.0-0.1.20070730162628git.fc8-root-mockbuild/usr/share/man/man8': Permission denied
error: Bad exit status from /var/tmp/rpm-tmp.28627 (%install)

This is in mock on x86_64 rawhide.


Comment 2 Eric Sandeen 2007-08-01 22:46:44 UTC
Ok, sorry.  I honestly just took a guess at BuildRequires... and am not familiar
enough w/ mock I guess.  Does just adding ghostscript to BuildRequires fix that up?

Hrm, not sure about the install failing; the man pages were installed as
executable by default, and rpmlint complained, so I chmod -x'd them in
%install... I suppose that's cheating.  Any other suggestion?

Thanks,

-Eric

Comment 3 Eric Sandeen 2007-08-01 23:04:06 UTC
Doh, %attr would probably work fine eh :)

Ok, newer version put up
http://people.redhat.com/esandeen/blktrace-rpm/blktrace-0.0-0.2.20070730162628git.src.rpm

Thanks,
-Eric

Comment 4 Jason Tibbitts 2007-08-01 23:18:09 UTC
You can do scratch builds in koji, you know.  Assuming you already have access,
that is. 
  koji build --scratch dist-fc8 blah.src.rpm

Yes, adding ghostscript helps to get rid of the gs-related errors.


Comment 5 Jason Tibbitts 2007-08-01 23:30:17 UTC
Unfortunately that URL in comment 3 is 404 for me.

One other thing I noticed is that the package naming is a bit weird.  I guess it
comes from upstream, though upstream doesn't have the "git" tacked on at the
end.  And what's the "62628" mean, anyway?  I don't really see it as a problem,
though.

Comment 6 Eric Sandeen 2007-08-02 17:17:47 UTC
> Unfortunately that URL in comment 3 is 404 for me.

*sigh* fat fingered the rsync.  It's there now.

> One other thing I noticed is that the package naming is a bit weird.  I guess it
> comes from upstream, though upstream doesn't have the "git" tacked on at the
> end.  And what's the "62628" mean, anyway?  I don't really see it as a 
> problem, though.

Upstream is something like "blktrace-git-20070730162628.tar.gz"

162628 is a timestamp, 16:26:28.  It's how the snap from the upstream tarball is
named.  I put "git" on the end just trying to follow some of the "pre-release
naming conventions" I found on the wiki.  I could use a git hash instead if
preferred...  since there is no release on this upstream package, I'm not really
sure how to do the versioning.  I'm open to any suggestions.

I'm also considering skipping the latex->pdf part of the build process, would
lighten up the build & chroot setup a bit, for a little hacker tool like this. 
There are already man pages & a README.

Thanks,

-Eric

Comment 7 Eric Sandeen 2007-08-02 20:59:56 UTC
FWIW, taking out the pdf build knocks the build chroot down to about 100 rpms
vs. 145.

Comment 8 Eric Sandeen 2007-08-14 20:14:36 UTC
I put up a newer version (0.3) which removes the pdf build.

-Eric

Comment 9 Jason Tibbitts 2007-08-15 05:07:44 UTC
I'm sorry for taking so long with this, but lately I've been having trouble
finding any free time.

The license should be GPLv2+ according to the information in the source files.

Other than a complaint about the license, rpmlint is silent.

The "pre-release naming conventions" it looks like you're following relate to
when you have no Source URL and are constructing a snapshot from upstream's SCM.
 In this case you have a tarball should follow upstream's naming conventions. 
That said, I have no particular issue with moving the "git" bit to the end.

There's a COPYING file in the tarball; it needs to be included in the built package.

The proper CFLAGS aren't passed to the compiler, although the debuginfo package
looks OK because -g is in the default set.  Unfortunately the Makefiles are
rather poor; I added CFLAGS="%{optflags}" to the make line and that gets the
proper flags passed, but this causes things to fail to build.  Looks like some
Makefile patching is in order.

* source files match upstream:
   70aa39a254b6c4007e5f184829a5fb7cae354efbf6801883f49853519a02cf99
   blktrace-git-20070730162628.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
X license field should be GPLv2+
* license is open source-compatible.
X license text is in tarball but not included in package.
* latest version is being packaged.
* BuildRequires are proper.
X compiler flags are not appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint complains about the license.
* final provides and requires are sane:
   blktrace = 0.0-0.3.20070730162628git.fc8
  =
   /bin/sh
   libpthread.so.0()(64bit)
   libpthread.so.0(GLIBC_2.2.5)(64bit)
* %check is not present; no test suite upstream.  The executables seem to run 
   without crashing, but outside of that I've no idea how to test this package.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Comment 10 Eric Sandeen 2007-08-15 05:11:17 UTC
Thanks Jason - I'll fix up those last bits (guess I need a newer rpmlint for
some of this!)

-Eric

Comment 11 Eric Sandeen 2007-08-17 04:39:38 UTC
Ok, blktrace-0.0-0.4.20070730162628git.src.rpm should have those last bits fixed up.

-Eric

Comment 12 Jason Tibbitts 2007-09-02 17:42:02 UTC
Oh, man, somehow I missed that last notification and let this sit around for
another two weeks.  Too much bugspam, I guess.  I'm really sorry about that; you
should have felt free to ping me if I didn't get back after a couple of days.

Everything builds OK, compiler flags and debuginfo look good to me, and the
license tag is fine.

APPROVED

Comment 13 Eric Sandeen 2007-09-02 22:32:17 UTC
Thanks Jason, no worries - I've been busy too.  If this were anywhere near the
top of my list of priorities, I'd have pinged you.  :)

Thanks,
-Eric

Comment 14 Eric Sandeen 2007-09-04 15:42:53 UTC
New Package CVS Request
=======================
Package Name: blktrace
Short Description: block IO tracer
Owners: sandeen
Branches: devel
InitialCC: 
Cvsextras Commits: yes

Thanks,
-Eric

Comment 15 Kevin Fenzi 2007-09-04 19:04:00 UTC
cvs done.

Comment 16 Eric Sandeen 2007-09-04 21:42:39 UTC
Built

Comment 17 Eric Sandeen 2008-04-10 15:11:32 UTC
Package Change Request
======================
Package Name: blktrace
New Branches: EL-5


Comment 18 Kevin Fenzi 2008-04-10 17:22:35 UTC
cvs done.


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