Issue 3176 - Add metadata to 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.
 #
@@ skipping 24 matching lines @@
     abp2blocklist_path = config["abp2blocklist_path"]
     if os.path.isdir(abp2blocklist_path):
         subprocess.check_call(("hg", "pull", "-u", "-R", abp2blocklist_path),
                               stdout=devnull)
     else:
       subprocess.check_call(("hg", "clone", config["abp2blocklist_url"],
                              abp2blocklist_path), stdout=devnull)
     subprocess.check_call(("npm", "install"), cwd=abp2blocklist_path,
                           stdout=devnull)
 
-def parse_filter_list_header(filter_list):
-  body_start = re.search(r"^[^![]", filter_list, re.MULTILINE).start()
-  field_re = re.compile(r"^!\s*([^:\s]+):\s*(.+)$", re.MULTILINE)

[Sebastian Noack, 2015/12/07 12:38:24]

This logic can be simplified, in particular since we are only looking for the
version:

for match in re.finditer(r"^(?:([^![])|!\s*Version:\s*(.+)$)", filter_list,
re.MULTILINE):
  end_of_headers, version = match.groups()
  if end_of_headers:
    break
  if version:
    return version

[kzar, 2015/12/08 12:52:21]

IMHO that doesn't look easier to read. Also it seems to me that we should just
parse all the fields while at it.

[Felix Dahlke, 2015/12/08 13:39:51]

Reading both pieces of code for the first time, I consider Sebastian's variant
to be a bit easier to get, but the difference isn't huge. Considering how we
really don't need anything but the version, I'd lean towards that one.

[kzar, 2015/12/08 14:34:24]

OK I've gone with a slightly simplified version of your approach Sebastian.

-  return { match.group(1): match.group(2)
-           for match in field_re.finditer(filter_list, 0, body_start) }
-
 def download_filter_list(url):
   with closing(urllib2.urlopen(url)) as response:
     body = response.read()
-  header = parse_filter_list_header(body)
-  return body, url, header["Version"]
+  version = re.search(r"^(?:[^[!])|^!\s*Version:\s*(.+)$",
+                      body, re.MULTILINE).group(1)
+  return body, url, version
 
 def generate_metadata(filter_lists, expires):
   metadata = OrderedDict((
     ("version", time.strftime("%Y%m%d%H%M", time.gmtime())),
     ("expires", expires),
     ("sources", [])
   ))
   for body, url, version in filter_lists:
-    metadata["sources"].append({ "url": url, "version": version })

[Sebastian Noack, 2015/12/07 12:38:24]

See https://www.python.org/dev/peps/pep-0008/#pet-peeves

[kzar, 2015/12/08 12:52:21]

Done.

+    metadata["sources"].append({"url": url, "version": version})
   return metadata
 
 def pipe_in(process, filter_lists):
   try:
     for body, _, _ in filter_lists:
       print >>process.stdin, body
   finally:
     process.stdin.close()
-    process.wait()
 
 def write_block_list(filter_lists, path, expires):
   block_list = generate_metadata(filter_lists, expires)
   process = subprocess.Popen(("node", "abp2blocklist.js"),
                              cwd=config["abp2blocklist_path"],
                              stdin=subprocess.PIPE, stdout=subprocess.PIPE)
   threading.Thread(target=pipe_in, args=(process, filter_lists)).start()
   block_list["rules"] = json.load(process.stdout)
-  if process.returncode:

[Sebastian Noack, 2015/12/07 12:38:24]

Note that returncode is set by the wait() method. So we should rather call it
here rather than in the thread to avoid race conditions.

[kzar, 2015/12/08 12:52:21]

Done.

+  if process.wait():
     raise Exception("abp2blocklist returned %s" % process.returncode)
 
   with open(path, "wb") as destination_file:
     json.dump(block_list, destination_file, indent=2, separators=(",", ": "))
 
 if __name__ == "__main__":
   update_abp2blocklist()
 
   easylist = download_filter_list(config["easylist_url"])
   exceptionrules = download_filter_list(config["exceptionrules_url"])
 
   write_block_list([easylist],
                    config["easylist_content_blocker_path"],
                    config["easylist_content_blocker_expires"])
   write_block_list([easylist, exceptionrules],
                    config["combined_content_blocker_path"],
                    config["combined_content_blocker_expires"])