[distcc] distcc for ada (part 1 of 2)

[Fergus Henderson]

On Wed, Oct 14, 2009 at 4:25 PM, Tim Dossett <timothy.dossett at gmail.com>wrote:

> Hello all,
> I have modified distcc 3.1 to distribute ada compilation as well as c
> and c++, and I'd like to contribute the modifications back to the
> project. Patch is attached. Please consider this for inclusion in the
> distcc baseline.
>

Cool, that looks great - thanks for the patch!
It is especially nice to see how the development of pump mode has helped you
to add support for Ada.

Here's some comments from a fairly quick initial scan:

- If possible, could you please base the patch on the current sources in
SVN, rather than on the last release?
- The changes to configure.ac and configure should be reverted.
- For the NEWS file, could you condense the new entries into a single one?
- Are you sure that we handle "-x c", "-x c++", and "-x objective-c"
correctly?  It might be better to fall back to local execution for those.
- What happens if you mix file extensions with "-x" options, e.g. "distcc -c
-x ada foo.c" or "distcc -c -x c foo.ada"?
  I suspect that probably doesn't work...
- The change to max_discrepancies_before_demotion is not the right thing to
do for C/C++.
- Don't hard-code the port number to use during tests (especially not to
3633); try a port, and if that doesn't work, keep trying higher port numbers
- Some documentation of the new protocol in doc/protocol-4.txt would be
nice.
- Do we need a new host option (similar to ",lzo" or ",lzo,cpp") for version
4 of the protocol?
- It would be nice if the Ada tests could be written in such a way that if
the required compilers aren't installed they pass, but print a warning
message (similar to the NOTRUN messages printed by the current tests), and
perhaps integrated into "make check".  Or alternatively, add some simple
tests of Ada functionality to test/testdistcc.py.
- It would be nice to have at least one test of a _failing_ Ada compilation.
- Some unit tests for include_server/Ada.py would be nice.

If you could update the patch after considering these comments, that would
be great.

Cheers,
  Fergus.
-- 
Fergus Henderson <fergus at google.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.samba.org/pipermail/distcc/attachments/20091014/6fc5c7fe/attachment.html>