The Chrome Remote Desktop installation for Windows can be build by bots now.

The Chrome Remote Desktop installation for Windows can be build by bots now.


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

[alexeypa (please no reviews), 2012-04-10 23:35:00]

Please take a look. The 1st patch is the original change that was reverted. See
https://chromiumcodereview.appspot.com/9980015/ for your reference.

The 2nd patchset is the fix that adds files to the official build properly.

[Sergey Ulanov, 2012-04-11 18:15:37]

+nsylvain - Nicolas, can you please take a look at this CL or suggest somebody
else who would be qualified to review it.

It looks good to me, but I'm not sure that it's Ok to disable the new target
when wix or SDK is not installed.

http://codereview.chromium.org/10031014/diff/2001/build/all.gyp
File build/all.gyp (right):

http://codereview.chromium.org/10031014/diff/2001/build/all.gyp#newcode535
build/all.gyp:535: ['component != "shared_library" and wix_exists == "True" and
platformsdk_exists == "True"', {
this can be wrapped to fit 80-chars

http://codereview.chromium.org/10031014/diff/2001/remoting/remoting.gyp
File remoting/remoting.gyp (right):

http://codereview.chromium.org/10031014/diff/2001/remoting/remoting.gyp#newco...
remoting/remoting.gyp:399: ['OS == "win" and component != "shared_library" and
wix_exists == "True" and platformsdk_exists == "True"', {
Wrap to to fit 80-chars lines.

[nsylvain, 2012-04-11 19:47:11]

redirecting to maruel and mmoss

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
File chrome/tools/build/win/FILES.cfg (right):

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
chrome/tools/build/win/FILES.cfg:420: 'buildtype': ['dev', 'official'],
If this is for "google chrome" only, maybe 'dev' should not be there?

[alexeypa (please no reviews), 2012-04-11 19:52:53]

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
File chrome/tools/build/win/FILES.cfg (right):

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
chrome/tools/build/win/FILES.cfg:420: 'buildtype': ['dev', 'official'],
On 2012/04/11 19:47:11, nsylvain wrote:
> If this is for "google chrome" only, maybe 'dev' should not be there?

Not, it is for both dev and official builds. For dev builds one should either
have files from the internal svn or specify wix_path% and platformsdk_path% in
~/.gyp/include.gypi.

[alexeypa (please no reviews), 2012-04-11 19:55:38]

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
File chrome/tools/build/win/FILES.cfg (right):

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
chrome/tools/build/win/FILES.cfg:420: 'buildtype': ['dev', 'official'],
On 2012/04/11 19:52:53, alexeypa wrote:
> On 2012/04/11 19:47:11, nsylvain wrote:
> > If this is for "google chrome" only, maybe 'dev' should not be there?
> 
> Not, it is for both dev and official builds. For dev builds one should either
> have files from the internal svn or specify wix_path% and platformsdk_path% in
> ~/.gyp/include.gypi.

On a second thought though, you may be right. Is FILES.cfg involved in any sort
of build that an external developer can run?

[Michael Moss, 2012-04-11 20:07:16]

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
File chrome/tools/build/win/FILES.cfg (right):

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/FILE...
chrome/tools/build/win/FILES.cfg:420: 'buildtype': ['dev', 'official'],
FILES.cfg should just be used by the buildbot scripts. I'm not going to worry if
this breaks any other usage.

[Michael Moss, 2012-04-11 20:12:04]

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/SYMBOLS
File chrome/tools/build/win/SYMBOLS (right):

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/SYMB...
chrome/tools/build/win/SYMBOLS:11: remoting_service.pdb
BTW, I recently merged SYMBOLS into FILES.cfg. You should put these entries
there with a 'filegroup' of 'symbols'. See 'syzygy/mini_installer.pdb' for an
example. This file will be going away very soon.

[alexeypa (please no reviews), 2012-04-11 20:54:28]

PTAL.

http://codereview.chromium.org/10031014/diff/2001/build/all.gyp
File build/all.gyp (right):

http://codereview.chromium.org/10031014/diff/2001/build/all.gyp#newcode535
build/all.gyp:535: ['component != "shared_library" and wix_exists == "True" and
platformsdk_exists == "True"', {
On 2012/04/11 18:15:37, sergeyu wrote:
> this can be wrapped to fit 80-chars

Done.

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/SYMBOLS
File chrome/tools/build/win/SYMBOLS (right):

http://codereview.chromium.org/10031014/diff/2001/chrome/tools/build/win/SYMB...
chrome/tools/build/win/SYMBOLS:11: remoting_service.pdb
On 2012/04/11 20:12:04, Michael Moss wrote:
> BTW, I recently merged SYMBOLS into FILES.cfg. You should put these entries
> there with a 'filegroup' of 'symbols'. See 'syzygy/mini_installer.pdb' for an
> example. This file will be going away very soon.

Done.

http://codereview.chromium.org/10031014/diff/2001/remoting/remoting.gyp
File remoting/remoting.gyp (right):

http://codereview.chromium.org/10031014/diff/2001/remoting/remoting.gyp#newco...
remoting/remoting.gyp:399: ['OS == "win" and component != "shared_library" and
wix_exists == "True" and platformsdk_exists == "True"', {
On 2012/04/11 18:15:37, sergeyu wrote:
> Wrap to to fit 80-chars lines.

Done.

[Michael Moss, 2012-04-11 20:59:17]

LGTM with possible typo.

http://codereview.chromium.org/10031014/diff/16006/chrome/tools/build/win/FIL...
File chrome/tools/build/win/FILES.cfg (right):

http://codereview.chromium.org/10031014/diff/16006/chrome/tools/build/win/FIL...
chrome/tools/build/win/FILES.cfg:603: 'filename': '+remoting_me2me_host.pdb',
Is that "+" a typo?

[alexeypa (please no reviews), 2012-04-11 21:02:13]

Thanks.

http://codereview.chromium.org/10031014/diff/16006/chrome/tools/build/win/FIL...
File chrome/tools/build/win/FILES.cfg (right):

http://codereview.chromium.org/10031014/diff/16006/chrome/tools/build/win/FIL...
chrome/tools/build/win/FILES.cfg:603: 'filename': '+remoting_me2me_host.pdb',
On 2012/04/11 20:59:17, Michael Moss wrote:
> Is that "+" a typo?

Yes, definitely.

[commit-bot: I haz the power, 2012-04-11 21:03:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/10031014/14007

[commit-bot: I haz the power, 2012-04-12 01:04:17]

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

[commit-bot: I haz the power, 2012-04-12 02:32:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/10031014/14007

[commit-bot: I haz the power, 2012-04-12 03:59:45]

Try job failure for 10031014-14007 (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-04-12 08:26:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/10031014/14007

[commit-bot: I haz the power, 2012-04-12 09:50:13]

Change committed as 131960