Bug 182122 - Review Request: multitail
Summary: Review Request: multitail
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 Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-20 17:51 UTC by Folkert van Heusden
Modified: 2012-02-28 13:15 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-03-14 16:31:01 UTC
Type: ---
Embargoed:
gwync: fedora-cvs+


Attachments (Terms of Use)
Suggested specfile patch (1.58 KB, patch)
2006-02-22 19:44 UTC, Jason Tibbitts
no flags Details | Diff
Specfile patch - missing changelog entry, convert make to macro, ... (876 bytes, patch)
2006-02-23 15:51 UTC, Jose Pedro Oliveira
no flags Details | Diff

Description Folkert van Heusden 2006-02-20 17:51:48 UTC
Spec Name or Url: http://www.vanheusden.com/multitail/multitail.spec
SRPM Name or Url: http://www.vanheusden.com/multitail/multitail-3.8.6-0.src.rpm
Description: Lets you view several logfiles at once in a window, with colors and filtering.

Comment 1 Folkert van Heusden 2006-02-20 17:53:28 UTC
MultiTail lets you view one or multiple files like the original tail program.
The difference is that it creates multiple windows on your console (with
ncurses). It can also monitor wildcards: if another file matching the wildcard
has a more recent modification date, it will automatically switch to that file.
That way you can, for example, monitor a complete directory of files. Merging of
2 or even more logfiles is possible. It can also use colors while displaying the
logfiles (through regular expressions), for faster recognition of what is
important and what not. It can also filter lines (again with regular
expressions). It has interactive menus for editing given regular expressions and
deleting and adding windows. One can also have windows with the output of shell
scripts and other software. When viewing the output of external software,
MultiTail can mimic the functionality of tools like 'watch' and such.

Comment 2 udo 2006-02-20 18:17:39 UTC
http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions
Please remove gcc and make from BuildRequires

Comment 3 Folkert van Heusden 2006-02-20 18:21:50 UTC
done

Comment 4 udo 2006-02-20 18:24:30 UTC
# rpmlint /usr/src/redhat/SRPMS/multitail-3.8.6-0.src.rpm
W: multitail strange-permission multitail-3.8.6.tgz 0750

Comment 5 Jason Tibbitts 2006-02-21 04:02:09 UTC
I wanted to provide a full review for this, but the links to the spec and srpm
are invalid (no permissions).

Comment 6 Folkert van Heusden 2006-02-21 08:59:46 UTC
oh darn :-(
Ok, I fixed the permissions.

Comment 7 Jason Tibbitts 2006-02-22 17:00:45 UTC
Umm, I was in the process of reviewing this.  Why did it get closed?

Comment 8 Folkert van Heusden 2006-02-22 17:07:20 UTC
I have no idea. Should I resubmit it?

Comment 9 Jason Tibbitts 2006-02-22 19:43:27 UTC
OK, the bug is repoened and back in shape.  Now, onto the review:

Release: should start at 1 and should include disttag.
Don't set Vendor: in extras packages.
Buildroot should include user ID.
Requires: ncurses is not necessary; rpm will pick up the dependency.
Suggest using %setup -q for a quieter build.
Suggest fixing up line endings in HTML files to quiet rpmlint.
Pass %{optflags} to make.
Add the release to the most recent changelog entry.

I will attach a patch to fix these and to put the spec more into line with the
Extras template.

I don't know what it hurts, but as in a previous comment, rpmlint complains
about the permissions on the source tarball.  I suggest changing the permisions
on the tarball in the srpm to 0644.

Please consider including the actual text of the license in your tarball.  (The
package submitter is supposed to bug upstream to do this, but you're the
upstream so I guess I'm supposed to bug you.)

Good stuff:
After building with the patched spec, rpmlint is silent and the package builds
in mock (development, i386 and x86_64).
Source file matches upstream.
License is appropriate and matches source.

Fix up the above issues and I'll approve the package, or let me know what you
disagree with and we'll figure something out.

Comment 10 Jason Tibbitts 2006-02-22 19:44:27 UTC
Created attachment 125051 [details]
Suggested specfile patch

Comment 11 Jose Pedro Oliveira 2006-02-22 20:35:54 UTC
Two more minor suggestions:

* We only need to override CFLAGS (no need to override the value of CC).
  The following line resolves the distro CFLAGS inheritance problem:

     CFLAGS="%{optflags}" make %{?_smp_mflags}

* We can also drop the %doc tag from the man page entry in the %files section.
  (rpm automatically tags manpages as doc files).

Everything else looks good with Jason's patch applied.

/jpo

PS - And the package can also be updated to version 3.8.7 ;)

Comment 12 Jason Tibbitts 2006-02-22 20:51:55 UTC
Thanks; I had tried a different setting of CFLAGS but that broke the build. 
Everything works with your method.  I also didn't realize that manpages were
automatically tagged as %doc.

I also notice that 3.8.7 includes the license text in the tarball so I think
Folkert is on top of things.

Comment 13 Folkert van Heusden 2006-02-22 21:18:55 UTC
My guess is that that build problem is caused by the 'VERSION' define not being
set if omitted from the CFLAGS variable. Suggestions for generic fixes are
welcome but I would like to keep it somewhere in the Makefiles-parts as I then
only have to set it in one place when I release a new version.

Comment 14 Jose Pedro Oliveira 2006-02-22 21:31:46 UTC
(In reply to comment #12)
> Thanks; I had tried a different setting of CFLAGS but that broke the build. 
> Everything works with your method.  I also didn't realize that manpages were
> automatically tagged as %doc.

No problem. I have been maintaining my own multitail specfile for several years
now ;)

/jpo

Comment 15 Folkert van Heusden 2006-02-23 07:08:14 UTC
Ok, my good friend Udo van den Heuvel fixed the suggested changes and I've
uploaded the new spec and srpm-file to my site:
http://www.vanheusden.com/multitail/multitail-3.8.7-1.src.rpm
http://www.vanheusden.com/multitail/multitail.spec

Comment 16 Jose Pedro Oliveira 2006-02-23 14:35:49 UTC
Folkert,

Please do the following:
  * apply the changes mentioned in comment #11,
  * drop one of the summary lines (there are two of them),
  * drop the INSTALL file from the %doc line,
  * and drop desktop_vendor definition line (not used in the specfile)

jpo

Comment 17 Folkert van Heusden 2006-02-23 15:21:31 UTC
Done except the man-page thing: the packager told me that if he does that, then
rpm complains about found files that are not mentioned in the %files-section.

Comment 18 Folkert van Heusden 2006-02-23 15:23:33 UTC
http://www.vanheusden.com/multitail/multitail-3.8.7-2.src.rpm
spec at previous location

Comment 19 Jose Pedro Oliveira 2006-02-23 15:50:03 UTC
(In reply to comment #17)
> Done except the man-page thing: the packager told me that if he does that, then
> rpm complains about found files that are not mentioned in the %files-section.

Never saw that complain. Which version of rpm/distro is he using?
For RedHat/Fedora distros is safe to remove the %doc tag from man pages
file entries.

You also forgot to add the changelog entry (and the last one is missing the
release entry; rpmlint complains about these omissions). It is also custom to
mention the bugzilla entry in changelog entries.

jpo

Comment 20 Jose Pedro Oliveira 2006-02-23 15:51:15 UTC
Created attachment 125112 [details]
Specfile patch - missing changelog entry, convert make to macro, ...

Comment 21 udo 2006-02-23 15:56:04 UTC
I am using FC4.
rpm 4.4.1.


Comment 22 Jason Tibbitts 2006-02-23 16:07:11 UTC
Too many people in this review....

I get clean builds after taking out the %doc tag, but just on the file in
%{_mandir}.  You will certainly see problems if you remove it from the other line.

Jose's being picky about change "make" to a macro, but everything else in the
spec is using them so it makes sense for consistency.  He's right about the
changelogs, they should be kept consistent and the release tag needs to appear
for each change.

I would like to get this package done and out the door.  With Jose's last patch,
I'm ready to approve.  Let me do a couple of final mock builds first.

Comment 23 Folkert van Heusden 2006-02-23 17:04:21 UTC
Hopefully this one is the final, fingers crossed! :-)
http://www.vanheusden.com/multitail/multitail.spec
http://www.vanheusden.com/multitail/multitail-3.8.7-3.src.rpm

Comment 24 Jose Pedro Oliveira 2006-02-23 17:23:02 UTC
PUBLISH ++

The above line gives my approval (one vote for approval). As there is at least
one more reviewer he will also cast his vote. As Jason has taken the lead in the
review process he will be the one given the final approval.

/jpo

Just a picky note in the second changelog entry - the update statement release
number doesn't match the the one in the changelog entry header (2 != 3). This
can be corrected later.

Comment 25 Jason Tibbitts 2006-02-23 17:41:56 UTC
Yes, I agree, but do be careful about the changelog entries when you check in.

Approved.

Comment 26 Jason Tibbitts 2006-03-08 03:04:36 UTC
It turns out that a sponsor is needed here.  I cannot sponsor, so I'm not sure
what should happen to this package.

Comment 27 Jose Pedro Oliveira 2006-03-08 20:59:47 UTC
I will sponsor Folkert.

Importing the following SRPM
d05b27f3dcfee11ebd8f856ab8dadab3  multitail-3.8.7-3.src.rpm
with the tarball
b216c9e0d598e3e048b2c6738a54ba08  multitail-3.8.7.tgz

jpo

Comment 28 Jose Pedro Oliveira 2006-03-14 16:31:01 UTC
Closnig this ticket.
multitail 3.8.7 already in the mirrors.
multitail 3.8.9 will appear shortly (next push).

Comment 29 Jon Stanley 2012-02-27 21:31:45 UTC
Package Change Request
======================
Package Name: multitail
New Branches: EL-5 EL-6
Owners: jstanley
InitialCC: 

Current maintainer does not wish to maintain in EPEL (will continue in Fedora) - see bug 797925.

Comment 30 Gwyn Ciesla 2012-02-28 13:15:50 UTC
Git done (by process-git-requests).


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