Bug 564537 - Review Request: grc - simple python logfile colouriser
Summary: Review Request: grc - simple python logfile colouriser
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Buland Kumar Singh
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-02-13 03:32 UTC by Carl van Tonder
Modified: 2014-07-12 13:24 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
: 1200038 (view as bug list)
Environment:
Last Closed: 2014-07-12 13:24:32 UTC
Type: ---
Embargoed:
i.am.fedora.bk: fedora-review?


Attachments (Terms of Use)

Description Carl van Tonder 2010-02-13 03:32:21 UTC
Spec URL: http://supervacuo.com/fedora/grc.spec
SRPM URL: http://supervacuo.com/fedora/grc-1.3-0.1.fc12.svac.src.rpm
Description: Generic Colouriser is yet another colouriser for beautifying your logfiles or
output of commands.

grc is a simple python-based log colouriser, which is packaged (by the upstream author) in Ubuntu/Debian, but no rpm apparently exists. The license is slightly unclear, but I have assumed GPLv2 on the basis of 
http://packages.debian.org/changelogs/pool/main/g/grc/grc_1.3/grc.copyright (which doesn't appear to be in the normal grc tarball).

Comment 1 Terje Røsten 2010-02-13 12:52:24 UTC
Some comments:


Release:        0.1%{?dist}.svac
Why so uncommon release tag, what't wrong with 1%{?dist}?

%build

- Add a comment about why %build in empty.

%install
rm -rf $RPM_BUILD_ROOT
%{_builddir}/grc-1.3/install.sh %{_builddir}/grc-1.3/ $RPM_BUILD_ROOT

- %install will cd into source dir, you should use:
  ./install.sh  $RPM_BUILD_ROOT
 I don't understand why you need the build patch, (which is a install patch:-)

Comment 2 Carl van Tonder 2010-02-13 13:45:30 UTC
Spec URL: http://supervacuo.com/fedora/grc.spec
SRPM URL: http://supervacuo.com/fedora/grc-1.3-0.2.fc12.src.rpm

> Why so uncommon release tag, what't wrong with 1%{?dist}?
It started life as a "personal use" package; fixed

> Add a comment about why %build in empty.
Done; it's because there is no "compilation" step — grc and grcat are python scripts

> %install will cd into source dir, you should use:
> ./install.sh  $RPM_BUILD_ROOT
> I don't understand why you need the build patch, (which is a install patch:-)
For some reason, specifying relative paths seemed not to work yesterday, but now it does indeed work. Patch now removed.

Thanks for taking a look!

Comment 3 Terje Røsten 2010-02-14 22:02:29 UTC
Release:       	0.2%{?dist}

Why still leading 0 in release tag, is 1.3.0 a beta release?

You have to escape the macro in changelog:

%install -> %%install

Comment 4 Carl van Tonder 2010-02-15 21:45:07 UTC
Spec URL: http://supervacuo.com/fedora/grc.spec
SRPM URL: http://supervacuo.com/fedora/grc-1.3-1.fc12.src.rpm

Updated to fix issues described.

Comment 5 Fabian Affolter 2010-07-02 20:51:39 UTC
Some quick comments:

- The source code contains a man page. The rpm is missing that man page.
- There is a mixed use of spaces and tabs.
- Please have a lot at https://fedoraproject.org/wiki/Packaging:Python
- Please get in touch with upstream about the license.

Comment 6 Fabian Affolter 2010-11-17 11:47:18 UTC
Any updates here?

Comment 7 Carl van Tonder 2011-01-14 15:31:09 UTC
I'm working on the issues described in comment 5; apologies for the delay. Fabian, is there any chance you could point out which parts of https://fedoraproject.org/wiki/Packaging:Python you feel I've missed?

Comment 8 Carl van Tonder 2011-11-19 20:08:35 UTC
Finally had some time to revisit this. New package:

Spec URL: http://supervacuo.com/fedora/grc.spec
SRPM URL: http://supervacuo.com/fedora/grc-1.4-1.fc16.src.rpm

(In reply to comment #5) 
> - The source code contains a man page. The rpm is missing that man page.
Man pages now included

> - There is a mixed use of spaces and tabs.
Fixed.

> - Please have a lot at https://fedoraproject.org/wiki/Packaging:Python
I've read it thoroughly, and I'm not sure it's relevant. Although grc is written in Python, it is not (yet?) implemented as a Python module: it does not need to know where python_sitelib is, or do any byte-compilation. My interpretation is therefore that no changes are needed: I'd appreciate your advice if this is not so.

> - Please get in touch with upstream about the license.
COPYING is now also correctly included. The license is GPLv2 with some additions: one of them is that it can be re-licensed under any licence fulfilling Debian's guidelines. Should I leave it as-is, or change it to GPL?

I've also updated to the latest upstream release, and am bypassing the included `install.sh` because it does not include all necessary files, anyway.

Comment 9 Fabian Affolter 2012-03-04 22:28:51 UTC
(In reply to comment #8)
> > - Please have a lot at https://fedoraproject.org/wiki/Packaging:Python
> I've read it thoroughly, and I'm not sure it's relevant. Although grc is
> written in Python, it is not (yet?) implemented as a Python module: it does not
> need to know where python_sitelib is, or do any byte-compilation. My
> interpretation is therefore that no changes are needed: I'd appreciate your
> advice if this is not so.

Sorry, it seams that I messed up two review requests.

Comment 10 Terje Røsten 2013-11-24 13:27:25 UTC
Hi Carl,

still interest in this package? 

There is 1.5 release available upstream.

Are you sponsored?

@Buland,
are you going to start the review any time soon?

Comment 11 Christopher Meng 2013-11-25 04:50:15 UTC
WARNING!

https://admin.fedoraproject.org/pkgdb/acls/name/grc

Comment 12 Carl van Tonder 2013-11-27 17:31:27 UTC
Terje, thanks for asking. I think I should back away from this package for the following reasons:

* other packages like ccze are available in the repos and I can't really see features that grc adds (I wanted it because I was coming from Ubuntu and didn't want to rewrite my bash aliases...)

* as Christopher Meng notes, there's a name clash with the GNU Radio GUI -- I don't see an easy way of resolving this

* I don't know how to go about byte-compiling the Python scripts in the spec.. if that's indeed required (I also don't know how to tell if it is!)

If anyone can advise on the name or packaging issue, though, I am happy to finish off the job just for the sake of completeness.


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