This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 629332 - Review Request: GoAccess - Apache web log analyzer
Review Request: GoAccess - Apache web log analyzer
Status: CLOSED DUPLICATE of bug 967253
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Martin Gieseking
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-01 13:12 EDT by Marco Ziesing
Modified: 2013-10-19 11:20 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-07-20 00:16:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
martin.gieseking: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Marco Ziesing 2010-09-01 13:12:13 EDT
Spec URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess.spec
SRPM URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess-0.3-1.fc13.src.rpm
Description: GoAccess is a free and open source Apache web log analyzer that provides fast and valuable HTTP statistics for system administrators that require a visual report on the fly.
Comment 1 David Nalley 2010-09-05 01:11:28 EDT
Marco: 
I can't sponsor you as a packager, however, I'll point out one or two things that jump out at me. 

First your source url doesn't conform to the package guidelines, see: 
https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

Second, your build requires are just a tad too explicit. 
You don't need to specify gcc or make. 
take a look here: 
http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

Your %files section has an empty %doc section and there are clearly docs that are appropriate to put in there. 

Your package has a license file, and it doesn't appear that's included either: 
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

You should use macros a bit more consistently - for instance in your files section it could be 
%{_mandir}/man1/*
instead of
/usr/share/man/man1/goaccess.1.gz
Comment 2 Martin Gieseking 2010-09-08 15:08:37 EDT
Hi Marco,

it seems your above linked SPEC file already contains the changes requested by David. However, you didn't upload a corresponding SRPM. Please always keep both files in sync, and bump the Release number every time you provide a modified package. Finally, post the URLs of the updated files so that the current reviewers get notified.

Here are some further comments:

- goaccess 0.3.1 has been released recently. Maybe you want to update the 
  package to the new version.

- According to the sources, the license seems to be GPLv2 only.

- You can also remove BuildRequires: glibc-devel, pkgconfig, autoconf, automake, 
  and libtool.

- In %build, drop the definition of variable CPPFLAGS. It's not required.

- Insert blank lines between the %changelog entries to increase legibility.
Comment 3 Marco Ziesing 2010-09-16 12:15:46 EDT
Hi David,
hi Martin,

thank you very much for your help.

I updated/uploaded the files for the current version.

Spec URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess.spec
SRPM URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess-0.3.2-1.fc13.src.rpm

I'm in contact with Gerardo from prosoftcorp cause of the wrong version number and file names on sf.net.
Comment 4 Martin Gieseking 2010-09-17 03:26:38 EDT
Hi Marco,

thanks for the update. The package looks much better now. Here are some more notes:

- In the latest upstream release, the copyright notice in the source files 
  has changed to GPLv2+, i.e. the text "or (at your option) any later version" 
  was added. Thus, you should update the License field accordingly.

- "Requires: autoconf" is still present 

- drop the "echo" statement in %prep to avoid redundant output

- Also, it's sufficient to use "%setup -q", as option "-n %{name}-%{version}" 
  is the default

- In the %changelog, replace "%build" with "%%build" to make rpmlint happy


$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
goaccess.src:54: W: macro-in-%changelog %build
goaccess.src: W: invalid-url Source0: http://downloads.sourceforge.net/goaccess/goaccess-0.3.2.tar.gz HTTP Error 404: Not Found
3 packages and 0 specfiles checked; 0 errors, 2 warnings.
Comment 5 Martin Gieseking 2010-09-17 03:34:49 EDT
(In reply to comment #4)
> - "Requires: autoconf" is still present 

I meant "BuildRequires: autoconf", of course. :)
Comment 6 Marco Ziesing 2010-09-20 05:25:50 EDT
Update:

Spec URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess.spec
SRPM URL:
 http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess-0.3.2-2.fc13.src.rpm

The author of GoAccess will fix the path on sf.net
Comment 7 Martin Gieseking 2010-09-24 05:44:04 EDT
When running goaccess without any options but with an additional argument, it crashes with a segfault. This should be reported upstream.

$ goaccess x

GoAccess - version 0.3.2 - Sep 24 2010 11:21:59

An error has occurred
Error occured at: parser.c - parse_log - 531
Message: An error has occurred while opening the log file. Make sure it exists.

Abgebrochen (Speicherabzug geschrieben)


BTW, I can sponsor you if you're willing to do two or three informal package reviews in order to show a (basic) understanding of the packaging guidelines. This is important because you're allowed to formally review and approve other packager's submissions, once you have been sponsored. For that reason, you should be familiar with the guidelines and the reviewing process. If you're still interested in joining the packager group, let me know. :)
Comment 8 Marco Ziesing 2010-09-27 04:34:37 EDT
Good morning,

bug is fixed, path on sf.net is correct now and update to version 0.3.3 is online:

Spec URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess.spec
SRPM URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess-0.3.3-1.fc13.src.rpm


@Martin: Yes, i'd like to review a few packages.
Comment 9 Martin Gieseking 2010-09-27 05:47:19 EDT
(In reply to comment #8)
> @Martin: Yes, i'd like to review a few packages.

OK, I'll contact you privately for further information.
Comment 10 Michael Schwendt 2010-10-05 05:07:01 EDT
> Vendor:         ProSoftCorp

http://fedoraproject.org/wiki/Packaging/Guidelines#Tags
|
| The Vendor tag should not be used. It is set automatically by the build system.

Further, it refers to the package vendor, currently the "Fedora Project".
Comment 11 Marco Ziesing 2010-12-22 09:23:25 EST
Hi,

sorry for the late response. Had a lot of pressure.

Spec URL: http://marco-ziesing.de/wp-content/uploads/2010/09/goaccess.spec
SRPM URL: http://marco-ziesing.de/wp-content/uploads/2010/12/goaccess-0.4.1-1.fc14.src.rpm
Comment 12 Christopher Meng 2013-07-20 00:16:47 EDT
Thank you for interested in this nice log software.

Unfortunately I've approved another request of this, and now goaccess is in Fedora. So I have to close this bug. Sorry for not asking your opinion before appproving that review.

Thanks for your concern again.

*** This bug has been marked as a duplicate of bug 967253 ***
Comment 13 Christopher Meng 2013-10-19 11:11:50 EDT
What are you doing here?
Comment 14 Björn "besser82" Esser 2013-10-19 11:20:19 EDT
Just cleaned the FE-NEEDSPONSOR bug from closed reviews.

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