Bug 226108 - Merge Review: lsof
Merge Review: lsof
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michal Nowak
Fedora Package Reviews List
:
Depends On:
Blocks: F9MergeReviewTarget
  Show dependency treegraph
 
Reported: 2007-01-31 14:35 EST by Nobody's working on this, feel free to take it
Modified: 2013-03-07 21:03 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-02-21 06:34:48 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mnowak: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:35:33 EST
Fedora Merge Review: lsof

http://cvs.fedora.redhat.com/viewcvs/devel/lsof/
Initial Owner: kzak@redhat.com
Comment 1 Ruben Kerkhof 2007-02-04 13:57:04 EST
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 16:42:45 EST
Thanks for your enthusiasm. I'm going to fix/improve the package in next week(s).
Comment 3 Karel Zak 2007-03-01 11:45:13 EST
(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 12:03:18 EST
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 13:29:37 EST
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 19:02:42 EDT
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 11:06:54 EDT
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 06:21:19 EDT
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 08:12:10 EST
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 08:55:26 EST
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.