Improved generation of filter subscription files

The main goal here was replacing rsync with atomic updates - from what I can tell, the only way of doing this is creating a new directory and changing the symbolic link that points to the data directory. However, I fixed a number of other issues with these ancient scripts, most importantly:

* Don't produce unnecessary temporary directories on server, work with in-memory data instead.
* Allow specifying multiple sources when combineSubscriptions.py is called directly (this file is used by filter authors for testing).

[Sebastian Noack, 2013-11-06 15:56:22]

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/b...
File sitescripts/subscriptions/bin/updateSubscriptionDownloads.py (right):

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/b...
sitescripts/subscriptions/bin/updateSubscriptionDownloads.py:57:
shutil.rmtree(destination, True)
I would go for shutil.rmtree(destination, ignore_errors=True) instead of passing
ignore_errors as positional argument, to make it more obvious for whoever reads
the code, what the boolean passed to shutil.rmtree() is about.

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/c...
File sitescripts/subscriptions/combineSubscriptions.py (right):

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/c...
sitescripts/subscriptions/combineSubscriptions.py:27: global
accepted_extensions, ignore, verbatim
The global keyword is unneeded here. I know it was there before, but feel free
to remove it. The global keyword makes assignments below, assigning the variable
in the global instead of local scope. However it doesn't affect reading of
variables.

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/c...
sitescripts/subscriptions/combineSubscriptions.py:102: lines.insert(0, "!
Checksum: %s" % re.sub(r"=", "", base64.b64encode(checksum.digest())))
You don't need a regex to strip a given character. You could just use
.replace("=", "") or since the equals character is only used as padding in the
end by base64 you could also use .rstrip('='), wich IMHO makes the intention of
the code even more obvious.

However I've never seen the use of base64 to bring an MD5 digit in a printable
form. Ususally you you just convert it to hexadecimal, which can be done as easy
as checksum.hexdigest() in Python. But I don't know if changing that now would
break something.

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/c...
sitescripts/subscriptions/combineSubscriptions.py:183: isexception = False
Apparently you don't like underscores, but "is_exception" would be way more
readable than "isexcpetion". ;)

[Wladimir Palant, 2013-11-08 15:08:06]

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/c...
File sitescripts/subscriptions/combineSubscriptions.py (right):

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/c...
sitescripts/subscriptions/combineSubscriptions.py:102: lines.insert(0, "!
Checksum: %s" % re.sub(r"=", "", base64.b64encode(checksum.digest())))
On 2013/11/06 15:56:22, sebastian wrote:
> However I've never seen the use of base64 to bring an MD5 digit in a printable
> form. Ususally you you just convert it to hexadecimal, which can be done as
easy
> as checksum.hexdigest() in Python. But I don't know if changing that now would
> break something.

It's way too late to change the specification of checksums. It's the way it is
because Mozilla APIs produce base64 - and we mimic this everywhere else.

http://codereview.adblockplus.org/28037010/diff/1/sitescripts/subscriptions/c...
sitescripts/subscriptions/combineSubscriptions.py:183: isexception = False
On 2013/11/06 15:56:22, sebastian wrote:
> Apparently you don't like underscores, but "is_exception" would be way more
> readable than "isexcpetion". ;)

As you wish...

[Sebastian Noack, 2013-11-09 09:50:16]

LGTM

[Wladimir Palant, 2013-11-11 14:56:19]

Turned out that this approach doesn't work too well. Two issues with it:

1. rsync might already be syncing files when we finish updating subscriptions
and remove the old directory - it doesn't like that directory to be removed
right under it.

2. New directory being created each times means that temporary failures (e.g.
download not working) result in files being removed from the directory.

So the approach now is keeping the directory but replace the files in it
atomically.

[Sebastian Noack, 2013-11-12 18:48:20]

LGTM

[Wladimir Palant, 2013-11-13 08:48:08]

For reference, this was creating files with wrong permissions
(NamedTemporaryFile always sets them to 0600) so an additional change became
necessary: https://hg.adblockplus.org/sitescripts/rev/135d45395e54. However,
things seem to work fine now.