Skip to content

Commit

Permalink
Rework how remoting JS compilation GYP works.
Browse files Browse the repository at this point in the history
There was an unfortunate bug that swallowed Java stacks.
Issues were discovered when Java stacks started surfacing:
https://codereview.chromium.org/476453002/

So change the way remoting is built. It doesn't need to be
built on all bots by default: they have undefined Java versions
and likely weren't working before (and shouldn't differ).

Instead, run when run_jscompile=1 is defined in GYP:

  export GYP_DEFINES=run_jscompile=1 && build/gyp_chromium
  # or add in ~/.gyp/includes.gypi or $SRC/../chromium.gyp_env
  # or build/gyp_chromium -Drun_jscompile=1

and on the Closure Compilation Linux FYI bot:
http://build.chromium.org/p/chromium.fyi/builders/Closure%20Compilation%20Linux

This also fixes some GYP inputs problems (if I changed compile_js.gypi or
other things in remoting/ ninja wouldn't rebuild anything).

R=jamiewalch@chromium.org
BUG=none
TEST=green bots, remoting runs on Closure Compliation Linux FYI bot

Review URL: https://codereview.chromium.org/1037613002

Cr-Commit-Position: refs/heads/master@{#322193}
  • Loading branch information
danbeam authored and Commit bot committed Mar 25, 2015
1 parent ae0c693 commit b59fe18
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 76 deletions.
11 changes: 1 addition & 10 deletions remoting/remoting_options.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
'chromium_code': 1,

# Set this to run the jscompile checks after building the webapp.
'run_jscompile%': 1,
'run_jscompile%': 0,

# Set this to enable cast mode on the android client.
'enable_cast%': 0,
Expand All @@ -27,14 +27,5 @@
'remoting_rdp_session%': 1,

'branding_path': '../remoting/branding_<(branding)',

'conditions': [
['OS=="win"', {
# Java is not available on Windows bots, so we need to disable
# JScompile checks.
'run_jscompile': 0,
}],
],
},

}
66 changes: 1 addition & 65 deletions remoting/remoting_webapp.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -22,71 +22,7 @@
],
'conditions': [
['run_jscompile != 0', {
'variables': {
'success_stamp': '<(PRODUCT_DIR)/<(_target_name)_jscompile.stamp',
'success_stamp_bt': '<(PRODUCT_DIR)/<(_target_name)_bt_jscompile.stamp',
'success_stamp_ut': '<(PRODUCT_DIR)/<(_target_name)_ut_jscompile.stamp',
},
'actions': [
{
'action_name': 'Verify remoting webapp',
'inputs': [
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_js_proto_files)',
],
'outputs': [
'<(success_stamp)',
],
'action': [
'python', '../third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp)',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_js_proto_files)',
],
},
{
'action_name': 'Verify remoting webapp with browsertests',
'inputs': [
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_browsertest_all_js_files)',
'<@(remoting_webapp_browsertest_js_proto_files)',
],
'outputs': [
'<(success_stamp_bt)',
],
'action': [
'python', '../third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp_bt)',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_browsertest_all_js_files)',
'<@(remoting_webapp_browsertest_js_proto_files)',
],
},
{
'action_name': 'Verify remoting webapp unittests',
'inputs': [
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_unittests_all_js_files)',
'<@(remoting_webapp_unittests_js_proto_files)',
],
'outputs': [
'<(success_stamp_ut)',
],
'action': [
'python', '../third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp_ut)',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_unittests_all_js_files)',
'<@(remoting_webapp_unittests_js_proto_files)',
],
},
], # actions
'includes': ['remoting_webapp_compile.gypi'],
}],
],
'actions': [
Expand Down
80 changes: 80 additions & 0 deletions remoting/remoting_webapp_compile.gypi
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Copyright 2015 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
#
# File in charge of Closure compiling remoting's webapp.

{
'variables': {
'success_stamp': '<(PRODUCT_DIR)/<(_target_name)_jscompile.stamp',
'success_stamp_bt': '<(PRODUCT_DIR)/<(_target_name)_bt_jscompile.stamp',
'success_stamp_ut': '<(PRODUCT_DIR)/<(_target_name)_ut_jscompile.stamp',
},
'actions': [
{
'action_name': 'Verify remoting webapp',
'inputs': [
'remoting_webapp_compile.gypi',
'remoting_webapp_files.gypi',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_js_proto_files)',
],
'outputs': [
'<(success_stamp)',
],
'action': [
'python', '<(DEPTH)/third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp)',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_js_proto_files)',
],
},
{
'action_name': 'Verify remoting webapp with browsertests',
'inputs': [
'remoting_webapp_compile.gypi',
'remoting_webapp_files.gypi',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_browsertest_all_js_files)',
'<@(remoting_webapp_browsertest_js_proto_files)',
],
'outputs': [
'<(success_stamp_bt)',
],
'action': [
'python', '<(DEPTH)/third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp_bt)',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_browsertest_all_js_files)',
'<@(remoting_webapp_browsertest_js_proto_files)',
],
},
{
'action_name': 'Verify remoting webapp unittests',
'inputs': [
'remoting_webapp_compile.gypi',
'remoting_webapp_files.gypi',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_unittests_all_js_files)',
'<@(remoting_webapp_unittests_js_proto_files)',
],
'outputs': [
'<(success_stamp_ut)',
],
'action': [
'python', '<(DEPTH)/third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp_ut)',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_unittests_all_js_files)',
'<@(remoting_webapp_unittests_js_proto_files)',
],
},
],
'includes': ['remoting_webapp_files.gypi'],
}
1 change: 1 addition & 0 deletions third_party/closure_compiler/compile_js.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
'depends%': [],
},
'inputs': [
'compile_js.gypi',
'<(CLOSURE_DIR)/checker.py',
'<(CLOSURE_DIR)/processor.py',
'<(CLOSURE_DIR)/build/inputs.py',
Expand Down
5 changes: 4 additions & 1 deletion third_party/closure_compiler/compiled_resources.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@
'../../ui/webui/resources/js/chromeos/compiled_resources.gyp:*',
'../../ui/webui/resources/js/compiled_resources.gyp:*',
'../../ui/webui/resources/js/cr/ui/compiled_resources.gyp:*',
]
],
'includes': [
'../../remoting/remoting_webapp_compile.gypi',
],
},
]
}

0 comments on commit b59fe18

Please sign in to comment.