Make GOMA_DIR injected from env.

Make GOMA_DIR injected from env.

Also,
if NOCONTROL_GOMA environment is set, the script will not start/stop goma compiler_proxy.  I am going to make goma recipe module start/stop goma compiler_proxy in the future, and the environment will be used at that time.

BUG=663630
Committed: https://chromium.googlesource.com/native_client/src/native_client/+/163dfeb43e76995b4265ecd4e78670f7dd432e44

[Yoshisato Yanagisawa, 2016-11-10 07:51:05]

yyanagisawa@chromium.org changed reviewers:
+ bradnelson@chromium.org, ukai@chromium.org

[Yoshisato Yanagisawa, 2016-11-10 07:51:05]

yyanagisawa@chromium.org changed required reviewers:
+ bradnelson@chromium.org

[ukai, 2016-11-10 08:01:07]

lgtm

https://codereview.chromium.org/2486313003/diff/1/buildbot/buildbot_pnacl_too...
File buildbot/buildbot_pnacl_toolchain.py (right):

https://codereview.chromium.org/2486313003/diff/1/buildbot/buildbot_pnacl_too...
buildbot/buildbot_pnacl_toolchain.py:34: start_goma = not
os.environ.get('NOSTART_GOMA')
this will be used to let goma start/stop in caller (recipe etc) ?

better to have comment or explain in CL description.

maybe, after use_goma= below? 

control_goma = use_goma and not os.environ.get('..

[Yoshisato Yanagisawa, 2016-11-10 08:28:13]

Description was changed from

==========
Make GOMA_DIR injected from env.

BUG=663630
==========

to

==========
Make GOMA_DIR injected from env.

Also,
if NOSTART_GOMA environment is set, the script will not start/stop goma
compiler_proxy.  I am going to make goma recipe module start/stop goma
compiler_proxy in the future, and the environment will be used at that time.

BUG=663630
==========

[Yoshisato Yanagisawa, 2016-11-10 08:31:56]

Description was changed from

==========
Make GOMA_DIR injected from env.

Also,
if NOSTART_GOMA environment is set, the script will not start/stop goma
compiler_proxy.  I am going to make goma recipe module start/stop goma
compiler_proxy in the future, and the environment will be used at that time.

BUG=663630
==========

to

==========
Make GOMA_DIR injected from env.

Also,
if NOCONTROL_GOMA environment is set, the script will not start/stop goma
compiler_proxy.  I am going to make goma recipe module start/stop goma
compiler_proxy in the future, and the environment will be used at that time.

BUG=663630
==========

[Yoshisato Yanagisawa, 2016-11-10 08:32:38]

https://codereview.chromium.org/2486313003/diff/1/buildbot/buildbot_pnacl_too...
File buildbot/buildbot_pnacl_toolchain.py (right):

https://codereview.chromium.org/2486313003/diff/1/buildbot/buildbot_pnacl_too...
buildbot/buildbot_pnacl_toolchain.py:34: start_goma = not
os.environ.get('NOSTART_GOMA')
On 2016/11/10 08:01:07, ukai wrote:
> this will be used to let goma start/stop in caller (recipe etc) ?
> 
> better to have comment or explain in CL description.
> 
> maybe, after use_goma= below? 
> 
> control_goma = use_goma and not os.environ.get('..

Done.

[ukai, 2016-11-11 02:56:10]

lgtm

[bradnelson, 2016-11-11 19:44:03]

The CQ bit was checked by bradnelson@chromium.org

[bradnelson, 2016-11-11 19:44:03]

lgtm

[commit-bot: I haz the power, 2016-11-11 19:44:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 16:14:19]

Description was changed from

==========
Make GOMA_DIR injected from env.

Also,
if NOCONTROL_GOMA environment is set, the script will not start/stop goma
compiler_proxy.  I am going to make goma recipe module start/stop goma
compiler_proxy in the future, and the environment will be used at that time.

BUG=663630
==========

to

==========
Make GOMA_DIR injected from env.

Also,
if NOCONTROL_GOMA environment is set, the script will not start/stop goma
compiler_proxy.  I am going to make goma recipe module start/stop goma
compiler_proxy in the future, and the environment will be used at that time.

BUG=663630

Committed:
https://chromium.googlesource.com/native_client/src/native_client/+/163dfeb43...
==========

[commit-bot: I haz the power, 2016-11-14 16:14:20]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/native_client/src/native_client/+/163dfeb43...

[Derek Schuff, 2017-01-03 20:33:04]

On 2016/11/14 16:14:20, commit-bot: I haz the power wrote:
> Committed patchset #2 (id:20001) as
>
https://chromium.googlesource.com/native_client/src/native_client/+/163dfeb43...

This seems to have broken the PNaCl mac buildbot (e.g.
https://uberchromegw.corp.google.com/i/client.nacl.toolchain/builders/mac-pna...)
which has the NOCONTROL_GOMA var set (and so doesn't manually start goma) but it
appears that goma is not getting started elsewhere either. Is there some other
change we need to make to get this working again?

[Fumitoshi Ukai, 2017-01-05 06:07:43]

On 2017/01/03 20:33:04, Derek Schuff wrote:
> On 2016/11/14 16:14:20, commit-bot: I haz the power wrote:
> > Committed patchset #2 (id:20001) as
> >
>
https://chromium.googlesource.com/native_client/src/native_client/+/163dfeb43...
> 
> This seems to have broken the PNaCl mac buildbot (e.g.
>
https://uberchromegw.corp.google.com/i/client.nacl.toolchain/builders/mac-pna...)
> which has the NOCONTROL_GOMA var set (and so doesn't manually start goma) but
it
> appears that goma is not getting started elsewhere either. Is there some other
> change we need to make to get this working again?

preprocess_for_goma.start_goma step should start goma.

https://uberchromegw.corp.google.com/i/client.nacl.toolchain/builders/mac-pna...

https://uberchromegw.corp.google.com/i/client.nacl.toolchain/builders/mac-pna...

-- Check for working CXX compiler: /b/build/slave/cache/cipd/goma/clang++ --
broken
CMake Error at
/b/build/slave/nacl-toolchain/build/native_client/cmake343/share/cmake-3.4/Modules/CMakeTestCXXCompiler.cmake:54
(message):
  The C++ compiler "/b/build/slave/cache/cipd/goma/clang++" is not able to
  compile a simple test program.

cmake doesn't support goma well?
I think checking CXX compiler should check actual compiler and cmake emits
"/b/build/slave/cache/cpid/goma/gomacc /path/to/clang++" in build.ninja.

[Yoshisato Yanagisawa, 2017-01-05 07:32:00]

Ah, this should be the case wrapping with goma module does not work well.
compiler_proxy <-> gomacc communication does not work well in this case.  We
might need to set GOMA_TMP_DIR as a workaround.

As Ukai-san said, I also feel using masquerade mode gomacc not good, though.

[Derek Schuff, 2017-01-05 17:49:25]

On 2017/01/05 07:32:00, Yoshisato Yanagisawa wrote:
> Ah, this should be the case wrapping with goma module does not work well.
> compiler_proxy <-> gomacc communication does not work well in this case.  We
> might need to set GOMA_TMP_DIR as a workaround.
> 
> As Ukai-san said, I also feel using masquerade mode gomacc not good, though.

Yeah, that's a separate issue. Recent version of CMake support a flag which
allows injecting a launcher like goma or ccache, e.g.
-DCMAKE_C_COMPILER_LAUNCHER=/path/to/goma which is what we use for the
WebAssembly waterfall. Last time I checked (a while ago), the version of CMake
installed on the bots is too old and doesn't support that, so that's why the
PNaCl waterfall doesn't use it. For WebAssembly the bot script manually
downloads a newer CMake.

[Yoshisato Yanagisawa, 2017-01-06 01:24:34]

On 2017/01/05 17:49:25, Derek Schuff wrote:
> On 2017/01/05 07:32:00, Yoshisato Yanagisawa wrote:
> > Ah, this should be the case wrapping with goma module does not work well.
> > compiler_proxy <-> gomacc communication does not work well in this case.  We
> > might need to set GOMA_TMP_DIR as a workaround.
> > 
> > As Ukai-san said, I also feel using masquerade mode gomacc not good, though.
> 
> Yeah, that's a separate issue. Recent version of CMake support a flag which
> allows injecting a launcher like goma or ccache, e.g.
> -DCMAKE_C_COMPILER_LAUNCHER=/path/to/goma which is what we use for the
> WebAssembly waterfall. Last time I checked (a while ago), the version of CMake
> installed on the bots is too old and doesn't support that, so that's why the
> PNaCl waterfall doesn't use it. For WebAssembly the bot script manually
> downloads a newer CMake.

I doubt it worth paying some efforts to make the script works like so but not to
forget this, I have filed the issue:
https://bugs.chromium.org/p/chromium/issues/detail?id=678857