Move remoting dark/candle/light steps to external script

Move remoting dark/candle/light steps to external script

This avoids the need to chain the output of gyp rules, which is not supported
across generators.

See also https://chromiumcodereview.appspot.com/10381103/ and https://groups.google.com/forum/?fromgroups#!topic/gyp-developer/f_oCV-Lavpw

R=alexeypa@chromium.org
BUG=127444
NOTRY=true

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

[alexeypa (please no reviews), 2012-05-18 23:46:39]

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
File remoting/candle_and_light.py (right):

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
remoting/candle_and_light.py:22: parameters = {
The command line is not validated in any way so it will be harder to maintain
the GYP rules and script in sync.

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
remoting/candle_and_light.py:28: 'defines': sys.argv[6],
This will break as soon as someone adds another define to |wix_defines| in gyp.

There should be a better way of passing parameters to this script. Ideally
candle/light and script-only parameters should be easily separable.

[scottmg, 2012-05-19 00:59:54]

thanks

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
File remoting/candle_and_light.py (right):

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
remoting/candle_and_light.py:22: parameters = {
On 2012/05/18 23:46:40, alexeypa wrote:
> The command line is not validated in any way so it will be harder to maintain
> the GYP rules and script in sync.

Switched to use optparse and named parameters instead.

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
remoting/candle_and_light.py:28: 'defines': sys.argv[6],
On 2012/05/18 23:46:40, alexeypa wrote:
> This will break as soon as someone adds another define to |wix_defines| in
gyp.

Oops, yes, it should have been <(wix_defines), not <@(wix_defines). Fixed.

> There should be a better way of passing parameters to this script. Ideally
> candle/light and script-only parameters should be easily separable. 

optparse makes it easier to read a validates there's not extra or too few
arguments, and allows reordering.

I'm not clear on what you mean by separable parameters though.

[alexeypa (please no reviews), 2012-05-21 16:24:50]

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
File remoting/candle_and_light.py (right):

https://chromiumcodereview.appspot.com/10392172/diff/3001/remoting/candle_and...
remoting/candle_and_light.py:28: 'defines': sys.argv[6],
On 2012/05/19 00:59:55, scottmg wrote:
> I'm not clear on what you mean by separable parameters though.

Never mind. optparse should be enough for our needs.

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.gyp
File remoting/remoting.gyp (right):

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.g...
remoting/remoting.gyp:700: '--intermediate_root',
'<(INTERMEDIATE_DIR)/<(RULE_INPUT_ROOT)',
I think it is better to pass '--intermediate_dir', '<(INTERMEDIATE_DIR)'. The
name of .wixobj is not very important so the script can extract it from |input|.
The only important thing is that .wixobj will be in intermediate directory.

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.g...
remoting/remoting.gyp:702: '--defines', '<(_wix_defines)',
Does this work if there is no defines or more than one defines?

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.g...
remoting/remoting.gyp:749: '--intermediate_dir', '<(INTERMEDIATE_DIR)',
It is confusing that |intermediate_dir| is a parent of |intermediate_root|.

I think it is better to let the script to reconstruct the files and directory
names from |output|. This will add "-test" to the names of .wxs and .wixobj. It
will not affect anything.

N.B. the current meaning of |intermediate_dir| - the directory where .wxs (and
other files) are unpacked.

[scottmg, 2012-05-21 17:00:38]

Thanks

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.gyp
File remoting/remoting.gyp (right):

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.g...
remoting/remoting.gyp:700: '--intermediate_root',
'<(INTERMEDIATE_DIR)/<(RULE_INPUT_ROOT)',
On 2012/05/21 16:24:51, alexeypa wrote:
> I think it is better to pass '--intermediate_dir', '<(INTERMEDIATE_DIR)'. The
> name of .wixobj is not very important so the script can extract it from
|input|.
> The only important thing is that .wixobj will be in intermediate directory.

Done.

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.g...
remoting/remoting.gyp:702: '--defines', '<(_wix_defines)',
On 2012/05/21 16:24:51, alexeypa wrote:
> Does this work if there is no defines or more than one defines?

Yes, when a list is inserted into a string context, it's converted with quoted
spaces, and then the whole thing is quoted (with escapes if necessary).

wix_defines: ['a', 'b'] --> "\"a\" \"b\""
wix_defines: [] --> ""

https://chromiumcodereview.appspot.com/10392172/diff/7001/remoting/remoting.g...
remoting/remoting.gyp:749: '--intermediate_dir', '<(INTERMEDIATE_DIR)',
On 2012/05/21 16:24:51, alexeypa wrote:
> It is confusing that |intermediate_dir| is a parent of |intermediate_root|.

Agreed, they should have had better names at least.

> I think it is better to let the script to reconstruct the files and directory
> names from |output|. This will add "-test" to the names of .wxs and .wixobj.
It
> will not affect anything.
> 
> N.B. the current meaning of |intermediate_dir| - the directory where .wxs (and
> other files) are unpacked. 

I wasn't sure if you meant you wanted the intermediate_dir passed at all, but it
seemed better to keep it the same as the other script. So, it passes the
intermediate_dir, and builds the full name from intermediate_dir and output.

[alexeypa (please no reviews), 2012-05-21 17:07:28]

LGTM.

PS. I guess now I'm ready to fully switch to ninja. Great work!

[scottmg, 2012-05-21 17:08:36]

On 2012/05/21 17:07:28, alexeypa wrote:
> LGTM.
> 
> PS. I guess now I'm ready to fully switch to ninja.

w00t! :)

[commit-bot: I haz the power, 2012-05-21 17:09:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/10392172/11001

[commit-bot: I haz the power, 2012-05-21 18:36:12]

Try job failure for 10392172-11001 (retry) on linux_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-05-21 19:21:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/10392172/11001

[commit-bot: I haz the power, 2012-05-21 21:02:24]

Try job failure for 10392172-11001 (retry) on win_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[scottmg, 2012-05-21 22:46:15]

On 2012/05/21 21:02:24, I haz the power (commit-bot) wrote:
> Try job failure for 10392172-11001 (retry) on win_rel for step "compile"
> (clobber build).
> It's a second try, previously, step "compile" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

I see why the trailing '.' was there before now. :( Trailing \ on escapes the
closing quote on msvs generator.

Also, wix_defines was indeed wrong on msvs generator. Fixed by passing them
directly as the -d and having the script accept -d as a named argument.

[commit-bot: I haz the power, 2012-05-21 23:48:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/10392172/5004

[commit-bot: I haz the power, 2012-05-22 03:02:22]

Try job failure for 10392172-5004 (retry) (retry) on win_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-22 16:37:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/10392172/9003

[commit-bot: I haz the power, 2012-05-22 17:50:18]

Try job failure for 10392172-9003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[scottmg, 2012-05-22 17:55:39]

On 2012/05/22 17:50:18, I haz the power (commit-bot) wrote:
> Try job failure for 10392172-9003 (retry) on win for step "compile" (clobber

Bleh

TRACKER : error TRK0002: Failed to execute command: ""C:\Program Files
(x86)\Microsoft Visual Studio 10.0\VC\bin\CL.exe" /nologo
@C:\Users\CHROME~2\AppData\Local\Temp\cmdfile.tmp". The media is write
protected.

[commit-bot: I haz the power, 2012-05-22 17:56:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/10392172/9003

[commit-bot: I haz the power, 2012-05-22 23:19:20]

Try job failure for 10392172-9003 (retry) on win_rel for steps "base_unittests,
sync_unit_tests" (clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-22 23:23:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/10392172/9003

[commit-bot: I haz the power, 2012-05-22 23:23:31]

Change committed as 138392