Opened on 11/18/2016 at 05:13:09 PM

Closed on 11/25/2016 at 02:02:09 PM

Last modified on 12/06/2016 at 11:31:59 AM

#4653 closed defect (fixed)

Safari Extension Update Manifest Generation Broken

Reported by: matze Assignee:
Priority: P1 Milestone:
Module: Sitescripts Keywords:
Cc: jsonesen, kvas, sebastian, kzar, philll, scheer Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by philll)

November 17th we received the following error-message in production:

Subject: Cron <sitescripts@***> /usr/local/bin/update_update_manifests
Date: Fri, 18 Nov 2016 07:20:13 +0000 (UTC)
From: Cron Daemon <root@***>

Traceback (most recent call last):
  File "/usr/lib/python2.7/", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/", line 72, in _run_code
    exec code in run_globals
  File "/opt/sitescripts/sitescripts/extensions/bin/", line 139, in <module>
  File "/opt/sitescripts/sitescripts/extensions/bin/", line 135, in updateUpdateManifests
  File "/opt/sitescripts/sitescripts/extensions/", line 324, in getDownloadLinks
    result.set(repo.repositoryName, 'downloadURL', downloadURL)
  File "/usr/lib/python2.7/", line 743, in set
    raise TypeError("option values must be strings")
TypeError: option values must be strings

This lead us to discovering an issue introduced in the context if #1483, revision 8c3d5a9c5328: The sitescripts/extensions/ script does not cover the case when no downloadURL is found any more. The following hot-fix made it work in production again:

diff -r 92b7a4428709 sitescripts/extensions/
--- a/sitescripts/extensions/   Mon Nov 07 14:01:53 2016 +0000
+++ b/sitescripts/extensions/   Fri Nov 18 16:50:44 2016 +0000
@@ -291,8 +291,15 @@
     gets the download link to the most current version of an extension
     if repo.galleryID and repo.type == 'gecko':
-        return _getMozillaDownloadLink(repo.galleryID)
-    return _getLocalLink(repo)
+        result = _getMozillaDownloadLink(repo.galleryID)
+    else:
+        result = _getLocalLink(repo)
+    if result[0] is None:
+        message = 'Failed to get download link for {}'.format(repo)
+        raise Exception(message)
+    return result

 def _getQRCode(text):

While the script does not abort on error with one repository any more, we now saw what resource caused the issue popping up in the first place:

Traceback (most recent call last):
  File "/opt/sitescripts/sitescripts/extensions/", line 325, in getDownloadLinks
    (downloadURL, version) = _getDownloadLink(repo)
  File "/opt/sitescripts/sitescripts/extensions/", line 300, in _getDownloadLink
    raise Exception(message)
Exception: Failed to get download link for adblockplussafari

So it seems like in addition to the minor bug above there is an issue with the Safari extension's update manifest generation. Probably this one is related to the circumstance that this one is part of the Chrome repository now. Here's an excerpt of the related sitescripts.ini settings:

$ grep safari /etc/sitescripts.ini

All other repositories' update manifest generations work flawlessly.

A major effect of this is that the link to the ABP for Safari extension on is not 404.


  • Improve on the aforementioned patch-set to fix the cron-job
  • Identify and fix the issue preventing Safari update manifest generation


Currently the update mechanism for the sitescripts repository on the affected production host(s) has been paused. Hence no other code updates will come through here until we re-activate that one, which we will do after a patch-set similar to the hot-fix has landed.

Attachments (0)

Change History (14)

comment:1 Changed on 11/23/2016 at 10:46:59 AM by kzar

  • Cc kzar philll scheer added

comment:2 Changed on 11/23/2016 at 03:55:11 PM by kzar

(I believe the current URL for the latest Safari release build is .)

comment:3 Changed on 11/23/2016 at 03:59:32 PM by philll

  • Description modified (diff)

I recommend providing the download for ABP for Safari from the URL kzar posted above until this issue gets fixed.

comment:4 Changed on 11/23/2016 at 04:14:33 PM by kzar

@matze what is the contents (if any) of the broken Safari update manifest? (File at the path given by safariUpdateManifestPath.)

comment:5 Changed on 11/23/2016 at 04:32:14 PM by matze

$ cat /var/www/update/adblockplussafari/updates.plist
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "">
<plist version="1.0">
   <key>Extension Updates</key>

comment:6 Changed on 11/23/2016 at 05:19:56 PM by kzar

I am not familiar with this code, but after having a quick look I've found the/a problem.

sitescripts.extensions.utils.getDownloads calls the readMetadata function with no arguments. That function defaults to the "tip" revision and so it will try to read the metadata.safari file for the "tip" revision. That no longer exists, which explains why the problem started happening since November 17th as that was the day that the commit to remove that file landed.

If I'm right - and I didn't test that I am - then I think this should fix it:

diff --git a/sitescripts/extensions/ b/sitescripts/extensions/
index da53e91..1e55607 100644
--- a/sitescripts/extensions/
+++ b/sitescripts/extensions/
@@ -207,7 +207,7 @@ class Configuration(object):
         return parser
     def getDownloads(self):
-        metadata = self.readMetadata()
+        metadata = self.readMetadata(self.revision)
         if metadata:
             prefix = metadata.get('general', 'basename')
Last edited on 11/23/2016 at 05:21:21 PM by kzar

comment:7 Changed on 11/23/2016 at 05:34:08 PM by matze

We have extended the hotfix already, pretty similar to what kzar proposed above:

diff -r 92b7a4428709 sitescripts/extensions/
--- a/sitescripts/extensions/	Mon Nov 07 14:01:53 2016 +0000
+++ b/sitescripts/extensions/	Wed Nov 23 17:29:21 2016 +0000
@@ -183,11 +183,12 @@
         return self.repositoryName
-    def readMetadata(self, version='tip'):
+    def readMetadata(self, version=None):
+        revision = version or self.revision
         genericFilename = 'metadata'
         filename = '%s.%s' % (genericFilename, self.type)
         files = subprocess.check_output(['hg', '-R', self.repository,
-                                         'locate', '-r', version]).splitlines()
+                                         'locate', '-r', revision]).splitlines()
         if filename not in files:
             # some repositories like those for Android and
@@ -198,7 +199,7 @@
             # Fall back to platform-independent metadata file
             filename = genericFilename
-        command = ['hg', '-R', self.repository, 'cat', '-r', version, os.path.join(self.repository, filename)]
+        command = ['hg', '-R', self.repository, 'cat', '-r', revision, os.path.join(self.repository, filename)]
         result = subprocess.check_output(command)
         parser = SafeConfigParser()
@@ -291,8 +292,15 @@
     gets the download link to the most current version of an extension
     if repo.galleryID and repo.type == 'gecko':
-        return _getMozillaDownloadLink(repo.galleryID)
-    return _getLocalLink(repo)
+        result = _getMozillaDownloadLink(repo.galleryID)
+    else:
+        result = _getLocalLink(repo)
+    if None is result[0]:
+        message = 'Failed to get download link for {}'.format(repo)
+        raise Exception(message)
+    return result
 def _getQRCode(text):

Smoke testing suggests that this should work now. Sidenote: The HG repositories on the update server did not feature *any* bookmarks. I guess they where initially cloned when the repos did not have bookmarks yet.

comment:8 Changed on 11/23/2016 at 05:35:25 PM by matze

Also, we've had to update sitescripts.ini to include the following:

Last edited on 11/23/2016 at 05:59:48 PM by matze

comment:9 Changed on 11/23/2016 at 05:49:12 PM by kvas

  • Priority changed from Unknown to P2
  • Ready set

comment:10 Changed on 11/24/2016 at 02:39:19 PM by sebastian

  • Priority changed from P2 to P1

While there is a temporary workaround manually deployed on the server now, thus Adblock Plus for Safari can be downloaded again, we should still fix this properly as soon as possible. We cannot update the sitescripts code on that server anymore until this bug has been addressed upstream, since doing so would revert the manually applied changes.

Last edited on 11/24/2016 at 02:39:48 PM by sebastian

comment:11 Changed on 11/25/2016 at 11:55:53 AM by jsonesen

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:12 Changed on 11/25/2016 at 01:56:21 PM by abpbot

A commit referencing this issue has landed:
Issue 4653 - Add handling for bookmarks to readMetadata

comment:13 Changed on 11/25/2016 at 02:02:09 PM by jsonesen

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:14 Changed on 12/06/2016 at 11:31:59 AM by abpbot

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
Note: See TracTickets for help on using tickets.