Support custom path of blink_gc_plugin for distributed builds.

Support custom path of blink_gc_plugin for distributed builds.

A "custom_clang_lib_path" argument is added to specify a custom path of
blink_gc_plugin for distributed builds such as distcc.

R=hans@chromium.org

Committed: https://crrev.com/188f74d1b676853bfa89477b66707e0b8b418d9b
Cr-Commit-Position: refs/heads/master@{#325442}

[wjywbs, 2015-04-14 22:24:47]

Please take a look, thanks.

[hans, 2015-04-15 09:33:42]

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
File tools/clang/scripts/blink_gc_plugin_flags.sh (right):

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
tools/clang/scripts/blink_gc_plugin_flags.sh:29: CLANG_LIB_PATH="$(cd ${arg#*=}
&& echo $PWD)"
Why bash is rusty. What does ${arg#*=} do?

[wjywbs, 2015-04-15 15:23:29]

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
File tools/clang/scripts/blink_gc_plugin_flags.sh (right):

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
tools/clang/scripts/blink_gc_plugin_flags.sh:29: CLANG_LIB_PATH="$(cd ${arg#*=}
&& echo $PWD)"
On 2015/04/15 09:33:42, hans wrote:
> Why bash is rusty. What does ${arg#*=} do?

If arg="custom_clang_lib_path=/tmp/llvm", then "${arg#*=}" will be "/tmp/llvm".

[hans, 2015-04-15 15:36:09]

I'm trying to understand the motivation for this patch.

You want to use a different blink_gc_plugin than the one that was built with
Clang? As Clang changes, is that safe? And where does distcc come into the
picture?

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
File tools/clang/scripts/blink_gc_plugin_flags.sh (right):

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
tools/clang/scripts/blink_gc_plugin_flags.sh:29: CLANG_LIB_PATH="$(cd ${arg#*=}
&& echo $PWD)"
On 2015/04/15 15:23:29, wjywbs wrote:
> On 2015/04/15 09:33:42, hans wrote:
> > Why bash is rusty. What does ${arg#*=} do?
> 
> If arg="custom_clang_lib_path=/tmp/llvm", then "${arg#*=}" will be
"/tmp/llvm".

Thanks.

What about quotes? In the "$arg" = test above, should the right-hand side be
quoted?

On line 29, the cd command will probably not work if the path has spaces, since
${arg#*=} is not quoted. Maybe that should be assigned to a variable first?

[wjywbs, 2015-04-16 02:19:22]

On 2015/04/15 15:36:09, hans wrote:
> I'm trying to understand the motivation for this patch.
> 
> You want to use a different blink_gc_plugin than the one that was built with
> Clang? As Clang changes, is that safe? And where does distcc come into the
> picture?

This script will output default blink_gc_plugin path to (for example)
/home/abc/chromium/src/third_party/llvm-build/Release+Asserts/lib/libBlinkGCPlugin.so.
It's hard to extract prebuilt clang in this path on the remote machine. distcc
will run clang with the plugin path on remote machine. clang will fail if it can
not find the plugin at the default path. Could you describe how goma solves this
if possible? I'm not using a different blink_gc_plugin.

[wjywbs, 2015-04-16 02:20:41]

Sorry that I didn't publish my drafts in previous comment.

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
File tools/clang/scripts/blink_gc_plugin_flags.sh (right):

https://codereview.chromium.org/1052033006/diff/1/tools/clang/scripts/blink_g...
tools/clang/scripts/blink_gc_plugin_flags.sh:29: CLANG_LIB_PATH="$(cd ${arg#*=}
&& echo $PWD)"
On 2015/04/15 15:36:09, hans wrote:
> What about quotes? In the "$arg" = test above, should the right-hand side be
> quoted?
> 
> On line 29, the cd command will probably not work if the path has spaces,
since
> ${arg#*=} is not quoted. Maybe that should be assigned to a variable first?

I tried that quoted string was for exact match, and non-quoted string was for
pattern matching.

I quoted ${arg#*=} and the echo line below for path with spaces.

[hans, 2015-04-16 10:51:38]

On 2015/04/16 02:19:22, wjywbs wrote:
> On 2015/04/15 15:36:09, hans wrote:
> > I'm trying to understand the motivation for this patch.
> > 
> > You want to use a different blink_gc_plugin than the one that was built with
> > Clang? As Clang changes, is that safe? And where does distcc come into the
> > picture?
> 
> This script will output default blink_gc_plugin path to (for example)
>
/home/abc/chromium/src/third_party/llvm-build/Release+Asserts/lib/libBlinkGCPlugin.so.
> It's hard to extract prebuilt clang in this path on the remote machine. distcc
> will run clang with the plugin path on remote machine. clang will fail if it
can
> not find the plugin at the default path. Could you describe how goma solves
this
> if possible? I'm not using a different blink_gc_plugin.

Ah, I understand better what you mean now.

I don't know how goma does it. Maybe it massages paths to third_party/llvm-build
or something.

[hans, 2015-04-16 10:51:45]

lgtm

[wjywbs, 2015-04-16 14:03:01]

The CQ bit was checked by wjywbs@gmail.com

[commit-bot: I haz the power, 2015-04-16 14:03:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1052033006/20001

[commit-bot: I haz the power, 2015-04-16 14:33:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-16 14:34:00]

Patchset 2 (id:??) landed as
https://crrev.com/188f74d1b676853bfa89477b66707e0b8b418d9b
Cr-Commit-Position: refs/heads/master@{#325442}