Bringing up the net target on iOS.

net/net.gyp

Index: net/net.gyp
diff --git a/net/net.gyp b/net/net.gyp
index f8de4ffc7297fc4c6466973ade3a7fb19111cd53..6e611bf88b9a4790463fe7252ed06f7839dd237f 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -8,8 +8,8 @@
 
     'linux_link_kerberos%': 0,
     'conditions': [
-      ['chromeos==1 or OS=="android"', {
-        # Disable Kerberos on ChromeOS and Android, at least for now.
+      ['chromeos==1 or OS=="android" or OS=="ios"', {
+        # Disable Kerberos on ChromeOS, Android and iOS, at least for now.
         # It needs configuration (krb5.conf and so on).
         'use_kerberos%': 0,
       }, {  # chromeos == 0
@@ -35,14 +35,10 @@
       'dependencies': [
         '../base/base.gyp:base',
         '../base/base.gyp:base_i18n',
-        '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
         '../build/temp_gyp/googleurl.gyp:googleurl',
-        '../crypto/crypto.gyp:crypto',
-        '../sdch/sdch.gyp:sdch',
         '../third_party/icu/icu.gyp:icui18n',
         '../third_party/icu/icu.gyp:icuuc',
         '../third_party/zlib/zlib.gyp:zlib',
-        '../v8/tools/gyp/v8.gyp:v8',
         'net_resources',
       ],
       'sources': [
@@ -805,6 +801,17 @@
         '../base/base.gyp:base',
       ],
       'conditions': [
+        ['OS != "ios"', {
+          'dependencies': [

[Ryan Sleevi, 2012/07/28 00:33:13]

Any chance this can be OS == "ios" and dependencies!?

While I suppose there is an open question as to whether it's more preferable to
'list all dependencies and subtract' or 'list common dependencies then add', I
think following in the spirit of sources/sources!/sources/ (which lists all then
subtracts, generally), it'd be good to use subtraction rather than addition
here. 

[stuartmorgan, 2012/07/28 13:53:43]

Unfortunately dependencies! doesn't work when the gyp file doesn't exist at all,
and we don't pull a full tree on iOS, so there's no v8.gyp

[Ryan Sleevi, 2012/07/29 09:01:25]

Is that the same for all of the targets below? It would seem that at least
crypto/crypto.gyp should be checked out, since it's part of a 'main' Chromium
check-out.

My own take would be that it'd be more preferable to do the dependencies: { }
just for any non-checked out paths, and dependencies! for the
checked-out-but-not-used.

[noyau (Ping after 24h), 2012/07/30 10:32:19]

I've reduced the list as much as I could. However including crypto and removing
it via a conditional makes gyp_chromium fail so I left it there for now.

+            '../v8/tools/gyp/v8.gyp:v8',
+            # TODO(ios): This is temporary; currently almost nothing builds with
+            # OS=ios. Move dependencies back to the main dependencies section above

[Ryan Sleevi, 2012/07/28 00:33:13]

nit: 80 character wrapping

[noyau (Ping after 24h), 2012/07/30 10:32:19]

Done.

+            # as more is brought up for iOS.
+            '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
+            '../crypto/crypto.gyp:crypto',
+            '../sdch/sdch.gyp:sdch',
+          ],
+        }],
         ['chromeos==1', {
           'sources!': [
              'base/network_change_notifier_linux.cc',
@@ -1047,6 +1054,41 @@
             ],
           },
         ],
+        [ 'OS == "ios"', {
+            'link_settings': {
+              'libraries': [
+                '$(SDKROOT)/System/Library/Frameworks/CFNetwork.framework',
+                '$(SDKROOT)/System/Library/Frameworks/MobileCoreServices.framework',
+                '$(SDKROOT)/System/Library/Frameworks/SystemConfiguration.framework',
+                '$(SDKROOT)/usr/lib/libresolv.dylib',
+              ],
+            },
+            'sources/': [
+              # TODO(ios): Right now there is only a very limited subset of net
+              # compiled on iOS, just enough to bring up the dependencies needed
+              # by the ui target.
+              ['exclude', '.*'],
+              ['include', '^base/dns_util\\..*$'],
+              ['include', '^base/escape\\..*$'],
+              ['include', '^base/ip_endpoint\\..*$'],
+              ['include', '^base/mime_util\\..*$'],
+              ['include', '^base/net_errors\\..*$'],
+              ['include', '^base/net_errors_posix\\.cc$'],
+              ['include', '^base/net_export\\.h$'],
+              ['include', '^base/net_log\\..*$'],
+              ['include', '^base/net_module\\..*$'],
+              ['include', '^base/net_util\\..*$'],
+              ['include', '^base/net_util_posix\\.cc$'],
+              ['include', '^base/platform_mime_util\\.h$'],
+              ['include', '^base/registry_controlled_domains/registry_controlled_domain\\..*$'],
+              ['include', '^http/http_byte_range\\..*$'],
+              ['include', '^http/http_content_disposition\\..*$'],
+              ['include', '^http/http_util\\..*$'],
+              ['include', '^http/http_util_icu\\.cc$'],
+              ['include', '^http/http_version\\.h$'],
+            ],
+          },
+        ],
       ],
       'target_conditions': [
         ['OS == "android"', {
@@ -1054,6 +1096,11 @@
             ['include', '^base/platform_mime_util_linux\\.cc$'],
           ],
         }],
+        ['OS == "ios"', {
+          'sources/': [
+            ['include', 'base/platform_mime_util_mac\\.mm$'],

[Ryan Sleevi, 2012/07/28 00:33:13]

Any reason this is part of target_conditions and not part of the above sources
inclusions (line 1071-1088) ?

(FWIW, I have no idea why Android is doing what it's doing on 1096)

[stuartmorgan, 2012/07/28 13:53:43]

Same reason we're doing this probably; _mac. is excluded in a target_conditions
section in filename_rules.gypi, and sources/ is order-sensitive (deliberately).
So to re-include a file that's excluded by default due to platform rules, you
have to put in in target_conditionals so that it will end up after those rules
when everything is merged.

[Ryan Sleevi, 2012/07/29 09:01:25]

Ah, right.

Would it be possible then to add a comment (on say, line 1093-1094) commenting
on this fact? I know I've received several questions in the past re:
GYP-in-Chromium that essentially boiled down to cargo-culting ("I don't
understand why this is being done, but since it matches the existing code, I
assume it's correct"). Since this part is subtle, it would be good to comment on
it.

[noyau (Ping after 24h), 2012/07/30 10:32:19]

Done.

+          ],
+        }],
       ],
     },
     {
@@ -1064,9 +1111,7 @@
         'net_test_support',
         '../base/base.gyp:base',
         '../base/base.gyp:base_i18n',
-        '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
         '../build/temp_gyp/googleurl.gyp:googleurl',
-        '../crypto/crypto.gyp:crypto',
         '../testing/gmock.gyp:gmock',
         '../testing/gtest.gyp:gtest',
         '../third_party/zlib/zlib.gyp:zlib',
@@ -1307,6 +1352,12 @@
         'websockets/websocket_throttle_unittest.cc',
       ],
       'conditions': [
+        ['OS != "ios"', {
+          'dependencies': [

[Ryan Sleevi, 2012/07/28 00:33:13]

same comments re: dependencies!

[noyau (Ping after 24h), 2012/07/30 10:32:19]

Same answer. Moved one I could up, but I could not move crypto.

+            '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
+            '../crypto/crypto.gyp:crypto',
+          ],
+        }],
         ['chromeos==1', {
           'sources!': [
             'base/network_change_notifier_linux_unittest.cc',
@@ -1329,7 +1380,7 @@
             ],
           },
         ],
-        [ 'os_posix == 1 and OS != "mac" and OS != "android"', {
+        [ 'os_posix == 1 and OS != "mac" and OS != "android" and OS != "ios"', {
           'conditions': [
             ['linux_use_tcmalloc==1', {
               'dependencies': [
@@ -1417,7 +1468,29 @@
           'sources!': [
             'base/x509_cert_types_unittest.cc',
           ],
-        }]
+        }],
+        ['OS == "ios"', {
+          # TODO: For now this only test the subset of code that was enabled in
+          # the net target.
+          'dependencies': [
+            '../testing/gtest.gyp:gtest_main',
+          ],
+          'dependencies!': [
+            'net_test_support',
+          ],
+          'sources/': [
+            ['exclude', '.*'],
+            ['include', '^base/dns_util_unittest\\.cc$'],
+            ['include', '^base/escape_unittest\\.cc$'],
+            ['include', '^base/ip_endpoint_unittest\\.cc$'],
+            ['include', '^base/mime_util_unittest\\.cc$'],
+            ['include', '^base/net_log_unittest\\.cc$'],
+            ['include', '^base/registry_controlled_domains/registry_controlled_domain_unittest\\.cc$'],
+            ['include', '^http/http_byte_range_unittest\\.cc$'],
+            ['include', '^http/http_content_disposition_unittest\\.cc$'],
+            ['include', '^http/http_util_unittest\\.cc$'],
+          ],
+        }],
       ],
     },
     {
@@ -1446,56 +1519,16 @@
             ],
           },
         ],
-      ],
-    },
-    {
-      'target_name': 'stress_cache',
-      'type': 'executable',
-      'dependencies': [
-        'net',
-        'net_test_support',
-        '../base/base.gyp:base',
-      ],
-      'sources': [
-        'disk_cache/stress_cache.cc',
-      ],
-    },
-    {
-      'target_name': 'tld_cleanup',
-      'type': 'executable',
-      'dependencies': [
-        '../base/base.gyp:base',
-        '../base/base.gyp:base_i18n',
-        '../build/temp_gyp/googleurl.gyp:googleurl',
-      ],
-      'sources': [
-        'tools/tld_cleanup/tld_cleanup.cc',
-      ],
-    },
-    {
-      'target_name': 'crash_cache',
-      'type': 'executable',
-      'dependencies': [
-        'net',
-        'net_test_support',
-        '../base/base.gyp:base',
-      ],
-      'sources': [
-        'tools/crash_cache/crash_cache.cc',
-      ],
-    },
-    {
-      'target_name': 'run_testserver',
-      'type': 'executable',
-      'dependencies': [
-        'net',
-        'net_test_support',
-        '../base/base.gyp:base',
-        '../build/temp_gyp/googleurl.gyp:googleurl',
-        '../testing/gtest.gyp:gtest',
-      ],
-      'sources': [
-        'tools/testserver/run_testserver.cc',
+        ['OS == "ios"', {
+          'sources!': [
+            # We do not support PAC scripts on iOS.

[Ryan Sleevi, 2012/07/28 00:33:13]

I seem to recall there being a conversation on IRC with willchan that decided
proxies were supported?

nit: I'll echo mark@'s normal comments and say "avoid 'we' in comments" (eg:
"PAC scripts are not supported on iOS")

[noyau (Ping after 24h), 2012/07/30 10:32:19]

This comment is copied from the comment in the ios tree where this test has been
explicitely disabled.

Fixed the comment.

+            'proxy/proxy_resolver_perftest.cc',
+            # TODO:(ios): Enable those tests once the code to exercise is
+            # present in the net target.
+            'cookies/cookie_monster_perftest.cc',
+            'disk_cache/disk_cache_perftest.cc',
+          ],
+        }],
       ],
     },
     {
@@ -1568,7 +1601,7 @@
         'url_request/url_request_test_util.h',
       ],
       'conditions': [
-        ['inside_chromium_build==1', {
+        ['inside_chromium_build==1 and OS != "ios"', {
           'dependencies': [
             '../chrome/app/policy/cloud_policy_codegen.gyp:cloud_policy_proto_compile',
             # The test server uses Python modules generated by the sync protos.
@@ -1576,7 +1609,7 @@
             '../third_party/protobuf/protobuf.gyp:py_proto',
           ],
         }],
-        ['os_posix == 1 and OS != "mac" and OS != "android"', {
+        ['os_posix == 1 and OS != "mac" and OS != "android" and OS != "ios"', {
           'conditions': [
             ['use_openssl==1', {
               'dependencies': [
@@ -1589,7 +1622,7 @@
             }],
           ],
         }],
-        ['os_posix == 1 and OS != "mac" and OS != "android"', {
+        ['os_posix == 1 and OS != "mac" and OS != "android" and OS != "ios"', {
           'conditions': [
             ['linux_use_tcmalloc==1', {
               'dependencies': [
@@ -1626,56 +1659,6 @@
       'includes': [ '../build/grit_target.gypi' ],
     },
     {
-      'target_name': 'gdig',
-      'type': 'executable',
-      'dependencies': [
-        '../base/base.gyp:base',
-        'net',
-      ],
-      'sources': [
-        'tools/gdig/file_net_log.cc',
-        'tools/gdig/gdig.cc',
-      ],
-    },
-    {
-      'target_name': 'fetch_client',
-      'type': 'executable',
-      'variables': { 'enable_wexit_time_destructors': 1, },
-      'dependencies': [
-        'net',
-        '../base/base.gyp:base',
-        '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
-        '../build/temp_gyp/googleurl.gyp:googleurl',
-        '../testing/gtest.gyp:gtest',
-      ],
-      'sources': [
-        'tools/fetch/fetch_client.cc',
-      ],
-    },
-    {
-      'target_name': 'fetch_server',
-      'type': 'executable',
-      'variables': { 'enable_wexit_time_destructors': 1, },
-      'dependencies': [
-        'net',
-        '../base/base.gyp:base',
-        '../build/temp_gyp/googleurl.gyp:googleurl',
-      ],
-      'sources': [
-        'tools/fetch/fetch_server.cc',
-        'tools/fetch/http_listen_socket.cc',
-        'tools/fetch/http_listen_socket.h',
-        'tools/fetch/http_server.cc',
-        'tools/fetch/http_server.h',
-        'tools/fetch/http_server_request_info.cc',
-        'tools/fetch/http_server_request_info.h',
-        'tools/fetch/http_server_response_info.cc',
-        'tools/fetch/http_server_response_info.h',
-        'tools/fetch/http_session.cc',
-        'tools/fetch/http_session.h',
-      ],
-    },
-    {
       'target_name': 'http_server',
       'type': 'static_library',
       'variables': { 'enable_wexit_time_destructors': 1, },
@@ -1694,52 +1677,158 @@
         'server/web_socket.h',
       ],
     },
-    {
-      'target_name': 'dnssec_chain_verify',
-      'type': 'executable',
-      'dependencies': [
-        'net',
-        '../base/base.gyp:base',
-      ],
-      'sources': [
-        'tools/dnssec_chain_verify/dnssec_chain_verify.cc',
-      ],
-    },
-    {
-      'target_name': 'crl_set_dump',
-      'type': 'executable',
-      'dependencies': [
-        'net',
-        '../base/base.gyp:base',
-      ],
-      'sources': [
-        'tools/crl_set_dump/crl_set_dump.cc',
-      ],
-    },
-    {
-      'target_name': 'dns_fuzz_stub',
-      'type': 'executable',
-      'dependencies': [
-        'net',
-        '../base/base.gyp:base',
-      ],
-      'sources': [
-        'tools/dns_fuzz_stub/dns_fuzz_stub.cc',
-      ],
-    },
-    {
-      'target_name': 'net_watcher',
-      'type': 'executable',
-      'dependencies': [
-        'net',
-        '../base/base.gyp:base',
-      ],
-      'sources': [
-        'tools/net_watcher/net_watcher.cc',
-      ],
-    },
   ],
   'conditions': [
+    ['OS != "ios"', {

[Ryan Sleevi, 2012/07/28 00:33:13]

If iOS is going to continue to be a very, very minimal set of net/ for the
long-term (which I suspect it is), does it make more sense to do what Android
did and that is to list individual, explicit targets within the iOS all.gyp,
rather than move them wholesale to these sorts of conditionals?

[stuartmorgan, 2012/07/28 13:53:43]

Since an "All" target gets generated for each project, it's kind of annoying
having targets exist that can't be compiled because it means you can't build
"net (All)". What's the down-side to moving them?

[Ryan Sleevi, 2012/07/29 09:01:25]

My general concern is one of trying to be able to trace what a GYP file will do,
particularly when tracking down build errors that arise inconsistently. Similar
to the Chromium preference for conditionals that are structured as

if (foo())
  return;
bar();

as opposed to

if (!foo()) {
  bar();
} else {
  return;
}

I tend to prefer GYP files that are nested as little as possible, when possible.

The use of "net (All)" seems to be exceptional and tied to iOS - the use of
ninja or make (on Mac) have made such meta-targets unimportant, and the
equivalent in Visual Studio (net.sln) has also been largely discarded as
irrelevant in the face of chrome.sln / all.sln / some.sln.

+      'targets': [
+        # iOS doesn't have the concept of simple executables, those targets can't

[Ryan Sleevi, 2012/07/28 00:33:13]

nit: line wrapping

And is this an iOS limitation or a limitation with how iOS makes use of the
XCode generator? I can't imagine that iOS cares one way or the other. Since it
"should" be able to package helper executables the same as a normal app, I
imagine that a target that produces a 'raw' executable isn't much different as
far as the toolchain is concerned, it's only XCode that gets it wrong.

Perhaps indicate that it's an XCode limitation, so that if (and a big if) iOS
builds ever switched to say, ninja, we 'could' run such tests?

[stuartmorgan, 2012/07/28 13:53:43]

The fact that a command-line tool can't be build is an iOS limitation. It's
possible to make bundles, but it requires a fair amount of hoop-jumping (see
testing/gtest.gyp's iOS section).

We could make a more generic version of that and have common.gypi inject it into
all 'executable' type targets, I suppose, and maybe even build that into the
generator.

It's not at all clear how useful that is though, because you also have issues
like, an app must service the event loop promptly, so any executable that
doesn't run almost instantly needs potentially extensive code changes to ensure
it doesn't get killed by the OS. We've done that for tests, but we can't do it
generically for all possible programs.

[Ryan Sleevi, 2012/07/29 09:01:25]

The android port has addressed this via the <(gtest_target_type) [or something
like that], which alternates between 'shared_library' (when building a JNI-based
test executable) and 'executable' (when building as an NDK executable)

I thought XCode supported bundled executables (eg: under Contents/Resources or
w/e) regardless of targeting for OS X or iOS, which is why they didn't need to
exclude these targets, even though they never use them (and ergo, never build
them). If it does support those, and I'm not mistaken, then it should be "as
simple as" marking these as executables, but not actually building/including
them in any bundle.

That said, this is admittedly largely a stylistic concern with the GYP file, but
it seems like this concern will be one with quite a bit of the iOS upstreaming,
so like with the Android team, I'm just trying to find a solution early if there
is one.

+        # be compiled on the platform.
+        {
+          'target_name': 'dnssec_chain_verify',
+          'type': 'executable',
+          'dependencies': [
+            'net',
+            '../base/base.gyp:base',
+          ],
+          'sources': [
+            'tools/dnssec_chain_verify/dnssec_chain_verify.cc',
+          ],
+        },
+        {
+          'target_name': 'fetch_server',
+          'type': 'executable',
+          'variables': { 'enable_wexit_time_destructors': 1, },
+          'dependencies': [
+            'net',
+            '../base/base.gyp:base',
+            '../build/temp_gyp/googleurl.gyp:googleurl',
+          ],
+          'sources': [
+            'tools/fetch/fetch_server.cc',
+            'tools/fetch/http_listen_socket.cc',
+            'tools/fetch/http_listen_socket.h',
+            'tools/fetch/http_server.cc',
+            'tools/fetch/http_server.h',
+            'tools/fetch/http_server_request_info.cc',
+            'tools/fetch/http_server_request_info.h',
+            'tools/fetch/http_server_response_info.cc',
+            'tools/fetch/http_server_response_info.h',
+            'tools/fetch/http_session.cc',
+            'tools/fetch/http_session.h',
+          ],
+        },
+        {
+          'target_name': 'gdig',
+          'type': 'executable',
+          'dependencies': [
+            '../base/base.gyp:base',
+            'net',
+          ],
+          'sources': [
+            'tools/gdig/file_net_log.cc',
+            'tools/gdig/gdig.cc',
+          ],
+        },
+        {
+          'target_name': 'net_watcher',
+          'type': 'executable',
+          'dependencies': [
+            'net',
+            '../base/base.gyp:base',
+          ],
+          'sources': [
+            'tools/net_watcher/net_watcher.cc',
+          ],
+        },
+        {
+          'target_name': 'crl_set_dump',
+          'type': 'executable',
+          'dependencies': [
+            'net',
+            '../base/base.gyp:base',
+          ],
+          'sources': [
+            'tools/crl_set_dump/crl_set_dump.cc',
+          ],
+        },
+        {
+          'target_name': 'tld_cleanup',
+          'type': 'executable',
+          'dependencies': [
+            '../base/base.gyp:base',
+            '../base/base.gyp:base_i18n',
+            '../build/temp_gyp/googleurl.gyp:googleurl',
+          ],
+          'sources': [
+            'tools/tld_cleanup/tld_cleanup.cc',
+          ],
+        },
+        {
+          'target_name': 'dns_fuzz_stub',
+          'type': 'executable',
+          'dependencies': [
+            'net',
+            '../base/base.gyp:base',
+          ],
+          'sources': [
+            'tools/dns_fuzz_stub/dns_fuzz_stub.cc',
+          ],
+        },
+        {
+          'target_name': 'stress_cache',
+          'type': 'executable',
+          'dependencies': [
+            'net',
+            'net_test_support',
+            '../base/base.gyp:base',
+          ],
+          'sources': [
+            'disk_cache/stress_cache.cc',
+          ],
+        },
+        {
+          'target_name': 'crash_cache',
+          'type': 'executable',
+          'dependencies': [
+            'net',
+            'net_test_support',
+            '../base/base.gyp:base',
+          ],
+          'sources': [
+            'tools/crash_cache/crash_cache.cc',
+          ],
+        },
+        {
+          'target_name': 'run_testserver',
+          'type': 'executable',
+          'dependencies': [
+            'net',
+            'net_test_support',
+            '../base/base.gyp:base',
+            '../build/temp_gyp/googleurl.gyp:googleurl',
+            '../testing/gtest.gyp:gtest',
+          ],
+          'sources': [
+            'tools/testserver/run_testserver.cc',
+          ],
+        },
+        {
+          'target_name': 'fetch_client',
+          'type': 'executable',
+          'variables': { 'enable_wexit_time_destructors': 1, },
+          'dependencies': [
+            'net',
+            '../base/base.gyp:base',
+            '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
+            '../build/temp_gyp/googleurl.gyp:googleurl',
+            '../testing/gtest.gyp:gtest',
+          ],
+          'sources': [
+            'tools/fetch/fetch_client.cc',
+          ],
+        },
+      ],
+    }],
     ['inside_chromium_build==1', {
       'targets': [
         # This target depends on dependencies not fetched by WebKit's DEPS.
@@ -1779,7 +1868,7 @@
         },
       ],
     }],
-    ['os_posix == 1 and OS != "mac" and OS != "android"', {
+    ['os_posix == 1 and OS != "mac" and OS != "ios" and OS != "android"', {
       'targets': [
         {
           'target_name': 'flip_in_mem_edsm_server',