Issue 3168 - Add a script for generating content blocker lists

sitescripts/content_blocker_lists/bin/generate_lists.py

 #!/usr/bin/env python
 # coding: utf-8
 
 # This file is part of Adblock Plus <https://adblockplus.org/>,
 # Copyright (C) 2006-2015 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
 import subprocess
 import urllib2
 
 from sitescripts.utils import get_config
 
-def _get_config_value(key):
-  return get_config().get("content_blocker_lists", key)

[Sebastian Noack, 2015/11/18 20:10:37]

We probably shouldn't call get_config every for every option again.

[Felix Dahlke, 2015/11/19 11:15:20]

After implementing your suggestion below, I could get rid of this thing without
hurting readability.

-
 def _update_abp2blocklist():
   with open(os.devnull, "w") as devnull:
-    abp2blocklist_path = _get_config_value("abp2blocklist_path")
+    config = get_config()
+    abp2blocklist_path = config.get("content_blocker_lists",
+                                    "abp2blocklist_path")
     if os.path.isdir(abp2blocklist_path):
         subprocess.check_call(("hg", "pull", "-u", "-R", abp2blocklist_path),
                               stdout=devnull)
     else:
-      abp2blocklist_url = _get_config_value("abp2blocklist_url")
+      abp2blocklist_url = config.get("content_blocker_lists",
+                                     "abp2blocklist_url")
       subprocess.check_call(("hg", "clone", abp2blocklist_url,
                              abp2blocklist_path), stdout=devnull)
     subprocess.check_call(("npm", "install"), cwd=abp2blocklist_path,
                           stdout=devnull)
 
-def _download(url):
+def _download(url_key):
+  url = get_config().get("content_blocker_lists", url_key)
   response = urllib2.urlopen(url)
   try:
     return response.read()
   finally:
     response.close()
 
-def _convert_filter_list(source, destination_path):
+def _convert_filter_list(sources, destination_path_key):
+  config = get_config()
+  destination_path = config.get("content_blocker_lists", destination_path_key)
   with open(destination_path, "wb") as destination_file:
-    abp2blocklist_path = _get_config_value("abp2blocklist_path")
+    abp2blocklist_path = config.get("content_blocker_lists",
+                                    "abp2blocklist_path")
     process = subprocess.Popen(("node", "abp2blocklist.js"),
                                cwd=abp2blocklist_path, stdin=subprocess.PIPE,
                                stdout=destination_file)
-    process.communicate(input=source)
-    if process.returncode:
-      raise Exception("abp2blocklist returned %s" % process.returncode)
+    try:
+      for source in sources:
+        print >>process.stdin, source
+    finally:
+      process.stdin.close()
+      process.wait()
+
+  if process.returncode:
+    raise Exception("abp2blocklist returned %s" % process.returncode)
 
 if __name__ == "__main__":
   _update_abp2blocklist()
 
-  easylist = _download(_get_config_value("easylist_url"))

[Sebastian Noack, 2015/11/18 20:10:37]

As _download is only used in these two lines, and both call _get_config_value()
before, how about moving that call into _download()?

[Felix Dahlke, 2015/11/19 11:15:20]

Done, did the same for _convert_filter_list.

-  exceptionrules = _download(_get_config_value("exceptionrules_url"))
+  easylist = _download("easylist_url")
+  exceptionrules = _download("exceptionrules_url")
 
-  _convert_filter_list(easylist,
-                       _get_config_value("easylist_content_blocker_path"))
-
-  combined = "\n".join((easylist, exceptionrules))
-  _convert_filter_list(combined,

[kzar, 2015/11/18 17:35:38]

Nit: Maybe avoid the combined variable?

_convert_filter_list(easylist + "\n" + exceptionrules,
...

[Sebastian Noack, 2015/11/18 20:10:37]

Even better:

_convert_filter_list("%s\n%s" % (easylist, exceptionrules))

[Sebastian Noack, 2015/11/18 23:24:34]

Or maybe even even better: Make _convert_filter_list() take a variable number of
sources, passes them separately to the subprocess. This would be way more
efficient, and doesn't seem to add much complexity:


def _convert_filter_list(sources, destination_path):
  with open(destination_path, "wb") as destination_file:
    process = subprocess.Popen(...)
    try:
      for source in sources:
        print >>process.stdin, source
    finally:
      process.stdin.close()
      retcode = process.wait()
    
  if retcode:
    raise Exception("abp2blocklist returned %s" % process.returncode)
      

[Felix Dahlke, 2015/11/19 11:15:20]

Yeah I actually like that one better too, done.

-                       _get_config_value("combined_content_blocker_path"))
+  _convert_filter_list([easylist], "easylist_content_blocker_path")
+  _convert_filter_list([easylist, exceptionrules],
+                       "combined_content_blocker_path")