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.
*** This bug has been marked as a duplicate of 174883 ***
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.
I have been involded in bug #174883, and I agree with Denis.
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... :-)
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.
> 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.
- 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?
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
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.
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.
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.)
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.
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
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 ?
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.
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
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.
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 :-)
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.
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.
I don't see any blockers here and it works-for-me so its APPROVED.
New Package CVS Request ======================= Package Name: distcc Short Description: Distributed C/C++ compilation Owners: denis Branches: F-7 F-8 InitialCC: Cvsextras Commits: yes
cvs done.
*** Bug 174883 has been marked as a duplicate of this bug. ***
Ok, I see the cvs files, koji builds, however I can't find it bodhi. What's up?
I could've sworn I ran bodhi on it for F-8, but then again maybe I forgot. Done now :-)
Package Change Request ====================== Package Name: distcc New Branches: el5 el6 Owners: limb Hi, I'd like to see this added to EPEL please?
Package Change Request ====================== Package Name: distcc New Branches: el5 el6 Owners: limb grover
Need an ack from the current owner here, especially if you're going to make him a maintainer.
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.
Git done (by process-git-requests). I'm ok with it, do you want to build or shall I?
Hey Jon, I'm happy to do it.
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.