It2Me native messaging: GYP and source refactoring

remoting/remoting.gyp

 # Copyright (c) 2012 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.
 
 {
   'variables': {
     'chromium_code': 1,
 
     'variables': {
       'conditions': [
@@ skipping 511 matching lines @@
                   # /MP conflicts with #import directive so we limit the number
                   # of processes to spawn to 1.
                   'AdditionalOptions': ['/MP1'],
                 },
               },
             }],
           ],
         },  # end of target 'remoting_host'
 
         {
+          'target_name': 'remoting_native_messaging_base',
+          'type': 'static_library',
+          'variables': { 'enable_wexit_time_destructors': 1, },
+          'dependencies': [
+            '../base/base.gyp:base',
+          ],
+          'sources': [
+            'host/native_messaging/native_messaging_channel.cc',
+            'host/native_messaging/native_messaging_channel.h',
+            'host/native_messaging/native_messaging_reader.cc',
+            'host/native_messaging/native_messaging_reader.h',
+            'host/native_messaging/native_messaging_writer.cc',
+            'host/native_messaging/native_messaging_writer.h',
+          ],
+        },  # end of target 'remoting_native_messaging_base'
+
+        {
           'target_name': 'remoting_me2me_host_static',
           'type': 'static_library',
           'variables': { 'enable_wexit_time_destructors': 1, },
           'dependencies': [
             '../base/base.gyp:base',
             '../base/base.gyp:base_i18n',
             '../net/net.gyp:net',
             '../third_party/webrtc/modules/modules.gyp:desktop_capture',
             'remoting_base',
             'remoting_breakpad',
@@ skipping 40 matching lines @@
             'host/setup/daemon_controller_delegate_linux.cc',
             'host/setup/daemon_controller_delegate_linux.h',
             'host/setup/daemon_controller_delegate_mac.h',
             'host/setup/daemon_controller_delegate_mac.mm',
             'host/setup/daemon_controller_delegate_win.cc',
             'host/setup/daemon_controller_delegate_win.h',
             'host/setup/daemon_installer_win.cc',
             'host/setup/daemon_installer_win.h',
             'host/setup/host_starter.cc',
             'host/setup/host_starter.h',
-            'host/setup/native_messaging_channel.cc',
-            'host/setup/native_messaging_channel.h',
-            'host/setup/native_messaging_host.cc',
-            'host/setup/native_messaging_host.h',
-            'host/setup/native_messaging_reader.cc',
-            'host/setup/native_messaging_reader.h',
-            'host/setup/native_messaging_writer.cc',
-            'host/setup/native_messaging_writer.h',
             'host/setup/oauth_client.cc',
             'host/setup/oauth_client.h',
             'host/setup/oauth_helper.cc',
             'host/setup/oauth_helper.h',
             'host/setup/pin_validator.cc',
             'host/setup/pin_validator.h',
             'host/setup/service_client.cc',
             'host/setup/service_client.h',
             'host/setup/test_util.cc',
             'host/setup/test_util.h',
             'host/setup/win/auth_code_getter.cc',
             'host/setup/win/auth_code_getter.h',
           ],
           'conditions': [
             ['OS=="win"', {
               'dependencies': [
                 '../google_update/google_update.gyp:google_update',
                 'remoting_lib_idl',
               ],
               # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
               'msvs_disabled_warnings': [4267, ],
             }],
           ],
         },  # end of target 'remoting_host_setup_base'
 
         {
+          'target_name': 'remoting_me2me_native_messaging_host_static',

[Sergey Ulanov, 2013/10/30 22:51:01]

Do we really need this target? I think you can just include
me2me_native_messaging_host.* in remoting_host_setup_base and trust the linker
to strip that code from binaries that don't use it.

[weitao, 2013/10/31 17:59:34]

I think this makes a cleaner componentization. setup_base includes all the
DaemonController functionality that is shared between the NPAPI plugin and the
native messaging host.

One good example is Jamie's fix for the Mac issue (293036). Since
native_messaging_host.cc is included in the setup_base target almost all
remoting targets need to be rebuilt. Now that it is moved
me2me_native_messaging_host_static, only the targets that include native
messaging (me2me_native_messaging_host and remoting_core) need to be rebuilt.

Initially I didn't have this me2me_native_messaging_host_static target and
included me2me_native_messaging_host.* directly in
remoting_me2me_native_messaging_host. But that broke the Windows build which
combines the me2me host and the NM host in one binary. So I created this static
target to accommodate that.

WDYT?

On 2013/10/30 22:51:01, Sergey Ulanov wrote:

[Sergey Ulanov, 2013/10/31 19:20:42]

Problem is that each additional target adds complexity in this file making it
harder for humans to read and edit it. It's already freaking 3000 lines long!

On 2013/10/31 17:59:34, weitaosu wrote:
Having multiple targets may make build faster in some cases, but it makes it
slower in all other cases, so it's not clear if it is a win overall. Each
additional target requires separate library file to be generated. It also means
that GYP will take longer to process this file (e.g. a separate .ninja file is
generated for each build target).
Right, that's why I think it should be part of remoting_host_setup_base.

[weitao, 2013/10/31 20:59:16]

Done.

+          'type': 'static_library',
+          'variables': { 'enable_wexit_time_destructors': 1, },
+          'dependencies': [
+            '../base/base.gyp:base',
+            'remoting_host',
+            'remoting_host_logging',
+            'remoting_native_messaging_base',
+            'remoting_host_setup_base',
+          ],
+          'defines': [
+            'VERSION=<(version_full)',
+          ],
+          'sources': [
+            'host/setup/me2me_native_messaging_host.cc',
+            'host/setup/me2me_native_messaging_host.h',
+          ],
+        },  # end of target 'remoting_me2me_native_messaging_host_static'
+
+        {
           'target_name': 'remoting_host_plugin',
           'type': 'loadable_module',
           'variables': { 'enable_wexit_time_destructors': 1, },
           'product_extension': '<(host_plugin_extension)',
           'product_prefix': '<(host_plugin_prefix)',
           'defines': [
             'HOST_PLUGIN_MIME_TYPE=<(host_plugin_mime_type)',
           ],
           'dependencies': [
             '../base/base.gyp:base_i18n',
@@ skipping 105 matching lines @@
             'remoting_base',
             'remoting_host',
             'remoting_host_event_logger',
             'remoting_host_logging',
             'remoting_infoplist_strings',
             'remoting_host_setup_base',
             'remoting_jingle_glue',
             'remoting_resources',
           ],
           'sources': [
-            'host/it2me/it2me_impl.cc',
-            'host/it2me/it2me_impl.h',
+            'host/it2me/it2me_host.cc',
+            'host/it2me/it2me_host.h',
           ],
         },  # end of target 'remoting_it2me_host_static'
         {
           'target_name': 'remoting_infoplist_strings',
           'type': 'none',
           'dependencies': [
             'remoting_resources',
           ],
           'actions': [
             {
@@ skipping 129 matching lines @@
                   'action': [ 'zip', '-j', '-0', '<@(_outputs)', '<@(_inputs)' ],
                 },
               ],
             },
             {
               'target_name': 'remoting_me2me_host_deb_installer',
               'type': 'none',
               'dependencies': [
                 'remoting_me2me_host',
                 'remoting_start_host',
-                'remoting_native_messaging_host',
+                'remoting_me2me_native_messaging_host',
+                'remoting_me2me_native_messaging_manifest',
               ],
               'actions': [
                 {
                   'action_name': 'build_debian_package',
                   'inputs': [
                     '<(deb_cmd)',
                     'host/installer/linux/Makefile',
                     'host/installer/linux/debian/chrome-remote-desktop.init',
                     'host/installer/linux/debian/chrome-remote-desktop.pam',
                     'host/installer/linux/debian/compat',
@@ skipping 101 matching lines @@
               ],  # conditions
             }],  # OS=mac
             ['OS=="linux" and linux_use_tcmalloc==1', {
               'dependencies': [
                 '../base/allocator/allocator.gyp:allocator',
               ],
             }],  # OS=linux
           ],  # end of 'conditions'
         },  # end of target 'remoting_me2me_host'
         {
-          'target_name': 'remoting_native_messaging_host',
+          'target_name': 'remoting_me2me_native_messaging_host',
           'type': 'executable',
           'variables': { 'enable_wexit_time_destructors': 1, },
           'dependencies': [
-            '../base/base.gyp:base',
-            'remoting_host',
-            'remoting_host_logging',
-            'remoting_host_setup_base',
-            'remoting_native_messaging_manifest',
-          ],
-          'defines': [
-            'VERSION=<(version_full)',
+            'remoting_me2me_native_messaging_host_static',
           ],
           'sources': [
-            'host/setup/native_messaging_host_main.cc',
+            'host/setup/me2me_native_messaging_host_main.cc',
           ],
           'conditions': [
             ['OS=="linux" and linux_use_tcmalloc==1', {
               'dependencies': [
                 '../base/allocator/allocator.gyp:allocator',
               ],
             }],
           ],
-        },  # end of target 'remoting_native_messaging_host'
+        },  # end of target 'remoting_me2me_native_messaging_host'
       ],  # end of 'targets'
     }],  # 'OS!="win" and enable_remoting_host==1'
 
 
     ['OS=="linux" and chromeos==0 and enable_remoting_host==1', {
       'targets': [
         # Linux breakpad processing
         {
           'target_name': 'remoting_linux_symbols',
           'type': 'none',
@@ skipping 93 matching lines @@
         # This packages up the files needed for the remoting host installer so
         # they can be sent off to be signed.
         # We don't build an installer here because we don't have signed binaries.
         {
           'target_name': 'remoting_me2me_host_archive',
           'type': 'none',
           'dependencies': [
             'remoting_host_prefpane',
             'remoting_host_uninstaller',
             'remoting_me2me_host',
-            'remoting_native_messaging_host',
+            'remoting_me2me_native_messaging_host',
+            'remoting_me2me_native_messaging_manifest',
           ],
           'variables': {
             'host_name': '<!(python <(version_py_path) -f <(branding_path) -t "@HOST_PLUGIN_FILE_NAME@")',
             'host_service_name': '<!(python <(version_py_path) -f <(branding_path) -t "@DAEMON_FILE_NAME@")',
             'host_uninstaller_name': '<!(python <(version_py_path) -f <(branding_path) -t "@MAC_UNINSTALLER_NAME@")',
             'bundle_prefix': '<!(python <(version_py_path) -f <(branding_path) -t "@MAC_UNINSTALLER_BUNDLE_PREFIX@")',
           },
           'actions': [
             {
               'action_name': 'Zip installer files for signing',
               'temp_dir': '<(SHARED_INTERMEDIATE_DIR)/remoting/remoting-me2me-host',
               'zip_path': '<(PRODUCT_DIR)/remoting-me2me-host-<(OS).zip',
               'variables': {
                 'host_name_nospace': '<!(echo <(host_name) | sed "s/ //g")',
                 'host_service_name_nospace': '<!(echo <(host_service_name) | sed "s/ //g")',
                 'host_uninstaller_name_nospace': '<!(echo <(host_uninstaller_name) | sed "s/ //g")',
               },
               'generated_files': [
                 '<(PRODUCT_DIR)/remoting_host_prefpane.prefPane',
                 '<(PRODUCT_DIR)/remoting_me2me_host.app',
                 '<(PRODUCT_DIR)/remoting_host_uninstaller.app',
                 '<(PRODUCT_DIR)/remoting_native_messaging_host',
                 '<(PRODUCT_DIR)/remoting/com.google.chrome.remote_desktop.json',
               ],
               'generated_files_dst': [
                 'PreferencePanes/org.chromium.chromoting.prefPane',
                 'PrivilegedHelperTools/org.chromium.chromoting.me2me_host.app',
                 'Applications/<(host_uninstaller_name).app',
                 'PrivilegedHelperTools/org.chromium.chromoting.me2me_host.app/Contents/MacOS/native_messaging_host',
-                'Config/com.google.chrome.remote_desktop.json',
+                'Config/com.google.chrome.remote_desktop.json',
               ],
               'source_files': [
                 '<@(remoting_host_installer_mac_files)',
               ],
               'defs': [
                 'VERSION=<(version_full)',
                 'VERSION_SHORT=<(version_short)',
                 'VERSION_MAJOR=<(version_major)',
                 'VERSION_MINOR=<(version_minor)',
                 'HOST_NAME=<(host_name)',
@@ skipping 327 matching lines @@
             'remoting_breakpad',
             'remoting_core_resources',
             'remoting_host',
             'remoting_host_event_logger',
             'remoting_host_logging',
             'remoting_host_setup_base',
             'remoting_lib_idl',
             'remoting_lib_ps',
             'remoting_lib_rc',
             'remoting_me2me_host_static',
+            'remoting_me2me_native_messaging_host_static',
+            'remoting_native_messaging_base',
             'remoting_protocol',
             'remoting_version_resources',
           ],
           'sources': [
             '<(SHARED_INTERMEDIATE_DIR)/remoting/core.rc',
             '<(SHARED_INTERMEDIATE_DIR)/remoting/host/chromoting_lib.rc',
             '<(SHARED_INTERMEDIATE_DIR)/remoting/host/remoting_host_messages.rc',
             '<(SHARED_INTERMEDIATE_DIR)/remoting/version.rc',
             'host/chromoting_messages.cc',
             'host/chromoting_messages.h',
@@ skipping 14 matching lines @@
             'host/host_exit_codes.h',
             'host/host_exit_codes.cc',
             'host/host_export.h',
             'host/host_main.cc',
             'host/host_main.h',
             'host/ipc_constants.cc',
             'host/ipc_constants.h',
             'host/remoting_me2me_host.cc',
             'host/sas_injector.h',
             'host/sas_injector_win.cc',
+            'host/setup/me2me_native_messaging_host_main.cc',
             'host/verify_config_window_win.cc',
             'host/verify_config_window_win.h',
             'host/win/chromoting_module.cc',
             'host/win/chromoting_module.h',
             'host/win/core.cc',
             'host/win/core_resource.h',
             'host/win/elevated_controller.cc',
             'host/win/elevated_controller.h',
             'host/win/host_service.cc',
             'host/win/host_service.h',
@@ skipping 359 matching lines @@
           ],
         },  # end of target 'remoting_host_installation'
 
         {
           'target_name': 'remoting_me2me_host_archive',
           'type': 'none',
           'dependencies': [
             'remoting_core',
             'remoting_desktop',
             'remoting_host_exe',
-            'remoting_native_messaging_manifest',
+            'remoting_me2me_native_messaging_manifest',
           ],
           'compiled_inputs': [
             '<(PRODUCT_DIR)/remoting_core.dll',
             '<(PRODUCT_DIR)/remoting_desktop.exe',
             '<(PRODUCT_DIR)/remoting_host.exe',
           ],
           'compiled_inputs_dst': [
             'files/remoting_core.dll',
             'files/remoting_desktop.exe',
             'files/remoting_host.exe',
@@ skipping 336 matching lines @@
                 '--patches',
                 '<@(remoting_webapp_patch_files)',
               ],
               'msvs_cygwin_shell': 0,
             },
           ],
         }],
       ],
     }, # end of target 'remoting_webapp'
 
-    # Generates 'native_messaging_manifest.json' to be included in the
+    # Generates 'me2me_native_messaging_manifest.json' to be included in the
     # installation.
     {
-      'target_name': 'remoting_native_messaging_manifest',
+      'target_name': 'remoting_me2me_native_messaging_manifest',
       'type': 'none',
       'dependencies': [
         'remoting_resources',
       ],
       'variables': {
-        'input': 'host/setup/native_messaging_manifest.json',
+        'input': 'host/setup/me2me_native_messaging_manifest.json',
         'output': '<(PRODUCT_DIR)/remoting/com.google.chrome.remote_desktop.json',
       },
       'target_conditions': [
         ['OS == "win" or OS == "mac" or OS == "linux"', {
           'conditions': [
             [ 'OS == "win"', {
               'variables': {
-                'native_messaging_host_path': 'remoting_host.exe',
+                'me2me_native_messaging_host_path': 'remoting_host.exe',
               },
             }], [ 'OS == "mac"', {
               'variables': {
-                'native_messaging_host_path': '/Library/PrivilegedHelperTools/org.chromium.chromoting.me2me_host.app/Contents/MacOS/native_messaging_host',
+                'me2me_native_messaging_host_path': '/Library/PrivilegedHelperTools/org.chromium.chromoting.me2me_host.app/Contents/MacOS/native_messaging_host',
               },
             }], ['OS == "linux"', {
               'variables': {
-                'native_messaging_host_path': '/opt/google/chrome-remote-desktop/native-messaging-host',
+                'me2me_native_messaging_host_path': '/opt/google/chrome-remote-desktop/native-messaging-host',
               },
             }], ['OS != "linux" and OS != "mac" and OS != "win"', {
               'variables': {
-                'native_messaging_host_path': '/opt/google/chrome-remote-desktop/native-messaging-host',
+                'me2me_native_messaging_host_path': '/opt/google/chrome-remote-desktop/native-messaging-host',
               },
             }],
           ],  # conditions
           'actions': [
             {
               'action_name': 'generate_manifest',
               'inputs': [
                 '<(remoting_localize_path)',
                 '<(input)',
               ],
               'outputs': [
                 '<(output)',
               ],
               'action': [
                 'python',
                 '<(remoting_localize_path)',
-                '--define', 'NATIVE_MESSAGING_HOST_PATH=<(native_messaging_host_path)',
+                '--define', 'ME2ME_NATIVE_MESSAGING_HOST_PATH=<(me2me_native_messaging_host_path)',
                 '--locale_dir', '<(webapp_locale_dir)',
                 '--template', '<(input)',
                 '--locale_output',
                 '<(output)',
                 '--encoding', 'utf-8',
                 'en',
               ],
             },
           ],  # actions
         },
        ],
       ],  # target_conditions
-    },  # end of target 'remoting_native_messaging_manifest'
+    },  # end of target 'remoting_me2me_native_messaging_manifest'
     {
       'target_name': 'remoting_resources',
       'type': 'none',
       'variables': {
         'grit_out_dir': '<(SHARED_INTERMEDIATE_DIR)',
         'grit_resource_ids': 'resources/resource_ids',
         'sources': [
           'base/resources_unittest.cc',
           'host/continue_window_mac.mm',
           'host/disconnect_window_mac.mm',
@@ skipping 386 matching lines @@
         '../ui/gfx/gfx.gyp:gfx',
         '../ui/ui.gyp:ui',
         'remoting_base',
         'remoting_breakpad',
         'remoting_client',
         'remoting_client_plugin',
         'remoting_host',
         'remoting_host_event_logger',
         'remoting_host_setup_base',
         'remoting_jingle_glue',
+        'remoting_me2me_native_messaging_host_static',
+        'remoting_native_messaging_base',
         'remoting_protocol',
         'remoting_resources',
         '../third_party/webrtc/modules/modules.gyp:desktop_capture',
       ],
       'defines': [
         'VERSION=<(version_full)',
       ],
       'include_dirs': [
         '../testing/gmock/include',
       ],
@@ skipping 44 matching lines @@
         'host/host_status_sender_unittest.cc',
         'host/host_change_notification_listener_unittest.cc',
         'host/host_mock_objects.cc',
         'host/host_mock_objects.h',
         'host/host_status_monitor_fake.h',
         'host/ipc_desktop_environment_unittest.cc',
         'host/json_host_config_unittest.cc',
         'host/linux/x_server_clipboard_unittest.cc',
         'host/local_input_monitor_unittest.cc',
         'host/log_to_server_unittest.cc',
+        'host/native_messaging/native_messaging_reader_unittest.cc',
+        'host/native_messaging/native_messaging_writer_unittest.cc',
         'host/pairing_registry_delegate_linux_unittest.cc',
         'host/pairing_registry_delegate_win_unittest.cc',
         'host/pin_hash_unittest.cc',
         'host/policy_hack/fake_policy_watcher.cc',
         'host/policy_hack/fake_policy_watcher.h',
         'host/policy_hack/mock_policy_callback.cc',
         'host/policy_hack/mock_policy_callback.h',
         'host/policy_hack/policy_watcher_unittest.cc',
         'host/register_support_host_request_unittest.cc',
         'host/remote_input_filter_unittest.cc',
         'host/resizing_host_observer_unittest.cc',
         'host/screen_capturer_fake.cc',
         'host/screen_capturer_fake.h',
         'host/screen_resolution_unittest.cc',
         'host/server_log_entry_unittest.cc',
-        'host/setup/native_messaging_host_unittest.cc',
-        'host/setup/native_messaging_reader_unittest.cc',
-        'host/setup/native_messaging_writer_unittest.cc',
+        'host/setup/me2me_native_messaging_host_unittest.cc',
         'host/setup/oauth_helper_unittest.cc',
         'host/setup/pin_validator_unittest.cc',
         'host/token_validator_factory_impl_unittest.cc',
         'host/video_scheduler_unittest.cc',
         'host/win/rdp_client_unittest.cc',
         'host/win/worker_process_launcher.cc',
         'host/win/worker_process_launcher.h',
         'host/win/worker_process_launcher_unittest.cc',
         'jingle_glue/chromium_socket_factory_unittest.cc',
         'jingle_glue/fake_signal_strategy.cc',
@@ skipping 82 matching lines @@
         }],
         [ '(OS!="linux" or chromeos==0)', {
           'sources!': [
             'client/plugin/normalizing_input_filter_cros_unittest.cc',
           ],
         }],
         ['enable_remoting_host == 0', {
           'dependencies!': [
             'remoting_host',
             'remoting_host_setup_base',
+            'remoting_me2me_native_messaging_host_static',
+            'remoting_native_messaging_base',
           ],
           'sources/': [
             ['exclude', 'codec/*'],
             ['exclude', 'host/*'],
           ]
         }],
         ['toolkit_uses_gtk == 1', {
           'dependencies': [
             # Needed for the following #include chain:
             #   base/run_all_unittests.cc
             #   ../base/test_suite.h
             #   gtk/gtk.h
             '../build/linux/system.gyp:gtk',
             '../build/linux/system.gyp:ssl',
           ],
           'conditions': [
             [ 'linux_use_tcmalloc==1', {
                 'dependencies': [
                   '../base/allocator/allocator.gyp:allocator',
                 ],
               },
             ],
           ],
         }],  # end of 'toolkit_uses_gtk == 1'
       ],  # end of 'conditions'
     },  # end of target 'remoting_unittests'
   ],  # end of targets
 }