Closed Bug 580175 Opened 9 years ago Closed 9 years ago

make ccache option more flexible

Categories

(Firefox Build System :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla2.0b3

People

(Reporter: t.matsuu, Assigned: t.matsuu)

References

Details

Attachments

(2 files, 5 obsolete files)

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b2pre) Gecko/20100720 Fedora/4.0b2pre-0.b2pre.2010072011.fc13 Minefield/4.0b2pre Firefox/3.6.6
Build Identifier: 

Add to detect ccache automatically if --with-ccache is defined.

Reproducible: Always
Attached patch To solve this (obsolete) — Splinter Review
Attachment #458565 - Flags: review?(me)
Blocks: 579704
Assignee: nobody → t.matsuu
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 458565 [details] [diff] [review]
To solve this

This will make --with-ccache work, but it will break --with-ccache=path/to/ccache.  Fix that and repost please.
Attachment #458565 - Flags: review?(me) → review-
Attached patch To solve this v2. (obsolete) — Splinter Review
Sorry for my mistake.

This should be work.
Attachment #458565 - Attachment is obsolete: true
Attachment #458585 - Flags: review?(me)
Attached patch To solve this v3. (obsolete) — Splinter Review
Fix to work without "--with-ccache".
Attachment #458585 - Attachment is obsolete: true
Attachment #458626 - Flags: review?(me)
Attachment #458585 - Flags: review?(me)
Much better, just two nits with the current version.

1) If I specify --with-ccache but ccache is not in my path, this will not give an error.  To fix this you probably need to use a temporary to store the value of ccache from MOZ_ARG_WITH_STRING and then set CCACHE appropriately.

2) Please use MOZ_PATH_PROG instead of AC_PATH_PROG.  MOZ_PATH_PROG is our own version of AC_PATH_PROG that has some extra goodness to work in our windows build environment.
Attached patch To solve this v4. (obsolete) — Splinter Review
Test result:

1.system doesn't have ccache
without "--with-ccache"
  (check skipped)

--with-ccache
  checking for ccache... :
  configure: error: ccache not found in $PATH

--with-ccache=
  configure: error: --with-ccache is set with a blank value

--with-ccache=/not_exist/bin/ccache
  checking for ccache... /not_exist/bin/ccache
  configure: error: /not_exist/bin/ccache not found


2. system have ccache in /usr/bin/ccache
without "--with-ccache"
  (check skipped)

--with-ccache
  checking for ccache... /usr/bin/ccache

--with-ccache=
  configure: error: --with-ccache is set with a blank value

--with-ccache=/not_exist/bin/ccache
  checking for ccache... /not_exist/bin/ccache
  configure: error: /not_exist/bin/ccache not found

--with-ccache=/exist/bin/ccache
  checking for ccache... /exist/bin/ccache
Attachment #458626 - Attachment is obsolete: true
Attachment #458907 - Flags: review?(me)
Attachment #458626 - Flags: review?(me)
Comment on attachment 458907 [details] [diff] [review]
To solve this v4.

>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>@@ -7402,13 +7402,23 @@ dnl ====================================
> dnl = Enable compiling with ccache
> dnl ======================================================
> MOZ_ARG_WITH_STRING(ccache,
>-[  --with-ccache=path/to/ccache
>+[  --with-ccache[=path/to/ccache]
>                           Enable compiling with ccache],
>-    CCACHE_PATH=$withval,)
>-
>-if test -n "$CCACHE_PATH"; then
>-  CC="$CCACHE_PATH $CC"
>-  CXX="$CCACHE_PATH $CXX"
>+    CCACHE_PATH=$withval, CCACHE_PATH="no")

Lets say I pass --with-ccache=/exists/bin/ccache but ccache isn't in the path

>+
>+if test -z "$CCACHE_PATH"; then
>+    AC_MSG_ERROR([--with-ccache is set with a blank value])

This test passes.

>+elif test "$CCACHE_PATH" != "no"; then
>+    CCACHE="$CCACHE_PATH"

We set CCACHE to /exists/bin/ccache

>+    MOZ_PATH_PROG(CCACHE, ccache, :)

We clobber CCACHE here with :

>+    if test -z "$CCACHE" -o "$CCACHE" = ":"; then
>+        AC_MSG_ERROR([ccache not found in \$PATH])
>+    elif test -x "$CCACHE"; then
>+        CC="$CCACHE $CC"
>+        CXX="$CCACHE $CXX"
>+    else
>+        AC_MSG_ERROR([$CCACHE not found])
>+    fi

You should avoid setting CCACHE to CCACHE_PATH until after MOZ_PATH_PROGS and guard on the result of MOZ_PATH_PROG being ':' before you do that (in addition to erroring out if MOZ_PATH_PROGS returns ':' and CCACHE_PATH is yes.

Isn't autoconf fun? :-)
Comment on attachment 458907 [details] [diff] [review]
To solve this v4.

So yeah, r- until we fix that, but we're almost there!
Attachment #458907 - Flags: review?(me) → review-
Attached patch To solve this v5. (obsolete) — Splinter Review
"--with-ccache=" is treated as "--with-ccache" now.

If I set "--with-ccache=/usr/bin/ccache", configure script returns:
checking for /usr/bin/ccache... no
checking for ccache... /usr/bin/ccache

I cannot understand it.
Attachment #458907 - Attachment is obsolete: true
In case of "--with-ccache=/usr/bin/ccache", configure script returns
checking for /usr/bin/ccache... no
checking for ccache... /usr/bin/ccache
Attached patch To solve this v6Splinter Review
Attachment #458967 - Attachment is obsolete: true
Attachment #460205 - Flags: review?(me)
Comment on attachment 460205 [details] [diff] [review]
To solve this v6

I think this should do it.  r=me.

We can take this for 2.0, it's a simple build system change that's backwards compatible.
Attachment #460205 - Flags: review?(me)
Attachment #460205 - Flags: review+
Attachment #460205 - Flags: approval2.0?
Attachment #460205 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/55002e95d9e6
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b3
just to note, this breaks the ability to use a specific version of ccache that's in your path with something like --with-ccache=ccache2.4.1.
(In reply to comment #14)
> just to note, this breaks the ability to use a specific version of ccache
> that's in your path with something like --with-ccache=ccache2.4.1.

I've not tested ccache as another name such as ccache2.4.1.

I suppose binary name of ccache is defined as MYNAME in ccache.h in the ccache source. If you want to use another name for ccache binary, you will need to modify the definition of MYNAME. It's the issue of compiling ccache.

Please make sure to run
$ ccache2.4.1
returns help text, not 
ccache: FATAL: Could not find compiler "ccache2.4.1" in PATH

If you got FATAL message, please try to redefine MYNAME during compiling ccache.
If you got help text successfully, could you try --with-ccache=/foo/bin/ccache2.4.1 ?
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.