Issue 3168 - Add a script for generating new content blocker lists

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 18 matching lines @@
 EXCEPTIONRULES_URL = "https://easylist-downloads.adblockplus.org/exceptionrules.txt"
 EASYLIST_CONTENT_BLOCKER_PATH = os.path.join(BASE_PATH, "easylist_content_blocker.json")
 COMBINED_CONTENT_BLOCKER_PATH = os.path.join(BASE_PATH, "easylist+exceptionrules_content_blocker.json")
 
 def _update_abp2blocklist():
   if os.path.isdir(ABP2BLOCKLIST_PATH):
     subprocess.check_call(["hg", "pull", "-u", "-R", ABP2BLOCKLIST_PATH])
   else:
     subprocess.check_call(["hg", "clone", ABP2BLOCKLIST_URL,
                            ABP2BLOCKLIST_PATH])
-    subprocess.check_call(["npm", "install", "tldjs"], cwd=ABP2BLOCKLIST_PATH)

[Felix Dahlke, 2015/10/06 18:23:58]

Well, this is a bit ugly. Once my patch that adds package.json lands in
abp2blocklist, I can change this to just call "npm install" after every
clone/pull.

[Sebastian Noack, 2015/10/12 13:08:31]

This can be addressed now.

[Felix Dahlke, 2015/10/21 20:36:21]

Done.

+  subprocess.check_call(["npm", "install"], cwd=ABP2BLOCKLIST_PATH)
 
 def _download_filter_lists():
-  easylist_response = urllib2.urlopen(EASYLIST_URL)

[Sebastian Noack, 2015/10/12 13:08:31]

Please close the file like object returned by urlopen() in a finally-block.

[Felix Dahlke, 2015/10/21 20:36:22]

Done.

-  with tempfile.NamedTemporaryFile(mode="w", delete=False) as easylist_file:

[Sebastian Noack, 2015/10/12 13:08:31]

Always add "b" to the mode when dealing with binary files, for same behavior on
all platforms.

[Sebastian Noack, 2015/10/12 13:08:31]

How about, using mode NamedTemporaryFile("wb+"), passing around the file object
instead the filename? That way you don't have to unnecessarily open the file
multiple times, and the cleanup is done implicitly when it gets closed.

[Felix Dahlke, 2015/10/21 20:36:21]

Passing around RW file objects seemed pretty hacky to me. I tried it now, and it
does feel hacky, but I like how it gets rid of the manual cleanup and shortens
things a bit. So I'm fine with this if you prefer it, it's in the latest patch
set.

[Felix Dahlke, 2015/10/21 20:36:22]

Those are ASCII files, however, should I still add the "b"?

[Sebastian Noack, 2015/10/21 20:56:23]

Either you treat it as a binary file. Then you should add "b" to prevent Windows
from messing with line endings. Or you can treat if as text files using
io.open(.., encoding=..).

[Felix Dahlke, 2015/10/22 02:43:45]

I see, done.

-    easylist_file.write(easylist_response.read())

[Sebastian Noack, 2015/10/12 13:08:31]

Please use shutil.copyfileobj consistently.

[Felix Dahlke, 2015/10/21 20:36:21]

Done.

-  exceptionrules_response = urllib2.urlopen(EXCEPTIONRULES_URL)
-  with tempfile.NamedTemporaryFile(mode="w", delete=False) as exceptionrules_file:
-    exceptionrules_file.write(exceptionrules_response.read())
-  return (easylist_file.name, exceptionrules_file.name)
+  try:
+    easylist_response = urllib2.urlopen(EASYLIST_URL)
+    easylist_file = tempfile.NamedTemporaryFile(mode="wb+")
+    shutil.copyfileobj(easylist_response, easylist_file)
+  finally:
+    easylist_response.close()
 
-def _concatenate_files(*source_paths):
-  with tempfile.NamedTemporaryFile(mode="w", delete=False) as destination_file:
-    for source_path in source_paths:
-      with open(source_path, "r") as source_file:
-        shutil.copyfileobj(source_file, destination_file)
-  return destination_file.name
+  try:
+    exceptionrules_response = urllib2.urlopen(EXCEPTIONRULES_URL)
+    exceptionrules_file = tempfile.NamedTemporaryFile(mode="wb+")
+    shutil.copyfileobj(exceptionrules_response, exceptionrules_file)
+  finally:
+    exceptionrules_response.close()
 
-def _convert_filter_list(source_path, destination_path):
-  with open(source_path, "r") as source_file, \
-       open(destination_path, "w") as destination_file:
+  return (easylist_file, exceptionrules_file)
+
+def _convert_filter_list(source_file, destination_path):
+  source_file.seek(0)
+  with open(destination_path, "wb") as destination_file:
     subprocess.check_call(["node", "abp2blocklist.js"],
                           cwd=ABP2BLOCKLIST_PATH, stdin=source_file,
                           stdout=destination_file)
+
+def _concatenate_files(*source_files):
+  destination_file = tempfile.NamedTemporaryFile(mode="wb+")
+  for source_file in source_files:
+    source_file.seek(0)
+    shutil.copyfileobj(source_file, destination_file)
+  return destination_file
 
 if __name__ == "__main__":
   print "Fetching/updating abp2blocklist ..."
   _update_abp2blocklist()
 
   print "Downloading filter lists ..."
-  easylist_path, exceptionrules_path = _download_filter_lists()
+  easylist_file, exceptionrules_file = _download_filter_lists()
 
   try:
     print "Generating %s ..." % os.path.basename(EASYLIST_CONTENT_BLOCKER_PATH)
-    _convert_filter_list(easylist_path, EASYLIST_CONTENT_BLOCKER_PATH)
+    _convert_filter_list(easylist_file, EASYLIST_CONTENT_BLOCKER_PATH)
 
     print "Generating %s ..." % os.path.basename(COMBINED_CONTENT_BLOCKER_PATH)
-    combined_path = _concatenate_files(easylist_path, exceptionrules_path)
-    try:
-      _convert_filter_list(combined_path, COMBINED_CONTENT_BLOCKER_PATH)
-    finally:
-      os.remove(combined_path)
+    with _concatenate_files(easylist_file, exceptionrules_file) as combined_file:
+      _convert_filter_list(combined_file, COMBINED_CONTENT_BLOCKER_PATH)
   finally:
-    os.remove(easylist_path)
-    os.remove(exceptionrules_path)
+    easylist_file.close()
+    exceptionrules_file.close()