Issue 4540 - Add Platform Specific Branch Support to createNightlies.py

sitescripts/extensions/bin/createNightlies.py

Index: sitescripts/extensions/bin/createNightlies.py
===================================================================
--- a/sitescripts/extensions/bin/createNightlies.py
+++ b/sitescripts/extensions/bin/createNightlies.py
@@ -60,16 +60,23 @@
     """
 
     def __init__(self, config):
         """
           Creates a NightlyBuild instance; we are simply
           recording the configuration settings here.
         """
         self.config = config
+        try:
+            self.bookmark = self.config.get(
+                'extensions', 'abp{}_bookmark'.format(self.config.type)
+            )

[Wladimir Palant, 2016/10/25 16:32:04]

Ok, so the config example makes the impression as if the entries would follow
the "abp{type}_{name}" scheme. However, they don't - the prefix is a build name
which is completely arbitrary. In fact, at least on the update server we use
"adblockplussafari_{name}". Rather than trying to guess the setting's name here,
it should just be added to the Configuration object in
sitescripts.extensions.utils:

  bookmark = _defineProperty('bookmark', default='master')

Then you can access it as self.config.bookmark here.

I'm also wondering why we need to call this "bookmark" here, asked in the issue:
https://issues.adblockplus.org/ticket/4540#comment:13

[Jon Sonesen, 2016/10/25 16:39:58]

so adding this will cause whatever option ending with 'bookmark' to be set as a
config object property?

[Jon Sonesen, 2016/10/25 16:46:48]

also then do I need to change it to bookmark? I think we changed it because that
is just what it is called in mercurial but I am happy to change it back to
branch

+        except ConfigParser.NoOptionError:
+            self.bookmark = 'master'
+
         self.revision = self.getCurrentRevision()
         try:
             self.previousRevision = config.latestRevision
         except:
             self.previousRevision = '0'
         self.buildNum = None
         self.tempdir = None
         self.outputFilename = None
@@ -78,18 +85,18 @@
     def hasChanges(self):
         return self.revision != self.previousRevision
 
     def getCurrentRevision(self):
         """
             retrieves the current revision ID from the repository
         """
         command = [
-            'hg', 'id', '-i', '-r', 'default', '--config', 'defaults.id=',
-            self.config.repository
+            'hg', 'id', '-i', '-r', self.bookmark, '--config',
+            'defaults.id=', self.config.repository
         ]
         return subprocess.check_output(command).strip()
 
     def getCurrentBuild(self):
         """
             calculates the (typically numerical) build ID for the current build
         """
         command = ['hg', 'id', '-n', '--config', 'defaults.id=', self.tempdir]
@@ -98,33 +105,36 @@
             build += '-beta'
         return build
 
     def getChanges(self):
         """
           retrieve changes between the current and previous ("first") revision
         """
 
-        command = ['hg', 'log', '-R', self.tempdir, '-r', 'tip:0',
-                   '-b', 'default', '-l', '50', '--encoding', 'utf-8',
-                   '--template', '{date|isodate}\\0{author|person}\\0{rev}\\0{desc}\\0\\0',
-                   '--config', 'defaults.log=']
+        command = [
+            'hg', 'log', '-R', self.tempdir, '-r', self.bookmark + ':0',
+            '-b', 'default', '-l', '50', '--encoding', 'utf-8', '--template',

[Wladimir Palant, 2016/10/25 16:32:04]

This is actually tricky now. The purpose of the `-b default` parameter is
selecting only commits from the default branch. However, we are in the process
of adding anonymous branches on the default branch - this restriction will no
longer help. What we want here is actual ancestors of the bookmark. From the
look of it, `hg log -r ancestors(master)` does the trick in modern Mercurial
versions, I didn't really test it thoroughly however.

[Jon Sonesen, 2016/10/25 16:39:58]

Are you saying I should literally change it to `hg log -r ancestors(master)`  or
is the is it ancestors(branch_name)?

[Wladimir Palant, 2016/10/26 15:34:03]

It should be ancestors({}) of course, with {} replaced by the revision
identifier.

+            '{date|isodate}\\0{author|person}\\0{rev}\\0{desc}\\0\\0',
+            '--config', 'defaults.log='
+        ]
         result = subprocess.check_output(command).decode('utf-8')
 
         for change in result.split('\x00\x00'):
             if change:
                 date, author, revision, description = change.split('\x00')
                 yield {'date': date, 'author': author, 'revision': revision, 'description': description}
 
     def copyRepository(self):
         """
           Create a repository copy in a temporary directory
         """
         self.tempdir = tempfile.mkdtemp(prefix=self.config.repositoryName)
-        command = ['hg', 'clone', '-q', self.config.repository, '-u', 'default', self.tempdir]
+        command = ['hg', 'clone', '-q', self.config.repository,  '-u',
+                   'default', self.tempdir]

[Wladimir Palant, 2016/10/25 16:32:04]

Shouldn't this update to the bookmark?

[Jon Sonesen, 2016/10/25 16:39:58]

I am not sure, are you saying I should add the -r bookmark option?

         subprocess.check_call(command)
 
         # Make sure to run ensure_dependencies.py if present
         depscript = os.path.join(self.tempdir, 'ensure_dependencies.py')
         if os.path.isfile(depscript):
             subprocess.check_call([sys.executable, depscript, '-q'])
 
     def writeChangelog(self, changes):