Re: distcc for ada (part 1 of 2)
Fergus Henderson <[email protected]>
| Newsgroups | gmane.comp.compilers.distcc |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Oct 14, 2009 at 4:25 PM, Tim Dossett <[email protected]>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 <[email protected]> __ distcc mailing list http://distcc.samba.org/ To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/distcc