Bug 226108 - Merge Review: lsof
Merge Review: lsof
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
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:
Last Closed: 2010-02-21 06:34:48 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
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

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. 


Well, I'll add some notes to spec file (according to "When Upstream uses
Prohibited Code"

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

> Patching starts from "Patch0" :)


> * 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


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


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.