[Sync] Make the 'sync' target the one clients should use

sync/sync.gyp

Index: sync/sync.gyp
diff --git a/sync/sync.gyp b/sync/sync.gyp
index 40f5817b19e42b31542a49c6af131579bc6ed21a..01f8352c1329d214babb574fff8e6c473a5c10c7 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -2,21 +2,35 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+########################################################################
+# If your target depends on the sync library, all you need to include
+# is:
+#
+#   'dependencies': [
+#      '../sync/sync.gyp:sync',
+#    ],
+#    'export_dependent_settings': [
+#      '../sync/sync.gyp:sync',
+#    ],

[Ryan Sleevi, 2012/07/20 23:24:49]

Can you explain why it's necessary to export the settings?

Targets should be able to depend just fine. If it's due to codegen/protobuf'ing,
then sync should export the dependency on the protobuf, which will make syncs
dependents coalesce on the protobuf step, rather than around sync.

export_dependent_settings is only necessary if 'your target' is including sync
headers as part of it's interface. Which is the same as any other GYP file.

I'd drop this whole comment block

+#
+# (If your target is an executable, you can omit the
+# 'export_dependent_settings' block.)
+#
+########################################################################
+
 {
   'variables': {
     'chromium_code': 1,
   },
   'targets': [
+
+    ####################################################################
+    # 'sync' and its dependencies
+    ####################################################################
+
     # The core sync library.
-    #
-    # TODO(akalin): Rename this to something like 'sync_core' and
-    # reserve the 'sync' name for the overarching library that clients
-    # should depend on.
     {
-      'target_name': 'sync',
-      # TODO(akalin): Change this to '<(component)'.  When we rename
-      # this to 'sync_core' and make the overarching 'sync' library,
-      # make that one '<(component)'.
+      'target_name': 'sync_core',
       'type': 'static_library',
       'variables': { 'enable_wexit_time_destructors': 1, },
       'include_dirs': [
@@ -40,36 +54,6 @@
       ],
       'sources': [
         'base/sync_export.h',
-        'internal_api/public/base/enum_set.h',
-        'internal_api/public/base/model_type.h',
-        'internal_api/public/base/model_type_payload_map.cc',
-        'internal_api/public/base/model_type_payload_map.h',
-        'internal_api/public/engine/model_safe_worker.cc',
-        'internal_api/public/engine/model_safe_worker.h',
-        'internal_api/public/engine/passive_model_worker.cc',
-        'internal_api/public/engine/passive_model_worker.h',
-        'internal_api/public/engine/polling_constants.cc',
-        'internal_api/public/engine/polling_constants.h',
-        'internal_api/public/engine/sync_status.cc',
-        'internal_api/public/engine/sync_status.h',
-        'internal_api/public/sessions/model_neutral_state.cc',
-        'internal_api/public/sessions/model_neutral_state.h',
-        'internal_api/public/sessions/sync_session_snapshot.cc',
-        'internal_api/public/sessions/sync_session_snapshot.h',
-        'internal_api/public/sessions/sync_source_info.cc',
-        'internal_api/public/sessions/sync_source_info.h',
-        'internal_api/public/util/experiments.h',
-        'internal_api/public/util/immutable.h',
-        'internal_api/public/util/syncer_error.cc',
-        'internal_api/public/util/syncer_error.h',
-        'internal_api/public/util/sync_string_conversions.cc',
-        'internal_api/public/util/sync_string_conversions.h',
-        'internal_api/public/util/report_unrecoverable_error_function.h',
-        'internal_api/public/util/unrecoverable_error_handler.h',
-        'internal_api/public/util/unrecoverable_error_info.h',
-        'internal_api/public/util/unrecoverable_error_info.cc',
-        'internal_api/public/util/weak_handle.cc',
-        'internal_api/public/util/weak_handle.h',
         'engine/all_status.cc',
         'engine/all_status.h',
         'engine/apply_updates_command.cc',
@@ -240,7 +224,7 @@
         # TODO(akalin): Remove this (http://crbug.com/133352).
         '../third_party/cacheinvalidation/cacheinvalidation.gyp:cacheinvalidation_proto_cpp',
         '../third_party/libjingle/libjingle.gyp:libjingle',
-        'sync',
+        'sync_core',
       ],
       'export_dependent_settings': [
         '../jingle/jingle.gyp:notifier',
@@ -282,7 +266,7 @@
 
     # The sync internal API library.
     {
-      'target_name': 'syncapi_core',
+      'target_name': 'sync_internal_api',
       'type': 'static_library',
       'variables': { 'enable_wexit_time_destructors': 1, },
       'include_dirs': [
@@ -294,19 +278,31 @@
         '../net/net.gyp:net',
         'protocol/sync_proto.gyp:sync_proto',
         'sync_notifier',
-        'sync',
+        'sync_core',
       ],
       'export_dependent_settings': [
         # Propagate sync_proto since our headers include its generated
         # files.
         'protocol/sync_proto.gyp:sync_proto',

[Ryan Sleevi, 2012/07/20 23:24:49]

Are you directly using/building these protobufs in your target?

It would seem like, as you structured, it should only be depending on sync_core
and only exporting sync_core (eg: drop line 279)

If you /are/ directly using the .pb.h, then I suppose it makes sense.

-        'sync',
+        'sync_core',
       ],
       'sources': [
+        'internal_api/public/base/enum_set.h',
+        'internal_api/public/base/model_type.h',
+        'internal_api/public/base/model_type_payload_map.cc',
+        'internal_api/public/base/model_type_payload_map.h',
         'internal_api/public/base_node.h',
         'internal_api/public/base_transaction.h',
         'internal_api/public/change_record.h',
         'internal_api/public/configure_reason.h',
+        'internal_api/public/engine/model_safe_worker.cc',
+        'internal_api/public/engine/model_safe_worker.h',
+        'internal_api/public/engine/passive_model_worker.cc',
+        'internal_api/public/engine/passive_model_worker.h',
+        'internal_api/public/engine/polling_constants.cc',
+        'internal_api/public/engine/polling_constants.h',
+        'internal_api/public/engine/sync_status.cc',
+        'internal_api/public/engine/sync_status.h',
         'internal_api/public/http_bridge.h',
         'internal_api/public/http_post_provider_factory.h',
         'internal_api/public/http_post_provider_interface.h',
@@ -314,10 +310,28 @@
         'internal_api/public/internal_components_factory_impl.h',
         'internal_api/public/read_node.h',
         'internal_api/public/read_transaction.h',
+        'internal_api/public/sessions/model_neutral_state.cc',
+        'internal_api/public/sessions/model_neutral_state.h',
+        'internal_api/public/sessions/sync_session_snapshot.cc',
+        'internal_api/public/sessions/sync_session_snapshot.h',
+        'internal_api/public/sessions/sync_source_info.cc',
+        'internal_api/public/sessions/sync_source_info.h',
         'internal_api/public/sync_manager.h',
         'internal_api/public/sync_manager.cc',
         'internal_api/public/sync_manager_factory.h',
         'internal_api/public/user_share.h',
+        'internal_api/public/util/experiments.h',
+        'internal_api/public/util/immutable.h',
+        'internal_api/public/util/syncer_error.cc',
+        'internal_api/public/util/syncer_error.h',
+        'internal_api/public/util/sync_string_conversions.cc',
+        'internal_api/public/util/sync_string_conversions.h',
+        'internal_api/public/util/report_unrecoverable_error_function.h',
+        'internal_api/public/util/unrecoverable_error_handler.h',
+        'internal_api/public/util/unrecoverable_error_info.h',
+        'internal_api/public/util/unrecoverable_error_info.cc',
+        'internal_api/public/util/weak_handle.cc',
+        'internal_api/public/util/weak_handle.h',
         'internal_api/public/write_node.h',
         'internal_api/public/write_transaction.h',
         'internal_api/base_node.cc',
@@ -350,7 +364,7 @@
 
     # The sync external API library.
     {
-      'target_name': 'syncapi_service',
+      'target_name': 'sync_api',
       'type': 'static_library',
       'variables': { 'enable_wexit_time_destructors': 1, },
       'include_dirs': [
@@ -359,7 +373,7 @@
       'dependencies': [
         '../base/base.gyp:base',
         'protocol/sync_proto.gyp:sync_proto',
-        'sync',
+        'sync_core',
       ],
       # We avoid including header files from sync_proto in our public
       # header files so we don't need to export its settings.
@@ -379,9 +393,49 @@
       ],
     },
 
-    # Test support files for the 'sync' target.
+    # The componentized sync library.
     {
-      'target_name': 'test_support_sync',
+      'target_name': 'sync_component',
+      # TODO(akalin): Change this to '<(component)'.
+      'type': 'static_library',
+      'variables': { 'enable_wexit_time_destructors': 1, },
+      'dependencies': [
+        'sync_core',
+        'sync_notifier',
+        'sync_internal_api',
+        'sync_api',
+      ],
+      'export_dependent_settings': [
+        'sync_core',
+        'sync_notifier',
+        'sync_internal_api',
+        'sync_api',
+      ],
+    },
+
+    # The public sync target.  This depends on 'sync_component' and
+    # 'sync_proto' separately since 'sync_proto' isn't exportable from
+    # 'sync_component' (for now).
+    {
+      'target_name': 'sync',
+      'type': 'none',
+      'dependencies': [
+        'sync_component',
+        'protocol/sync_proto.gyp:sync_proto',
+      ],
+      'export_dependent_settings': [
+        'sync_component',
+        'protocol/sync_proto.gyp:sync_proto',
+      ],
+    },
+
+    ####################################################################
+    # Test support targets
+    ####################################################################
+
+    # Test support files for the 'sync_core' target.
+    {
+      'target_name': 'test_support_sync_core',
       'type': 'static_library',
       'variables': { 'enable_wexit_time_destructors': 1, },
       'include_dirs': [
@@ -391,13 +445,11 @@
         '../base/base.gyp:base',
         '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
       ],
       'export_dependent_settings': [
         '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
       ],
       'sources': [
@@ -449,11 +501,11 @@
       ],
       'dependencies': [
         '../testing/gmock.gyp:gmock',
-        'sync_notifier',
+        'sync',
       ],
       'export_dependent_settings': [
         '../testing/gmock.gyp:gmock',
-        'sync_notifier',
+        'sync',
       ],
       'sources': [
         'notifier/mock_invalidation_state_tracker.cc',
@@ -463,9 +515,9 @@
       ],
     },
 
-    # Test support files for the 'syncapi_core' target.
+    # Test support files for the 'sync_internal_api' target.
     {
-      'target_name': 'test_support_syncapi_core',
+      'target_name': 'test_support_sync_internal_api',
       'type': 'static_library',
       'variables': { 'enable_wexit_time_destructors': 1, },
       'include_dirs': [
@@ -474,13 +526,13 @@
       'dependencies': [
         '../base/base.gyp:base',
         '../testing/gtest.gyp:gtest',
-        'syncapi_core',
-        'test_support_sync',
+        'sync',
+        'test_support_sync_core',
       ],
       'export_dependent_settings': [
         '../testing/gtest.gyp:gtest',
-        'syncapi_core',
-        'test_support_sync',
+        'sync',
+        'test_support_sync_core',
       ],
       'sources': [
         'internal_api/public/test/fake_sync_manager.h',
@@ -494,20 +546,20 @@
       ],
     },
 
-    # Test support files for the 'syncapi_service' target.
+    # Test support files for the 'sync_api' target.
     {
-      'target_name': 'test_support_syncapi_service',
+      'target_name': 'test_support_sync_api',
       'type': 'static_library',
       'include_dirs': [
         '..',
       ],
       'dependencies': [
         '../testing/gmock.gyp:gmock',
-        'syncapi_service',
+        'sync',
       ],
       'export_dependent_settings': [
         '../testing/gmock.gyp:gmock',
-        'syncapi_service',
+        'sync',
       ],
       'sources': [
         'api/fake_syncable_service.cc',
@@ -517,12 +569,16 @@
       ],
     },
 
-    # Unit tests for the 'sync' target.  This cannot be a static
+    ####################################################################
+    # 'sync_unit_tests' and its dependencies
+    ####################################################################
+
+    # Unit tests for the 'sync_core' target.  This cannot be a static
     # library because the unit test files have to be compiled directly
     # into the executable, so we push the target files to the
     # depending executable target via direct_dependent_settings.
     {
-      'target_name': 'sync_tests',
+      'target_name': 'sync_core_tests',
       'type': 'none',
       # We only want unit test executables to include this target.
       'suppress_wildcard': 1,
@@ -530,9 +586,8 @@
         '../base/base.gyp:base',
         '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
-        'test_support_sync',
+        'test_support_sync_core',
       ],
       # Propagate all dependencies since the actual compilation
       # happens in the dependents.
@@ -540,9 +595,8 @@
         '../base/base.gyp:base',
         '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
-        'test_support_sync',
+        'test_support_sync_core',
       ],
       'direct_dependent_settings': {
         'variables': { 'enable_wexit_time_destructors': 1, },
@@ -613,7 +667,6 @@
         '../third_party/cacheinvalidation/cacheinvalidation.gyp:cacheinvalidation',
         '../third_party/libjingle/libjingle.gyp:libjingle',
         'sync',
-        'sync_notifier',
         'test_support_sync_notifier',
       ],
       # Propagate all dependencies since the actual compilation
@@ -627,7 +680,6 @@
         '../third_party/cacheinvalidation/cacheinvalidation.gyp:cacheinvalidation',
         '../third_party/libjingle/libjingle.gyp:libjingle',
         'sync',
-        'sync_notifier',
         'test_support_sync_notifier',
       ],
       'direct_dependent_settings': {
@@ -654,12 +706,12 @@
       },
     },
 
-    # Unit tests for the 'syncapi_core' target.  This cannot be a static
+    # Unit tests for the 'sync_internal_api' target.  This cannot be a static
     # library because the unit test files have to be compiled directly
     # into the executable, so we push the target files to the
     # depending executable target via direct_dependent_settings.
     {
-      'target_name': 'syncapi_core_tests',
+      'target_name': 'sync_internal_api_tests',
       'type': 'none',
       # We only want unit test executables to include this target.
       'suppress_wildcard': 1,
@@ -669,11 +721,8 @@
         '../net/net.gyp:net_test_support',
         '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
-        'sync_notifier',
-        'syncapi_core',
-        'test_support_syncapi_core',
+        'test_support_sync_internal_api',
       ],
       # Propagate all dependencies since the actual compilation
       # happens in the dependents.
@@ -683,11 +732,8 @@
         '../net/net.gyp:net_test_support',
         '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
-        'sync_notifier',
-        'syncapi_core',
-        'test_support_syncapi_core',
+        'test_support_sync_internal_api',
       ],
       'direct_dependent_settings': {
         'variables': { 'enable_wexit_time_destructors': 1, },
@@ -706,32 +752,28 @@
       },
     },
 
-    # Unit tests for the 'syncapi_service' target.  This cannot be a static
+    # Unit tests for the 'sync_api' target.  This cannot be a static
     # library because the unit test files have to be compiled directly
     # into the executable, so we push the target files to the
     # depending executable target via direct_dependent_settings.
     {
-      'target_name': 'syncapi_service_tests',
+      'target_name': 'sync_api_tests',
       'type': 'none',
       # We only want unit test executables to include this target.
       'suppress_wildcard': 1,
       'dependencies': [
         '../base/base.gyp:base',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
-        'syncapi_service',
-        'test_support_syncapi_service',
+        'test_support_sync_api',
       ],
       # Propagate all dependencies since the actual compilation
       # happens in the dependents.
       'export_dependent_settings': [
         '../base/base.gyp:base',
         '../testing/gtest.gyp:gtest',
-        'protocol/sync_proto.gyp:sync_proto',
         'sync',
-        'syncapi_service',
-        'test_support_syncapi_service',
+        'test_support_sync_api',
       ],
       'direct_dependent_settings': {
         'variables': { 'enable_wexit_time_destructors': 1, },
@@ -751,10 +793,10 @@
       'type': '<(gtest_target_type)',
       'dependencies': [
         '../base/base.gyp:run_all_unittests',
-        'sync_tests',
+        'sync_core_tests',
         'sync_notifier_tests',
-        'syncapi_core_tests',
-        'syncapi_service_tests',
+        'sync_internal_api_tests',
+        'sync_api_tests',
       ],
       # TODO(akalin): This is needed because histogram.cc uses
       # leak_annotations.h, which pulls this in.  Make 'base'
@@ -780,6 +822,10 @@
       ],
     },
 
+    ####################################################################
+    # Helper executables
+    ####################################################################
+
     # A tool to listen to sync notifications and print them out.
     {
       'target_name': 'sync_listen_notifications',
@@ -790,7 +836,6 @@
         '../net/net.gyp:net',
         '../net/net.gyp:net_test_support',
         'sync',
-        'sync_notifier',
       ],
       'sources': [
         'tools/sync_listen_notifications.cc',
@@ -801,23 +846,19 @@
     {
       'target_name': 'sync_client',
       'type': 'executable',
-      'defines': [
-        'SYNC_TEST',
-      ],
       'dependencies': [
         '../base/base.gyp:base',
         '../jingle/jingle.gyp:notifier',
         '../net/net.gyp:net',
         '../net/net.gyp:net_test_support',
         'sync',
-        'sync_notifier',
-        'syncapi_core',
       ],
       'sources': [
         'tools/sync_client.cc',
       ],
     },
   ],
+
   'conditions': [
     # Special target to wrap a gtest_target_type==shared_library
     # sync_unit_tests into an android apk for execution.