pump does not parse response files

[GoogleCodeExporter]

pump cannot process compiler invocations with response files as they are used 
to shorten the command line under windows usually passed with 
"@more_arguments.rsp"

Answering the following questions is a big help:

1. What version of distcc are you using (e.g. "2.7.1")?  You can run "distcc 
--version" to see.  If you got distcc from a distribution package rather than 
building from source, please say which one.
distcc 3.1

2. What platform are you running on (e.g. "Red Hat 8.0", "HP-UX 11.11")?  What 
compilare are you using ("gcc 3.3")?  Run "uname -a" and "cc --version" to see.
Linux gold 2.6.18-194.32.1.el5 #1 SMP Wed Jan 5 17:52:25 EST 2011 x86_64 x86_64 
x86_64 GNU/Linux

x86_64-w64-mingw32-g++ (GCC) 4.5.1
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

3. What were you trying to do (e.g. "install distcc", "build Mozilla")?

We use CMake to generate Makefiles - we use a toolchain for mingw32 targets
CMake as of version 2.8.5 uses response files to make the commandlines shorter 
since mysys shells on windows can sometimes not support long commandlines.

4. What went wrong?  Did you get an error message, did it hang, did it build a 
program that didn't work, did it not distribute compilation to machines that 
ought to get it?

when running 'pump make', the following warning is emitted:

WARNING include server: Preprocessing locally. Include server not covering: 
Could not locate name of translation unit: 
['@CMakeFiles/bclexampleutil.dir/includes_CXX.rsp', 
'example/util/example_util_implementation.cpp']. for translation unit 'unknown 
translation unit'

and the file is preprocessed locally and compiled remotely without problems.

5. If you have an example of a compiler invocation that failed, quote it, in 
full e.g.:

distcc x86_64-w64-mingw32-g++ @CMakeFiles/bclexampleutil.dir/includes_CXX.rsp   
-m32 -Wno-deprecated -fno-pretty-templates -o 
CMakeFiles/bclexampleutil.dir/example_util_implementation.cpp.obj -c 
example/util/example_util_implementation.cpp

6. What error logging do you get?  Turn on client and server error logging.  On 
the client, set these environment variables, and try to reproduce the problem: 
=export DISTCC_VERBOSE=1 DISTCC_LOG=/tmp/distcc.log=.  Start the server with 
the --verbose option. If the problem is intermittent, leave logging enabled and 
then pull out the lines from the log file when the problem recurs.

problem might be in pump component "parse_command.py"

7. If you got an error message on stderr, quote that error exactly. Find the 
lines in the log files pertaining to the compile, and include all of them in 
your report, by looking at the process ID in square brackets. If you can't work 
that out, quote the last few hundred lines leading up to the failure.

problem might be in pump component "parse_command.py"

I think I can track down the problem to parse_command.py:343
It might be necessary to actually expand the content of the file given with 
'@...rsp' since it contains additional commandline arguments (like "-Ipath1 
...") that the compiler would need to parse - and pump as well.

Before I suggest a patch, I just want to make sure there is no other solution.

Thanks 

Original issue reported on code.google.com by nilswoet...@gmail.com on 18 Sep 2011 at 11:13

[GoogleCodeExporter]

I suggest you go ahead and make a patch -- I don't know of any other solution.

Original comment by fer...@google.com on 19 Sep 2011 at 12:12

[GoogleCodeExporter]

Attached is the patch for the parse_command.py for the pump server.

The problem that remains is that I now get this error:
distcc[20879] ERROR: compile example_util.cpp on gold:3363/4,lzo,cpp failed
distcc[20879] (dcc_build_somewhere) Warning: remote compilation of 
'example_util.cpp' failed, retrying locally
distcc[20879] Warning: failed to distribute example_util.cpp to 
gold:3363/4,lzo,cpp, running locally instead
distcc[20879] (dcc_please_send_email_after_investigation) Warning: remote 
compilation of 'example_util.cpp' failed, retried locally and got a different 
result.
distcc[20879] (dcc_note_discrepancy) Warning: now using plain distcc, possibly 
due to inconsistent file system changes during build

I assume that the compile.c/remote.c needs to be changed to expand the response 
files as well in either of the functions:

dcc_compile_remote
dcc_compile_local
dcc_build_somewhere_timed
dcc_build_somewhere

which all take *argv[] - which should be expanded as well.

Can you advice what the best place for the expansion would be (I might have 
even overlooked functions that depend on the arguments? Or would the function, 
that adds the compiler name to the list of arguments be the best place?)

Thanks

Original comment by nilswoet...@gmail.com on 20 Sep 2011 at 10:19

Attachments:

• parse_command.patch

[GoogleCodeExporter]

I would be inclined to start at main() and traverse down through the call graph 
until you get to a function that passes argc/argv to more than one subroutine, 
or that manipulates argc/argv to parse the command line.  That function (which 
could be main() itself) is likely to be the best place to expand response files.

Original comment by fergus.h...@gmail.com on 7 Feb 2012 at 1:49

[GoogleCodeExporter]

The attached patch adds a function that expands response files recursively (up 
to depth 10); it combines the previous patch for the parse_command.py for the 
include_server component and adds the fixes that were necessary in the C 
sources (creating the "compile_flags" argument in distcc.c main() function.
I tried to stick to the coding conventions.
Hope it finds approval and will be part of a new release.

Original comment by nilswoet...@gmail.com on 29 Feb 2012 at 7:37

Attachments:

• distcc_response_file.patch

[GoogleCodeExporter]

The patch has potential buffer overflows, e.g. using fscanf( fp, "%s", arg) 
where arg is declared as a fixed-length buffer 'char arg[999];'.

Also the patch is full of arbitrary fixed limits (999, 10), and is inconsistent 
between the C code and the Python code about whether there is a depth limit to 
the recursion.

Also in src/distcc.c, the code added should be a single function call to a 
second function defined in argutil.c.

So the patch is not acceptable in its current state.

Original comment by fer...@google.com on 16 May 2012 at 6:47

[GoogleCodeExporter]

Some other notes on the patch.

Expanding @FILE before dcc_build_somewhere means local compiles will be less 
conservative than previously.  It is however preferable to perform expansion 
before invoking the preprocessor.

Inside libiberty there is exactly the function used by gcc: expandargv.  It 
does not suffer the issues mentioned in comment #5.  Libiberty also includes 
other argv helpers similar to the ones used in distcc (duplicate, count, etc.). 
 Typically it is shipped as a static library (i.e. in Debian binutils-dev).

If there are no objections to introducing this library as a build-time 
dependency, then I will rework the patch.  Actually, as the library is GPLv2+, 
I can include a fallback copy and handle it similar to popt.

Anyway, should be done over the next two days.

Original comment by mand...@gmail.com on 17 Feb 2013 at 2:58

[GoogleCodeExporter]

Note that the previous include_server patch does not implement the same 
algorithm as gcc.

Attached is trivial patch to use 'expandargv' in distcc main.  This occurs 
before any processing is done — including passing command line to server — 
so no patching of include_server is required.  Works for plain mode, and 
confirmed for pump mode by this session:

$ cat args
-I/home/daniel/src/distcc/tmp/argstest
$ pump distcc gcc @args -c -o test.o test.c
…
TRACE: ParseCommand ['gcc', '-I/home/daniel/src/distcc/tmp/argstest', '-c', 
'-o', 'test.o', 'test.c']
…

Adds a (typically) build time dependency on libiberty, and distcc binary is 
increased by a few kB.  There are other functions in libiberty that can 
trivially replace much of e.g. distcc/src/argutil.c, but I leave that for a 
subsequent patch.

Libiberty is distributed under GPLv2+.

This patch also fixes issue 122.

Original comment by mand...@gmail.com on 15 Apr 2013 at 1:20

Attachments:

• expandargv.patch

[GoogleCodeExporter]

Issue 122 has been merged into this issue.

Original comment by fergus.h...@gmail.com on 15 Apr 2013 at 9:42

[GoogleCodeExporter]

Thanks for the patch!

Looks pretty good, but can you please also update the INSTALL file to document 
the new dependency?

Also, I am wondering if it would be better to code this so that if libiberty 
isn't found, then we issue a configure-time warning and then fall back to the 
previous behaviour of not expanding response files.  What do you think?

Original comment by fergus.h...@gmail.com on 15 Apr 2013 at 10:26

• Changed state: Started

[GoogleCodeExporter]

> Also, I am wondering if it would be better to code this so
> that if libiberty isn't found, then we issue a configure-
> time warning and then fall back to the previous behaviour
> of not expanding response files.  What do you think?

Some platforms (e.g. in OP) are severly affected.  On others failure will be 
less common, but apparently random and perhaps difficult to trace back to a 
configure time warning.

I would not make this optional, but I suppose the concern is the atypical 
distribution of libiberty.  Debian packages contain them (binutils-dev), other 
systems perhaps not.  Aside from the bundled distribution, building and 
installing libiberty is a fairly standard procedure that anyone hoping to build 
distcc should be capable of.

Including a verbatim copy is suggested by the docs, about 1.8 MB of sources 
with decent compression potential.

Original comment by mand...@gmail.com on 17 Apr 2013 at 5:11

[GoogleCodeExporter]

Minimal INSTALL patch for libiberty is required.

Original comment by mand...@gmail.com on 6 May 2013 at 6:15

Attachments:

• libiberty-req.patch

[GoogleCodeExporter]

The combination of these two patches (expandargv.patch and libiberty-req.patch) 
looks good to me.  Do you want to go ahead and commit them?

I have added you to the list of users who have permission to commit changes.
See <https://code.google.com/p/distcc/source/checkout> for details on how to 
check out the SVN repository.

Original comment by fergus.h...@gmail.com on 7 May 2013 at 11:37

[GoogleCodeExporter]

This issue was closed by revision r774.

Original comment by mand...@gmail.com on 8 May 2013 at 2:22

• Changed state: Fixed