Bug 226437 - Merge Review: strace
Summary: Merge Review: strace
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:02 UTC by Nobody's working on this, feel free to take it
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2007-11-21 04:33:45 UTC
Type: ---
Embargoed:
kevin: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 21:02:18 UTC
Fedora Merge Review: strace

http://cvs.fedora.redhat.com/viewcvs/devel/strace/
Initial Owner: roland

Comment 1 Kevin Fenzi 2007-02-18 05:27:38 UTC
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (BSD)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
ef40944118841803391d212cb64d3c5b  strace-4.5.15.tar.bz2
ef40944118841803391d212cb64d3c5b  strace-4.5.15.tar.bz2.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:

SHOULD Items:

See below - Should build in mock.
See below - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version
3 outstanding bugs - check for outstanding bugs on package.

Issues:

1. Might consider adding the COPYRIGHT file as a %doc.
Additionally: Changelog CREDITS NEWS PORTING TODO
might also be nice to have as doc files.

2. Please use the approved buildroot:
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

3. If this package can avoid it, please don't use '%makeinstall'.
Instead use 'make DESTDIR=%{buildroot} install' instead.
See: http://www.fedoraproject.org/wiki/PackagingDrafts/MakeInstall

4. Our pal rpmlint says:

a)
W: strace summary-ended-with-dot Tracks and displays system calls associated
with a running process.

Suggest: remove . at the end of the summary.

b)
W: strace macro-in-%changelog patch

Suggest: change the "%patch" in the changelog to "%%patch" to make sure rpm
doesn't expand it as a macro.

5. This package doesn't seem to compile under mock for i386/x86_64 in devel.

The build ends in:

net.c: In function 'printsock':
net.c:957: error: field 'nl' has incomplete type
make[1]: *** [net.o] Error 1

Can you duplicate this problem there?
I ran the above checks against the fc6 version for now, but once it
builds I will want to make a recheck for devel.

6. Only 3 outstanding bugs, and none of them seem directly related to
packaging. You might want to take a look at them and see if any of them
can be addressed while you are making the above changes.

7. Why the strace64_arches sections? It seems to contain ppc64, but
I don't think thats a platform fedora currently builds for.

8. Minor: might add '%{?_smp_mflags}' to the make line to support
faster builds on multi cpu machines.

9. Is there a reason to not ship the 'strace-graph' binary?

Comment 2 Kevin Fenzi 2007-02-24 03:25:19 UTC
Resetting flags and such per the new offical review guidelines. 
https://www.redhat.com/archives/fedora-maintainers/2007-February/msg00682.html

Comment 3 Roland McGrath 2007-11-20 21:56:57 UTC
I think all these issues are resolved in the F8 (maybe even F7) rpms.
Please complete the review.

strace-graph has never been included or supported in any Fedora or RHL release
before.  I don't intend to start maintaining it now.

Comment 4 Kevin Fenzi 2007-11-21 04:03:14 UTC
Yes indeed. All above items are taken care of. Thanks for looking at it. 

The only very minor issue I see now is: 

strace.x86_64: W: file-not-utf8 /usr/share/doc/strace-4.5.16/ChangeLog
strace.x86_64: W: file-not-utf8 /usr/share/doc/strace-4.5.16/CREDITS

If you like, you could run 'iconv' and make sure those files are utf8. 
No big deal. 

You are of course free to not ship strace-graph, but do you think it would be an
additional support burden?

This merge review is APPROVED. Feel free to close RAWHIDE. 

Comment 5 Roland McGrath 2007-11-21 04:33:45 UTC
I converted those files, so they won't be flagged in the next version when it's
released.


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