roll clang 138187:138188

roll clang 138187:138188

(Rolling just 1 revision because something farther on seems to tickle an ld crash, but I'd like to get the rest of this CL in.)

Also pass --disable-threads and --disable-pthreads to configure, for goma.

Also modify the script to try to download prebuilt binaries first. This is faster,
and makes sure the bots use the exact same binary as goma. The binaries are
downloaded from http://commondatastorage.googleapis.com/chromium-browser-clang/index.html

Also add a script that packages a clang build up into a tgz for uploading with
gsutil, based on code by ukai@.

Finally, if the svn checkout fails, remove the destination directory and try
again (for bots that have a checkout from before we had an internal llvm svn
mirror).

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98416

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98541

[hans, 2011-08-26 09:02:46]

Oh, didn't notice that I was just CC'd and not in the Reviewers field.

Anyway, LGTM, just a nit or two.

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh
File tools/clang/scripts/package.sh (right):

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh...
tools/clang/scripts/package.sh:23: PDIR=clang-$R
you've been careful to use absolute paths so far, until here. maybe this should
be PDIR="${THIS_DIR}/clang-${R}", and quoted below as the other paths?
Or is it intentional that PDIR is relative to CWD, rather than the dir the
script lives in?

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh...
tools/clang/scripts/package.sh:31: (cd $PDIR/bin && ln -sf clang clang++ && cd
-)
is the space in the first column intentional?

[Nico, 2011-08-26 09:06:19]

Thanks!

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh
File tools/clang/scripts/package.sh (right):

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh...
tools/clang/scripts/package.sh:23: PDIR=clang-$R
On 2011/08/26 09:02:46, hans wrote:
> you've been careful to use absolute paths so far, until here. maybe this
should
> be PDIR="${THIS_DIR}/clang-${R}", and quoted below as the other paths?
> Or is it intentional that PDIR is relative to CWD, rather than the dir the
> script lives in?

This is intentional; the tgz file should just appear where I run this.

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh...
tools/clang/scripts/package.sh:31: (cd $PDIR/bin && ln -sf clang clang++ && cd
-)
On 2011/08/26 09:02:46, hans wrote:
> is the space in the first column intentional?

Hm, I don't see a space. You mean the '('?

[hans, 2011-08-26 09:17:41]

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh
File tools/clang/scripts/package.sh (right):

http://codereview.chromium.org/7741039/diff/14/tools/clang/scripts/package.sh...
tools/clang/scripts/package.sh:31: (cd $PDIR/bin && ln -sf clang clang++ && cd
-)
On 2011/08/26 09:06:19, Nico wrote:
> On 2011/08/26 09:02:46, hans wrote:
> > is the space in the first column intentional?
> 
> Hm, I don't see a space. You mean the '('?

You're right. Sorry, I probably need my eyes adjusted or something :-p

[Mark Mentovai, 2011-08-26 16:55:19]

Thoughts, suggestions, and questions for when you recommit this.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/package.sh
File tools/clang/scripts/package.sh (right):

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/package....
tools/clang/scripts/package.sh:24: rm -rf $PDIR
It’s wise to always use this as "${PDIR}" instead of $PDIR.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/package....
tools/clang/scripts/package.sh:35: if [ "$(uname -s)" = "Darwin" ]; then
Same |uname -s| suggestion as in the other file applies here.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/package....
tools/clang/scripts/package.sh:53: echo gsutil cp -a public-read $PDIR.tgz \
What sort of access control do we have on this?

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.sh
File tools/clang/scripts/update.sh (right):

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:39:
CDS_URL=http://commondatastorage.googleapis.com/chromium-browser-clang
Can you skip the “try to download binary” step if a special environment variable
is set? Maybe stick this entire block inside

if [[ -z "${CLANG_FORCE_BUILD_FROM_SOURCE}" ]]; then

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:41: echo Trying to download prebuilt clang
It’s good practice to quote the entire string you’re passing to echo. Also in
cases like the ones you have on lines 51 and 55, you can put the quotes around
the entire string you want to print, not just around ${CLANG_REVISION}.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:42: if [ "$(uname -s)" = "Linux" ]; then
You run |uname -s| repeatedly to check the same thing. Just run it once and save
the result in a variable.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:64: echo Checkout failed, retrying
Why do you retry this one but not the one to check out tools/clang?

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:88: --disable-threads \
Curious about what GOMA’s requirements are that make this necessary.

Also curious about what impact this has on the built-up compiler.

[Nico, 2011-08-27 05:25:33]

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/package.sh
File tools/clang/scripts/package.sh (right):

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/package....
tools/clang/scripts/package.sh:24: rm -rf $PDIR
On 2011/08/26 16:55:19, Mark Mentovai wrote:
> It’s wise to always use this as "${PDIR}" instead of $PDIR.

Done.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/package....
tools/clang/scripts/package.sh:53: echo gsutil cp -a public-read $PDIR.tgz \
On 2011/08/26 16:55:19, Mark Mentovai wrote:
> What sort of access control do we have on this?

Everyone can read, only people with Magic Permissions handed out by nsylvain can
write.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.sh
File tools/clang/scripts/update.sh (right):

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:39:
CDS_URL=http://commondatastorage.googleapis.com/chromium-browser-clang
On 2011/08/26 16:55:19, Mark Mentovai wrote:
> Can you skip the “try to download binary” step if a special environment
variable
> is set? Maybe stick this entire block inside
> 
> if [[ -z "${CLANG_FORCE_BUILD_FROM_SOURCE}" ]]; then

I wanted to add some flag for that, and I'm waiting on
http://codereview.chromium.org/7741026/ to land so that I can build on the flag
processing code in their.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:41: echo Trying to download prebuilt clang
On 2011/08/26 16:55:19, Mark Mentovai wrote:
> It’s good practice to quote the entire string you’re passing to echo. Also in
> cases like the ones you have on lines 51 and 55, you can put the quotes around
> the entire string you want to print, not just around ${CLANG_REVISION}.

Done.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:42: if [ "$(uname -s)" = "Linux" ]; then
On 2011/08/26 16:55:19, Mark Mentovai wrote:
> You run |uname -s| repeatedly to check the same thing. Just run it once and
save
> the result in a variable.

It's cheap to run and its result is unlikely to change.

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:64: echo Checkout failed, retrying
On 2011/08/26 16:55:19, Mark Mentovai wrote:
> Why do you retry this one but not the one to check out tools/clang?

clang's checked out to llvm/tools/clang. It's very unlikely that one checkout is
from the internal mirror and the other isn't (since llvm is checked out first,
and since LLVM_REPO_URL doesn't change during the execution of this script, it
can only happen if someone mucks around manually on the bots)

http://codereview.chromium.org/7741039/diff/8016/tools/clang/scripts/update.s...
tools/clang/scripts/update.sh:88: --disable-threads \
On 2011/08/26 16:55:19, Mark Mentovai wrote:
> Curious about what GOMA’s requirements are that make this necessary.

They run gcc in a fake chrooted sandbox, and I'm guessing that sandbox doesn't
permit threads.

> Also curious about what impact this has on the built-up compiler.

Me too! If I read the source correctly, the only effect of this is to replace
all methods on llvm's Mutex class with no-ops. This class isn't used at all in
tools/clang, and is only used in the JIT part of llvm (I think). I don't think
clang uses multiple threads when compiling.

I relanded this CL with a clang revision increment of 1, so if ld crashes again
on the waterfall, it's pretty likely it's caused by this configure change. So
far, everything looks good though.

[Nico, 2011-08-28 00:53:29]

> I relanded this CL with a clang revision increment of 1, so if ld crashes
again
> on the waterfall, it's pretty likely it's caused by this configure change. So
> far, everything looks good though.

I tracked down the ld crash, it's caused by this clang revision:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110822/045707.html
(see the tryjob results at http://codereview.chromium.org/7777001/ )

It looks like the crash is present with Xcode 3.2.2's ld, but fixed in Xcode
3.2.3's ld. (At least, I see the crash on the bots but not locally, and I have a
newer ld locally.) What's the best way forward here? Require people and bots to
use 3.2.3? Do you know how difficult it is to upgrade Xcode on the bots?

[Mark Mentovai, 2011-08-28 03:33:24]

thakis@chromium.org wrote:
>> I relanded this CL with a clang revision increment of 1, so if ld crashes
>
> again
>>
>> on the waterfall, it's pretty likely it's caused by this configure change.
>> So
>> far, everything looks good though.
>
> I tracked down the ld crash, it's caused by this clang revision:
>
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110822/045707.html
> (see the tryjob results at http://codereview.chromium.org/7777001/ )
>
> It looks like the crash is present with Xcode 3.2.2's ld, but fixed in Xcode
> 3.2.3's ld. (At least, I see the crash on the bots but not locally, and I
> have a newer ld locally.) What's the best way forward here? Require people and
bots
> to use 3.2.3? Do you know how difficult it is to upgrade Xcode on the bots?

Bev, can you tell us which Xcode version the bots are using? We should
be on 3.2.6. I know we talked about this back in April but I’m not
sure if the upgrades were rolled out to the entire fleet.

[bevc, 2011-08-28 17:07:18]

On Sat, Aug 27, 2011 at 8:33 PM, Mark Mentovai <mark@chromium.org> wrote:

> thakis@chromium.org wrote:
> >> I relanded this CL with a clang revision increment of 1, so if ld
> crashes
> >
> > again
> >>
> >> on the waterfall, it's pretty likely it's caused by this configure
> change.
> >> So
> >> far, everything looks good though.
> >
> > I tracked down the ld crash, it's caused by this clang revision:
> >
>
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110822/045707.html
> > (see the tryjob results at http://codereview.chromium.org/7777001/ )
> >
> > It looks like the crash is present with Xcode 3.2.2's ld, but fixed in
> Xcode
> > 3.2.3's ld. (At least, I see the crash on the bots but not locally, and I
> > have a newer ld locally.) What's the best way forward here? Require
> people and bots
> > to use 3.2.3? Do you know how difficult it is to upgrade Xcode on the
> bots?
>
> Bev, can you tell us which Xcode version the bots are using? We should
> be on 3.2.6. I know we talked about this back in April but I’m not
> sure if the upgrades were rolled out to the entire fleet.
>

We're still on  xcode 3.2.2. I can upgrade the entire fleet today.

[Mark Mentovai, 2011-08-28 17:32:35]

Bev Cristobal wrote:
> We're still on  xcode 3.2.2. I can upgrade the entire fleet today.

I think that’d be good.

We are on 10.6.8 now across the board, right?

[bevc1, 2011-08-28 17:59:53]

On Sun, Aug 28, 2011 at 10:32 AM, Mark Mentovai <mark@chromium.org> wrote:

> Bev Cristobal wrote:
> > We're still on  xcode 3.2.2. I can upgrade the entire fleet today.
>
> I think that’d be good.
>
> We are on 10.6.8 now across the board, right?
>

We will be by the end of the day.

[Mark Mentovai, 2011-08-28 18:02:48]

Bev Cristobal wrote:
> On Sun, Aug 28, 2011 at 10:32 AM, Mark Mentovai <mark@chromium.org> wrote:
>> We are on 10.6.8 now across the board, right?
>
> We will be by the end of the day.

Awesome. Thanks, Bev!

[Mark Mentovai, 2011-08-28 18:20:49]

thakis@chromium.org wrote:
> It looks like the crash is present with Xcode 3.2.2's ld, but fixed in Xcode
> 3.2.3's ld. (At least, I see the crash on the bots but not locally, and I have
a
> newer ld locally.) What's the best way forward here? Require people and bots
to
> use 3.2.3? Do you know how difficult it is to upgrade Xcode on the bots?

To answer Nico’s question more directly:

It’s perfectly reasonable to require 3.2.6.