Bug 433228 - Review Request: distcc - Distributed C/C++ compilation
Review Request: distcc - Distributed C/C++ compilation
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ed Hill
Fedora Extras Quality Assurance
: Reopened
: 174883 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-17 15:48 EST by Denis Leroy
Modified: 2012-10-09 13:38 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-03-10 05:16:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ed: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Denis Leroy 2008-02-17 15:48:27 EST
Spec URL: http://www.poolshark.org/src/distcc.spec
SRPM URL: http://www.poolshark.org/src/distcc-2.18.3-1.fc8.src.rpm

Description:
distcc is a program to distribute compilation of C or C++ code across
several machines on a network. distcc should always generate the same
results as a local compile, is simple to install and use, and is often
two or more times faster than a local compile.
Comment 1 Ed Hill 2008-02-17 15:55:38 EST

*** This bug has been marked as a duplicate of 174883 ***
Comment 2 Denis Leroy 2008-02-17 16:04:32 EST
The bug you're pointing to is a stalled 2+ year old review. Please leave this
open and let review go through, then we'll let Fesco rule on the competing reviews.
Comment 3 Laurent Rineau 2008-02-17 16:15:42 EST
I have been involded in bug #174883, and I agree with Denis.
Comment 4 Ed Hill 2008-02-17 16:28:36 EST
Hi Denis and Laurent, please pardon my knee-jerk reaction of marking this as a
duplicate.  I've also been following bug #174883 and was hoping that it would 
make better progress.  But perhaps that's too much optimism...  :-)
Comment 5 Ed Hill 2008-02-17 17:16:07 EST
Here's a quick and not-yet-complete review:

 - ok : follows naming guidelines
 - ok : source matches upstream:

   88e4c15826bdbc5a3de0f7c1bcb429e558c6976d  distcc-2.18.3.tar.bz2
   88e4c15826bdbc5a3de0f7c1bcb429e558c6976d  distcc-2.18.3.tar.bz2.1

 - ok : license (GPLv2+) appears to be correct
 - ok : license appears in source and included in %doc
 - ok : spec file looks sane/legible
 - ok : built using "mock -r fedora-8-x86_64 distcc-2.18.3-1.fc8.src.rpm"
 - rpmlint reports:

   distcc.x86_64: W: file-not-utf8 /usr/share/doc/distcc-2.18.3/NEWS
   distcc-server.x86_64: W: incoherent-subsys /etc/init.d/distccd $prog
   distcc-server.x86_64: W: incoherent-subsys /etc/init.d/distccd $prog
   distcc-server.x86_64: W: incoherent-subsys /etc/init.d/distccd $prog
   distcc-server.x86_64: W: incoherent-init-script-name distccd

   where the file-not-utf8 warning can be ignored and the four other 
   warnings appear to be due to the package name being "distcc-server"
   (and not distcc or distccd) which means they are also harmless

 - ok : no need for ldconfig and no *.la files
 - ok : dir ownership looks good
 - ok : no need for -devel
 - ok : use of %clean and "rm -rf $RPM_BUILD_ROOT" at start of %install

Nits:
 - perhaps use Source0:
     http://distcc.samba.org/ftp/distcc/distcc-%{version}.tar.bz2
   which seems to be the prefered link from the web site

Next, I'll install it on a few machines and give it a try.
Comment 6 Denis Leroy 2008-02-17 19:24:00 EST
> perhaps use Source0:

Whoops. Fixed.

Rev 1 also came with a problematic server init script. Fixed.

Spec URL: http://www.poolshark.org/src/distcc.spec
SRPM URL: http://www.poolshark.org/src/distcc-2.18.3-2.fc8.src.rpm


A couple of open-ended question:
- The server RPM won't "start" as shipped. This is by design, in the sense that
distcc is a tool that really needs to be configured manually (and carefully).
One could argue that we should ship with '--allow 127.0.0.1' by default, but
that configuration is not useful in itself, so I prefer the first option, but
I'm open for discussion.

- In the same spirit of keeping the package simple and not doing too much
configuration, I did not add a logrotate and separate log file, but chose to let
the default syslog behavior. However it seems distccd has a tendency to spam
/var/log/messages quite a lot, so I'm having second thoughts about this.
Comment 7 Ville Skyttä 2008-02-18 12:50:52 EST
- LSB comment block missing, see
http://fedoraproject.org/wiki/PackagingDrafts/SysVInitScript

- Incorrect exit status (per LSB spec) when /usr/bin/distccd is not installed:
should be 5 when $1 != status.  What is this check needed for, BTW?

- The init script does not appear to make any checks whether we're in a required
configured state or not, which will probably lead to another incorrect exit
status per the LSB spec and I guess a suboptimal error message.  When
unconfigured, exit status should be 6 when $1 != status.  See for example how
the vdr and hddtemp packages handle this more gracefully.

- No profile.d snippet to have distcc in use out of the box?
Comment 8 Denis Leroy 2008-02-18 19:39:42 EST
Excellent. Ok so I rewrote the init file from scratch based on the packaging
draft, that should address your comments above, the default behavior after
installation now is to fail silently with code 6 (unconfigured).

> No profile.d snippet to have distcc in use out of the box?

I took a look at the profile.d setup you have for ccache, but I'm not thrilled
about a similar setup for distcc. Unlike ccache, there's no advantage at all to
having distcc run by default if it falls back to localhost everytime. The
profile.d script could make it conditional on the hosts file being non-empty
though (is that what you had in mind ?)...

Spec URL: http://www.poolshark.org/src/distcc.spec
SRPM URL: http://www.poolshark.org/src/distcc-2.18.3-3.fc8.src.rpm
Comment 9 Ville Skyttä 2008-02-19 02:42:14 EST
I didn't have anything specific about the profile.d script in mind, just wanted
to make sure not shipping it is intentional.  If determining whether the system
config is useful for running distcc is not expensive and can be done robustly, I
think it could be nice to have.  FWIW, icecream which is a similar package does
ship such a script, see it and some related discussion in bug 402881 if you're
interested.

But anyway, I don't think lack of the profile.d snippet is a blocker.

BTW, does a setup that has icecream and distcc installed (icecream automatically
enabled, distcc automatically or manually) need some special
considerations/configuration?  It doesn't seem that there's a convenient way to
opt out from using icecream if it's installed at the moment.
Comment 10 Laurent Rineau 2008-02-19 04:40:15 EST
Actually, I think that the behavior of ccache is not right. There is a file 
in /etc/profile.d/ that modifies the environment of all users of the machine. 
For a single-user machine, that is find: the user is the one that installed 
the ccache package. For a machine with a lot of users, like a lab main server 
machine in a university, that is not really a good idea to push that feature 
to all users (even if a good administrator can see that file and empty it, as 
it is a %config file). I would have preferred a "opt-in" option instead of 
a "opt-out". I agree with Denis: pushing distcc the same way would not be a 
good idea.
Comment 11 Ville Skyttä 2008-02-19 11:41:11 EST
I disagree wrt ccache; it works out of the box, is different, has been set up
automatically that way for several years and nobody has complained, and it
provides an easy way for per-user opt-out (see comments in
/etc/profile.d/ccache.*sh) and I strongly believe that auto-enabling it
w/opt-out is the best way to package it.

But anyway, this review is not about ccache, I realize that distcc is different,
and I don't use or plan to use distcc but am just providing a few comments
because I was asked to, so please feel free to proceed with the review the way
you see fit.

(And BTW, no need for personal Cc, I read the reviews mailing list.)
Comment 12 Denis Leroy 2008-02-19 18:46:29 EST
Ville, sorry about the CC.

I guess the whole opt-in/opt-out comes down to personal preference. I personally
like the way ccache is packaged. I actually discovered it was enabled on my
system by accident, when I couldn't understand why my rebuilds were so fast :-)
I like the fact that it's plug-and-play. Anyways, distcc is different and I
think I'll stick to the opt-in strategy for now, so let's close this particular
discussion :-)

I'll investigate the icecream/distcc combination shortly, thanks for pointing
that out.
Comment 13 Jason Tibbitts 2008-02-21 19:10:03 EST
Did you happen to see this bit, which Lennart had asked about in the other
review ticket (without response):
http://0pointer.de/blog/projects/avahi-distcc.html
Comment 14 Denis Leroy 2008-02-22 06:16:36 EST
icecream integration: so icecream offers a similar service to distcc, and
enables the service automatically by using a ccache-style file in profile.d. To
be honest, I cringe in horror when I look at /etc/profile.d/icecream.sh: it
manually check for the presence of ccache and changes its path order
accordingly. How can that possibly scale when more and more projects want to
provide a gcc alternative ? I guess that would be a would debate to have on
fedora-devel or on the packaging committee (assuming it didn't happen already).
So if you have both distcc and icecream installed, and if you use distcc the old
fashioned way with "make CC=distcc", distcc will be called first, but builds
scheduled to localhost will then invoke icecream, which will fall back to
localhost again or go remote if icecream is configured. So it will work of
course, but may confuse some people if you actually configured both servicess.

About Lennart's avahi patch: Lennart I had some issues with your patch: it
touches both "configure" and "configure.ac", and a Makefile.in change references
a file "distccmon-gnome.1" that's not part of the tarball and causing "make
install" to fail. I massaged the patch to make it build and gave the feature a
try on a virtual machine. Unfortunately I was unable to make it work. distccd
starts correctly with --zeroconf (although the verbose log mentions nothing
about avahi service being enabled?), but I'm seeing this ominous message on the
client system when I start distccd:

Feb 22 11:54:27 jupiler avahi-daemon[30737]: Invalid legacy unicast query packet.
Feb 22 11:54:28 jupiler avahi-daemon[30737]:last message repeated 2 times

(server and client systems run F-8).

Any thoughts ? 
Comment 15 Lennart Poettering 2008-02-22 07:25:22 EST
Yes, the patch is a bit dirty and based on Debian's distcc tarball. It's
perfectly fine to ignore the missing files. 

The legacy unicast issues are unrelated to the distcc patch. It's probably
borked multicast support in your virtual machine software. What VM system did
you use? Have you configured it for bridging support? Unless you enable bridging
for the VM mDNS (multicasting) will not work.
Comment 16 Denis Leroy 2008-02-22 09:50:16 EST
Lennart, the VM was probably be the problem. I used VMWare, in NAT mode.

I tried a simpler approach by running distccd locally, and that seems to work
better. I did however have one occurrence of this error:

> distcc -j
distcc[4016] (dcc_parse_tcp_host) ERROR: invalid tcp port specification in
":213:e8ff:fea3:b66f:1234/8
192.168.127.1:1234/8
"

> cat .distcc/zeroconf/hosts 
fe80::213:e8ff:fea3:b66f:1234/8
192.168.127.1:1234/8
Comment 17 Lennart Poettering 2008-02-26 11:49:16 EST
Uh, yeah, there some issues with IPv6. I guess I need to fix those. Should be
trivial to fix, though. IPv6 addresses need to be formatted a [address]:port 
instead of just address:port as for IPv4.
Comment 18 Denis Leroy 2008-03-04 03:02:29 EST
Okay, I suggest we file a separate bug to track the avahi patch ipv6 after
review. Here's the new version with the cleaned up avahi patch:

Spec URL: http://www.poolshark.org/src/distcc.spec
SRPM URL: http://www.poolshark.org/src/distcc-2.18.3-4.fc8.src.rpm


I'm still looking for a formal review :-)
Comment 19 Ed Hill 2008-03-05 01:53:02 EST
I built distcc-2.18.3-4.fc8.src.rpm in mock (F8 x86_64) and rpmlint 
reports:

  distcc.x86_64: W: file-not-utf8 /usr/share/doc/distcc-2.18.3/NEWS
  distcc-server.x86_64: W: incoherent-subsys /etc/init.d/distccd $prog
  distcc-server.x86_64: W: incoherent-init-script-name distccd

which all appear to be harmless.

I installed the RPMs on two dual-core F8 x86_64 machines and verified 
the fails-silently-w/error-code=6 behavior using the default config 
files (good).  I then edited the default /etc/sysconfig/distcc files, 
adding a ~/.distcc/hosts file, launched /usr/bin/distccmon-gnome, and 
ran "make -j4 CXX=distcc" for a ~50kloc C++ code base.  The compile 
proceeded nicely across the two machines.  Good!

I think the opt-in (off by default) setup is a good approach.

Unfortunately, I don't understand how to use the avahi (zeroconf) 
functionality.  Can some kind soul point me towards a README?  Or 
perhaps one could be included with the package?  Its just a 
suggestion...

I don't see any blockers here.  Since I don't fully understand the 
avahi patch or the interaction-with-icecream issue I'll give others 
24hrs to add comments.  If no one objects, I'll approve it.
Comment 20 Denis Leroy 2008-03-05 16:22:58 EST
Ed, good point about the documentation. Let's open a bugzilla against distcc
post-review on this: we'll integrate the information Lennart posted at
http://0pointer.de/blog/projects/avahi-distcc.html into the man page.
Comment 21 Ed Hill 2008-03-06 10:58:27 EST
I don't see any blockers here and it works-for-me so its APPROVED.
Comment 22 Denis Leroy 2008-03-07 13:45:04 EST
New Package CVS Request
=======================
Package Name: distcc
Short Description: Distributed C/C++ compilation
Owners: denis
Branches: F-7 F-8
InitialCC:
Cvsextras Commits: yes
Comment 23 Kevin Fenzi 2008-03-07 16:44:05 EST
cvs done.
Comment 24 Denis Leroy 2008-03-07 17:25:35 EST
*** Bug 174883 has been marked as a duplicate of this bug. ***
Comment 25 Terje Rosten 2008-04-24 07:51:25 EDT
Ok, I see the cvs files, koji builds, however I can't find it bodhi.

What's up?



Comment 26 Denis Leroy 2008-04-24 11:04:55 EDT
I could've sworn I ran bodhi on it for F-8, but then again maybe I forgot. Done
now :-)
Comment 27 Andy Grover 2011-09-14 19:07:26 EDT
Package Change Request
======================
Package Name: distcc
New Branches: el5 el6
Owners: limb

Hi, I'd like to see this added to EPEL please?
Comment 28 Andy Grover 2012-10-05 19:28:57 EDT
Package Change Request
======================
Package Name: distcc
New Branches: el5 el6
Owners: limb grover
Comment 29 Jason Tibbitts 2012-10-08 12:38:05 EDT
Need an ack from the current owner here, especially if you're going to make him a maintainer.
Comment 30 Denis Leroy 2012-10-09 04:11:34 EDT
Andy,  feel free to take over this package for all branches. I retired from Fedora packaging a while ago due to lack of time, unfortunatley.
Comment 31 Gwyn Ciesla 2012-10-09 06:47:59 EDT
Git done (by process-git-requests).

I'm ok with it, do you want to build or shall I?
Comment 32 Andy Grover 2012-10-09 12:06:55 EDT
Hey Jon, I'm happy to do it.
Comment 33 Andy Grover 2012-10-09 13:38:17 EDT
built .el6 and pushed to updates-testing. el5 ftbfs even after fixing popt-devel -> popt buildrequires, I'm going to give up on it if that's ok.

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