Refactor the python system_impl module.

third_party/cython/rules.gni

Index: third_party/cython/rules.gni
diff --git a/third_party/cython/rules.gni b/third_party/cython/rules.gni
index 3938110df18dc381dffb92a7c4efabfab4939c4c..36b6225fd6f338e69c44298b2335eacba163c197 100644
--- a/third_party/cython/rules.gni
+++ b/third_party/cython/rules.gni
@@ -2,34 +2,37 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-template("python_binary_module_sources") {
+template("python_binary_source_set") {
   # Only available on linux for now.
   assert(is_linux)
-  assert(defined(invoker.sources))
+  assert(defined(invoker.cython_sources) || defined(invoker.sources))
 
-  cython_root = "//third_party/cython"
-  cython_script = "$cython_root/src/cython.py"
-  cython_output = "${target_out_dir}/${target_name}.cc"
-
-  generator_target_name = target_name + "_cython_compiler"
   config_name = target_name + "_python_config"
 
   target_visibility = [ ":$target_name" ]
 
-  action(generator_target_name) {
-    visibility = target_visibility
-    script = cython_script
-    sources = invoker.sources
-    outputs = [
-      cython_output,
-    ]
-    args = [
-             "--cplus",
-             "-I",
-             rebase_path("//", root_build_dir),
-             "-o",
-             rebase_path(cython_output, root_build_dir),
-           ] + rebase_path(sources, root_build_dir)
+  if (defined(invoker.cython_sources)) {
+    generator_target_name = target_name + "_cython_compiler"
+
+    cython_root = "//third_party/cython"
+    cython_script = "$cython_root/src/cython.py"
+    cython_output = "${target_out_dir}/${target_name}.cc"
+
+    action(generator_target_name) {
+      visibility = target_visibility
+      script = cython_script
+      sources = invoker.cython_sources
+      outputs = [
+        cython_output,
+      ]
+      args = [
+               "--cplus",
+               "-I",
+               rebase_path("//", root_build_dir),
+               "-o",
+               rebase_path(cython_output, root_build_dir),
+             ] + rebase_path(sources, root_build_dir)
+    }
   }
 
   config(config_name) {
@@ -45,9 +48,10 @@ template("python_binary_module_sources") {
   }
 
   source_set(target_name) {
-    deps = [
-      ":$generator_target_name",
-    ]
+    deps = []

[qsr, 2014/12/12 09:09:54]

Could you run gn format, I'm pretty sure deps should be the last thing in the
target.

[etiennej, 2014/12/12 10:58:25]

I do not mind moving it, but I ran gn format (both now, and as a presubmit
before sending the CL) and there's no change.

[qsr, 2014/12/12 12:27:45]

 Hum, weird. Can you take a look at
https://code.google.com/p/chromium/wiki/GNStyleGuide and make sure you follow
the syle, in particular for the ordering of the variables. Thanks.

 Also, this still should be pushed in the chromium repo and not the mojo repo.

[etiennej, 2014/12/12 13:39:46]

Made the fixes. I'll prepare a CL in chromium with the same changes.

+    if (defined(invoker.cython_sources)) {
+      deps += [ ":$generator_target_name" ]
+    }
     if (defined(invoker.visibility)) {
       visibility = invoker.visibility
     }
@@ -57,11 +61,12 @@ template("python_binary_module_sources") {
     if (defined(invoker.datadeps)) {
       datadeps = invoker.datadeps
     }
-    sources = [
-      cython_output,
-    ]
-    if (defined(invoker.additional_sources)) {
-      sources += invoker.additional_sources
+    sources = []
+    if (defined(invoker.cython_sources)) {
+      sources += [ cython_output ]
+    }
+    if (defined(invoker.sources)) {
+      sources += invoker.sources
     }
     all_dependent_configs = [ ":$config_name" ]
   }
@@ -70,7 +75,10 @@ template("python_binary_module_sources") {
 template("python_binary_module") {
   # Only available on linux for now.
   assert(is_linux)
-  assert(defined(invoker.sources))
+
+  has_sources = defined(invoker.cython_sources) || defined(invoker.sources)
+
+  assert(has_sources || defined(invoker.deps))
   assert(defined(invoker.python_base_module))
 
   sources_target_name = target_name + "_cython_sources"
@@ -88,25 +96,39 @@ template("python_binary_module") {
     ":$target_name",
   ]
 
-  python_binary_module_sources(sources_target_name) {
-    visibility = target_visibility
-    sources = invoker.sources
+  if (has_sources) {
+    python_binary_source_set(sources_target_name) {
+      visibility = target_visibility
+      if (defined(invoker.cython_sources)) {
+        cython_sources = invoker.cython_sources
+      }
+      if (defined(invoker.sources)) {
+        sources = invoker.sources
+      }
+      if (defined(invoker.deps)) {
+        deps = invoker.deps
+      }
+      if (defined(invoker.datadeps)) {
+        datadeps = invoker.datadeps
+      }
+      if (defined(invoker.configs)) {
+        configs = invoker.configs
+      }
+    }
   }
 
   shared_library(shared_library_name) {
     visibility = target_visibility
-    deps = [
-      ":$sources_target_name",
-    ]
+    deps = []
+    if (has_sources) {
+      deps += [ ":$sources_target_name" ]
+    }
     if (defined(invoker.deps)) {
       deps += invoker.deps
     }
     if (defined(invoker.datadeps)) {
       datadeps = invoker.datadeps
     }
-    if (defined(invoker.additional_sources)) {
-      sources = invoker.additional_sources
-    }
     if (defined(invoker.configs)) {
       configs += invoker.configs
     }