Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226108 - Merge Review: lsof
Summary: Merge Review: lsof
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michal Nowak
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: F9MergeReviewTarget
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:35 UTC by Nobody's working on this, feel free to take it
Modified: 2013-03-08 02:03 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-02-21 11:34:48 UTC
mnowak: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:35:33 UTC
Fedora Merge Review: lsof

http://cvs.fedora.redhat.com/viewcvs/devel/lsof/
Initial Owner: kzak@redhat.com

Comment 1 Ruben Kerkhof 2007-02-04 18:57:04 UTC
Hi Karel,

Review for release 3:
* RPM name is OK
* This is the latest version
* Builds fine in mock
* File list looks OK

Needs work:
* BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
  (wiki: PackagingGuidelines#BuildRoot)
* Missing SMP flags. If it doesn't build with it, please add a comment
  (wiki: PackagingGuidelines#parallelmake)
* Package is marked as relocatable, please check.
  (wiki: PackagingGuidelines#RelocatablePackages)
* No downloadable source. Please give the full URL in the Source tag.
* Preserve timestamps when installing files
* Use {?dist} in Release tag

Rpmlint is not silent:

Source RPM:
W: lsof summary-ended-with-dot A utility which lists open files on a Linux/UNIX system.
W: lsof invalid-license Free
W: lsof no-url-tag
W: lsof redundant-prefix-tag
W: lsof macro-in-%changelog install

rpmlint of lsof:
W: lsof summary-ended-with-dot A utility which lists open files on a Linux/UNIX system.
W: lsof invalid-license Free
W: lsof no-url-tag


Comment 2 Karel Zak 2007-02-07 21:42:45 UTC
Thanks for your enthusiasm. I'm going to fix/improve the package in next week(s).

Comment 3 Karel Zak 2007-03-01 16:45:13 UTC
(In reply to comment #1)

> * No downloadable source. Please give the full URL in the Source tag.

Not possible. We create our own tarball with linux dialect only.

The rest is fixed. Update to lsof-4.78-5.fc7.


Comment 4 Jason Tibbitts 2007-03-01 17:03:18 UTC
How is the source tarball created, then?  The procedure needs to be documented
in the specfile; see http://fedoraproject.org/wiki/Packaging/SourceURL

Or is this a case where Fedora has simply forked the upstream code completely?

Comment 5 Karel Zak 2007-03-01 18:29:37 UTC
No fork. The reason are licences -- we have to remove non-linux things from
original tarball. There is README.maintainer in CVS that describe the way. 

http://cvs.devel.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/rpms/lsof/devel/README.maintainer?rev=1.1;content-type=text%2Fplain

Well, I'll add some notes to spec file (according to "When Upstream uses
Prohibited Code"
http://fedoraproject.org/wiki/Packaging/SourceURL#head-b46af5567f2338faafa13aff9855caa3c91f2c8c)

It's probably better than the README file.


Comment 6 Karel Zak 2007-10-02 23:02:42 UTC
I've added a note about tarball & script to the lsof.spec file. See lsof-4.78-6.fc8.

Comment 7 Michal Nowak 2009-04-08 15:06:54 UTC
Will do the Merge Review here, let's finish this one.
--

* `upstream2downstream.sh' -- could have license statement

* looks like there's new 4.82 version (not sure whether there are any Linux related stuff)

* %define lsofrh lsof_4.81-rh -- incorporate %{version} here

* 

[...]
# 184338 - allow lsof access nptl threads
Patch1: lsof_4.81-threads.patch
# 480694 - lsof manpage mismarked and formats badly
Patch2: lsof_4.81-man.patch
[...]
%patch1 -p1
%patch2 -p1
[...]

Patching starts from "Patch0" :)

* Don't mix macro and variable style 
  - $RPM_OPT_FLAGS -> %{optflags}
  - ${RPM_BUILD_ROOT} -> %{buildroot}

* %defattr(644,root,root,755) -> %defattr(-,root,root,-) it's the same and the later preferred

* %attr(0755,root,root) %{_sbindir}/lsof
`-------^^^^^---(not necessary)

--

The rest looks good.

Comment 8 Karel Zak 2009-04-29 10:21:19 UTC
Michal, thanks for your review. I'll fix all in the next rawhide update (probably together with some others issues).

Comment 9 Karel Zak 2010-02-19 13:12:10 UTC
Sorry for delay, I forgot...

(In reply to comment #7)
> * `upstream2downstream.sh' -- could have license statement

 well, "Public Domain" :-) -- note that the file is not distributed in srpm. It's in our CVS only.

> * %define lsofrh lsof_4.81-rh -- incorporate %{version} here

 Fixed.
 
> Patching starts from "Patch0" :)

 Fixed.

> * Don't mix macro and variable style 
>   - $RPM_OPT_FLAGS -> %{optflags}
>   - ${RPM_BUILD_ROOT} -> %{buildroot}

 Ah.. so pedentic ;-) Not fixed. I don't think it's more readable.

> * %defattr(644,root,root,755) -> %defattr(-,root,root,-) it's the same and the
> later preferred

 Fixed.

> * %attr(0755,root,root) %{_sbindir}/lsof
> `-------^^^^^---(not necessary)

 Fixed.

All changes are avaialable in lsof-4.83-2.fc14, Thanks  for review.

Comment 10 Michal Nowak 2010-02-19 13:55:26 UTC
Thanks for doing that. Approved (or whatever word is correct here).


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