Bug 700833 - Review Request: colorgcc - Script to colorize the terminal output of gcc, g++, cc, c++
Summary: Review Request: colorgcc - Script to colorize the terminal output of gcc, g++...
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-29 14:41 UTC by Martin Cermak
Modified: 2011-06-08 23:56 UTC (History)
4 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2011-05-25 03:07:24 UTC
hdegoede: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Martin Cermak 2011-04-29 14:41:35 UTC
Spec URL: http://www.physics.muni.cz/~cermak/colorgcc/colorgcc.spec
SRPM URL: http://www.physics.muni.cz/~cermak/colorgcc/colorgcc-1.3.2-1.fc14.src.rpm
Patch0 URL: http://www.physics.muni.cz/~cermak/colorgcc/colorgcc-1.3.2-invocation.patch

Description: 

Hi! I had a look at the Package maintainers wishlist [1] and decided to get the
colorgcc script packaged for Fedora. I would appreciate a review.

+ rpmlint colorgcc.spec
colorgcc.spec: W: no-%build-section
0 packages and 1 specfiles checked; 0 errors, 1 warnings.
+ rpmlint colorgcc-1.3.2-1.fc14.src.rpm
colorgcc.src: W: no-%build-section
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
+ rpmlint colorgcc-1.3.2-1.fc14.noarch.rpm
colorgcc.noarch: W: no-documentation
colorgcc.noarch: W: no-manual-page-for-binary color-g++
colorgcc.noarch: W: no-manual-page-for-binary color-cc
colorgcc.noarch: W: no-manual-page-for-binary color-gcc
colorgcc.noarch: W: no-manual-page-for-binary color-c++
1 packages and 0 specfiles checked; 0 errors, 5 warnings.


-------------------------------
[1] http://fedoraproject.org/wiki/PackageMaintainers/WishList

Comment 1 Thomas Spura 2011-04-29 15:16:30 UTC
Hmm, is there an easy way to call ccache instead of gcc within colorgcc?
Currently I have this for calling colorgcc:
$ ll /usr/local/bin/
insgesamt 0
lrwxrwxrwx. 1 root root 15 15. Mai 2010  cc -> /usr/bin/ccache
lrwxrwxrwx. 1 root root 15 15. Mai 2010  g++ -> /usr/bin/ccache
lrwxrwxrwx. 1 root root 15 15. Mai 2010  gcc -> /usr/bin/ccache

Replacing that link to color-gcc will unfortunately disable ccache...

Comment 2 Hans de Goede 2011-04-30 08:34:57 UTC
Reviewing this one ...

Comment 3 Hans de Goede 2011-04-30 08:43:26 UTC
Full review done:

Good:
=====
- rpmlint checks return:
colorgcc.src: W: no-%build-section
colorgcc.noarch: W: no-documentation
colorgcc.noarch: W: no-manual-page-for-binary color-g++
colorgcc.noarch: W: no-manual-page-for-binary color-cc
colorgcc.noarch: W: no-manual-page-for-binary color-gcc
colorgcc.noarch: W: no-manual-page-for-binary color-c++
 These can all be ignored
- package meets naming guidelines
- package meets packaging guidelines
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

Needs work:
-----------
- Please use the full URL for Source0

- The license tag should be GPL+, per:
  http://fedoraproject.org/wiki/Licensing
  See the GPL (no version) table

- If you're not adding a %clean, nor cleaning the buildroot in %install, you should not specify a buildroot at all, so drop:
BuildRoot:  %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

- A bitter way to handle %prep would be:
%prep
%setup -q -c -T
cp -p %{SOURCE0} .
%patch0 -p1
  You can then also drop the "cd %{name}-%{version}" from %install

Comment 4 Jason Tibbitts 2011-04-30 18:15:48 UTC
I see no SCM request to process.

Comment 5 Martin Cermak 2011-05-06 06:56:56 UTC
> Needs work:
> -----------
> - Please use the full URL for Source0
>
> - The license tag should be GPL+, per:
>   http://fedoraproject.org/wiki/Licensing
>   See the GPL (no version) table
>
> - If you're not adding a %clean, nor cleaning the buildroot in %install, you
> should not specify a buildroot at all, so drop:
> BuildRoot:  %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>
> - A bitter way to handle %prep would be:
> %prep
> %setup -q -c -T
> cp -p %{SOURCE0} .
> %patch0 -p1
>   You can then also drop the "cd %{name}-%{version}" from %install

This should be resolved in colorgcc-1.3.2-2:

Spec URL: http://www.physics.muni.cz/~cermak/colorgcc-1.3.2-2/colorgcc.spec
SRPM URL: http://www.physics.muni.cz/~cermak/colorgcc-1.3.2-2/colorgcc-1.3.2-2.fc14.src.rpm
Patch0 URL: http://www.physics.muni.cz/~cermak/colorgcc-1.3.2-2/colorgcc-1.3.2-invocation.patch

$ rpmlint colorgcc.spec
colorgcc.spec: W: no-%build-section
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

$ rpmlint colorgcc-1.3.2-2.fc14.src.rpm
colorgcc.src: W: no-%build-section
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint colorgcc-1.3.2-2.fc14.noarch.rpm
colorgcc.noarch: W: no-documentation
colorgcc.noarch: W: no-manual-page-for-binary color-gcc
colorgcc.noarch: W: no-manual-page-for-binary color-g++
colorgcc.noarch: W: no-manual-page-for-binary color-cc
colorgcc.noarch: W: no-manual-page-for-binary color-ccache
colorgcc.noarch: W: no-manual-page-for-binary color-c++
1 packages and 0 specfiles checked; 0 errors, 6 warnings.


> Hmm, is there an easy way to call ccache instead of gcc within colorgcc?
> Currently I have this for calling colorgcc:
> $ ll /usr/local/bin/
> insgesamt 0
> lrwxrwxrwx. 1 root root 15 15. Mai 2010  cc -> /usr/bin/ccache
> lrwxrwxrwx. 1 root root 15 15. Mai 2010  g++ -> /usr/bin/ccache
> lrwxrwxrwx. 1 root root 15 15. Mai 2010  gcc -> /usr/bin/ccache
>
> Replacing that link to color-gcc will unfortunately disable ccache...

On a fresh F14 system, ccache is called transparently if you call gcc:

$ strace -eexecve -f gcc hello.c 2>&1 | grep ^execve | grep -v ENOENT
execve("/usr/lib64/ccache/gcc", ["gcc", "hello.c"], [/* 33 vars */]) = 0
execve("/usr/bin/gcc", ["/usr/bin/gcc", "hello.c"], [/* 34 vars */]) = 0

So I just removed absolute paths pointing to compiler binaries from colorgcc.
Now it should work as expected if you have ccache installed:

$ strace -eexecve -f color-gcc hello.c 2>&1 | grep ^execve | grep -v ENOENT
execve("/usr/bin/color-gcc", ["color-gcc", "hello.c"], [/* 33 vars */]) = 0
execve("/usr/lib64/ccache/gcc", ["gcc", "hello.c"], [/* 33 vars */]) = 0
execve("/usr/bin/gcc", ["/usr/bin/gcc", "hello.c"], [/* 34 vars */]) = 0

And also if you haven't:

trace -eexecve -f color-gcc hello.c 2>&1 | grep ^execve | grep -v ENOENT
execve("/usr/bin/color-gcc", ["color-gcc", "hello.c"], [/* 33 vars */]) = 0
execve("/usr/bin/gcc", ["gcc", "hello.c"], [/* 33 vars */]) = 0

And I also provided the color-ccache link:

$ rpm -ql colorgcc
/usr/bin/color-c++
/usr/bin/color-cc
/usr/bin/color-ccache
/usr/bin/color-g++
/usr/bin/color-gcc

Hope I didn't miss something. What remains is to set up some nice aliases... :)

Comment 6 Hans de Goede 2011-05-06 13:29:35 UTC
Looks good now: approved.

Comment 7 Martin Cermak 2011-05-07 10:44:57 UTC
New Package SCM Request
=======================
Package Name: colorgcc
Short Description: Script to colorize the compiler output
Owners: mcermak
Branches: f14 f15
InitialCC: jwrdegoede

Comment 8 Jason Tibbitts 2011-05-10 15:53:16 UTC
Git done (by process-git-requests).

Comment 9 Fedora Update System 2011-05-13 09:27:10 UTC
colorgcc-1.3.2-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/colorgcc-1.3.2-2.fc15

Comment 10 Fedora Update System 2011-05-13 09:32:25 UTC
colorgcc-1.3.2-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/colorgcc-1.3.2-2.fc14

Comment 11 Fedora Update System 2011-05-13 23:11:14 UTC
colorgcc-1.3.2-2.fc14 has been pushed to the Fedora 14 testing repository.

Comment 12 Fedora Update System 2011-05-25 03:07:19 UTC
colorgcc-1.3.2-2.fc14 has been pushed to the Fedora 14 stable repository.

Comment 13 Fedora Update System 2011-06-08 23:56:00 UTC
colorgcc-1.3.2-2.fc15 has been pushed to the Fedora 15 stable repository.


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