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")
+    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")
+      subprocess.check_call(("hg", "clone", abp2blocklist_url,
+                             abp2blocklist_path), stdout=devnull)
+    subprocess.check_call(("npm", "install"), cwd=abp2blocklist_path,
+                          stdout=devnull)
+
+def _download(url):
+  response = urllib2.urlopen(url)
+  try:
+    return response.read()
+  finally:
+    response.close()
+
+def _convert_filter_list(source, destination_path):
+  with open(destination_path, "wb") as destination_file:
+    abp2blocklist_path = _get_config_value("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)
+
+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"))
+
+  _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"))