991035 - NSPR is calling ccache recursively

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Cervantes Yu [:cyu] [:cervantes]]

We recently observed that building b2g gecko with distcc fails permanently. It turns out that we prepend ccache commands to the gcc command twice in building NSPR. This is OK for ccache but distcc (masqueraded as ccache) doesn't allow recursive call to itself and gecko fails to build.

To observe the problem:
* build gecko with ccache.
* delete some .o under objdir-gecko/nsprpub
* under objdir-gecko/nsprpub, run make -S

Here is my output in building prfdcach.o:
/usr/bin/ccache /usr/bin/ccache /home/distcc/prebuilt/linux-x86/toolchain/arm-linux-androideabi-4.4.x/bin/arm-linux-androideabi-gcc -o prfdcach.o -c 
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                 ccache is prepended twice!!

-fvisibility=hidden   -DANDROID -isystem /home/cervantes/git/unagi/B2G/bionic/libc/arch-arm/include -isystem /home/cervantes/git/unagi/B2G/bionic/libc/include/ -isystem 
/home/cervantes/git/unagi/B2G/bionic/libc/kernel/common -isystem /home/cervantes/git/unagi/B2G/bionic/libc/kernel/arch-arm -isystem /home/cervantes/git/unagi/B2G/bionic/libm/include -I/home/cervantes/git/unagi/B2G/system -I/home/cervantes/git/unagi/B2G/system/core/include -isystem /home/cervantes/git/unagi/B2G/bionic -I/home/cervantes/git/unagi/B2G/hardware/libhardware/include -I/home/cervantes/git/unagi/B2G/external/valgrind/fxos-include -I/home/cervantes/git/unagi/B2G/frameworks/base/opengl/include -I/home/cervantes/git/unagi/B2G/frameworks/base/native/include -I/home/cervantes/git/unagi/B2G/frameworks/base/include -I/home/cervantes/git/unagi/B2G/frameworks/base/services/camera -I/home/cervantes/git/unagi/B2G/frameworks/base/include/media/stagefright -I/home/cervantes/git/unagi/B2G/frameworks/base/include/media/stagefright/openmax -I/home/cervantes/git/unagi/B2G/frameworks/base/media/libstagefright/rtsp -I/home/cervantes/git/unagi/B2G/frameworks/base/media/libstagefright/include -I/home/cervantes/git/unagi/B2G/external/dbus -I/home/cervantes/git/unagi/B2G/external/bluetooth/bluez/lib -I/home/cervantes/git/unagi/B2G/dalvik/libnativehelper/include/nativehelper  -g -Wall -Wpointer-arith -Wdeclaration-after-statement -Werror=return-type -Werror=int-to-pointer-cast -Wtype-limits -Wempty-body -Wsign-compare -Wno-unused -mandroid -fno-short-enums -fno-exceptions -march=armv7-a -mthumb -mfpu=neon -mfloat-abi=softfp -std=gnu99 -fgnu89-inline -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pipe -g -std=c99 -fomit-frame-pointer -march=armv7-a -mthumb -mfpu=neon -mfloat-abi=softfp -Wall -O2 -g -fno-inline -fPIC  -UDEBUG -DPACKAGE_NAME=\"\" -DPACKAGE_TARNAME=\"\" -DPACKAGE_VERSION=\"\" -DPACKAGE_STRING=\"\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DANDROID=1 -DMOZILLA_CLIENT=1 -D_PR_ARM_KUSER=1 -DNDEBUG=1 -DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1 -DHAVE_VISIBILITY_PRAGMA=1 -DXP_UNIX=1 -D_GNU_SOURCE=1 -DHAVE_FCNTL_FILE_LOCKING=1 -DLINUX=1 -DHAVE_DLADDR=1 -DHAVE_GETTID=1 -DHAVE_LCHOWN=1 -DHAVE_SETPRIORITY=1 -DHAVE_STRERROR=1 -DHAVE_SYSCALL=1 -D_REENTRANT=1 -DFORCE_PR_LOG -D_PR_PTHREADS -UHAVE_CVAR_BUILT_ON_SEM -D_NSPR_BUILD_ -I/home/cervantes/git/unagi/B2G/objdir-gecko/dist/include/nspr -I/home/cervantes/hg/mozilla-central/nsprpub/pr/include -I/home/cervantes/hg/mozilla-central/nsprpub/pr/include/private  /home/cervantes/hg/mozilla-central/nsprpub/pr/src/io/prfdcach.c

Comment 1

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Don't prepend ccache command twice in building NSPR

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

Comment on attachment 8408020 [details] [diff] [review]
Don't prepend ccache command twice in building NSPR

Loop Ted in for feedback.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8408020 [details] [diff] [review]
Don't prepend ccache command twice in building NSPR

You're just modifying NSPR's generated configure file here, which isn't going to be useful. I'd guess the root problem here is that Gecko's configure prepends ccache to the compile line, and then NSPR winds up double-appending. I think the fix you actually want is to ensure that Gecko's configure either doesn't set CC to include ccache, or if it does that then it should omit --with-ccache from NSPR's options.

Comment 4

[(not currently active) Ted Mielczarek]

glandium might have ideas here.

Comment 5

[Mike Hommey [:glandium]]

I vote for omitting --with-ccache.

Comment 6

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Don't prepend ccache command twice in building NSPR (v2)

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 8482200 [details] [diff] [review]
Don't prepend ccache command twice in building NSPR (v2)

Review of attachment 8482200 [details] [diff] [review]:
-----------------------------------------------------------------

This doesn't look like the patch glandium suggested. I think you want to do almost exactly what's in the block above, and remove --enable-ccache from _SUBDIR_CONFIG_ARGS.

Comment 8

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Don't prepend ccache command twice in building NSPR (v3)

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 8486252 [details] [diff] [review]
Don't prepend ccache command twice in building NSPR (v3)

Review of attachment 8486252 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +9103,5 @@
>      # Avoid doubling wrap malloc arguments
>      _SUBDIR_CONFIG_ARGS="`echo $_SUBDIR_CONFIG_ARGS | sed -e 's/--enable-wrap-malloc *//'`"
>  fi
>  
> +if (echo $CC | grep -q "^\ *$CCACHE\ *") then

I think you could just write:
if test -n "$MOZ_USING_CCACHE"; then

Comment 10

[Cervantes Yu [:cyu] [:cervantes]]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=fbd5574153a2

Comment 11

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #9)
> Comment on attachment 8486252 [details] [diff] [review]
> Don't prepend ccache command twice in building NSPR (v3)
> 
> Review of attachment 8486252 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: configure.in
> @@ +9103,5 @@
> >      # Avoid doubling wrap malloc arguments
> >      _SUBDIR_CONFIG_ARGS="`echo $_SUBDIR_CONFIG_ARGS | sed -e 's/--enable-wrap-malloc *//'`"
> >  fi
> >  
> > +if (echo $CC | grep -q "^\ *$CCACHE\ *") then
> 
> I think you could just write:
> if test -n "$MOZ_USING_CCACHE"; then

Thanks for your suggestion and review :).

Comment 12

[Cervantes Yu [:cyu] [:cervantes]]

Push to inbound with the suggested change in comment #9.

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c377a5855f4

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5c377a5855f4