NSPR is calling ccache recursively

RESOLVED FIXED in mozilla35

Status

defect
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: cyu, Assigned: cyu)

Tracking

Trunk
mozilla35
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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 on attachment 8408020 [details] [diff] [review]
Don't prepend ccache command twice in building NSPR

Loop Ted in for feedback.
Attachment #8408020 - Flags: feedback?(ted)
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.
Attachment #8408020 - Flags: feedback?(ted) → feedback-
glandium might have ideas here.
I vote for omitting --with-ccache.
Attachment #8408020 - Attachment is obsolete: true
Attachment #8408020 - Flags: feedback?(wtc)
Attachment #8482200 - Flags: review?(ted)
Product: Firefox → Core
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.
Attachment #8482200 - Flags: review?(ted) → review-
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
Attachment #8486252 - Flags: review?(ted) → review+
Assignee: nobody → cyu
(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 :).
https://hg.mozilla.org/mozilla-central/rev/5c377a5855f4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Depends on: 1066669
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.