Issue 4354, 4355 - handle dirty/outdated repos on release

releaseAutomation.py

Index: releaseAutomation.py
diff --git a/releaseAutomation.py b/releaseAutomation.py
index 5578abf1d948fe185e046cdb70ccd4734fa876ed..f66f5b44d96c8f8f8bf6a63f2e0461b486fcb928 100644
--- a/releaseAutomation.py
+++ b/releaseAutomation.py
@@ -2,6 +2,8 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+from __future__ import print_function
+
 import os
 import re
 import codecs
@@ -45,7 +47,78 @@ def create_sourcearchive(repo, output):
                 process.wait()
 
 
+def repo_has_uncommitted():
+    """Checks if the given repository is clean"""
+    buff = subprocess.check_output(['hg', 'status'])
+
+    if len(buff):
+        print('Dirty / uncommitted changes in repository!')
+        return True
+
+    return False
+
+
+def repo_has_outgoing():
+    """Checks whether there would be outgoing changesets to the given path"""
+    try:
+        subprocess.check_output(['hg', 'outgoing'])
+        print('Detected outgoing changesets!')
+        return True
+    except subprocess.CalledProcessError as e:
+        if e.returncode == 1:
+            return False
+        raise
+
+
+def repo_has_incoming(repo_paths):
+    """Checks whether the local repositories are up-to-date"""
+    incoming = False
+
+    for repo_path in repo_paths:
+        try:
+            subprocess.check_output(['hg', 'incoming', '-R', repo_path])
+            print('Detected incoming changesets in "{}"'.format(repo_path))
+            incoming = True

[Wladimir Palant, 2017/08/16 10:46:09]

This seems inconsistent compared to repo_has_outgoing() above, I guess you want
`return True` without any additional variables here as well?

[tlucas, 2017/08/16 11:05:19]

If i returned immediately on the first occurrence of incoming changesets the
user would not be notified about incoming changesets of repositories, that might
be checked later. (e.g. if both the extension itself and the
downloads-repository have incoming changesets). Imho that missing information
would not be user-friendly and i would rather check for outgoing changesets for
all involved repositories as well - what do you think about this?

[Wladimir Palant, 2017/08/16 11:11:46]

I see, I overlooked that difference between the two functions. However, does
this even work? This function requires a list of repositories as parameter, yet
the caller isn't passing in any parameters.

[tlucas, 2017/08/16 11:27:54]

You are right, re-added the correct parameters to the callers / callees 

+        except subprocess.CalledProcessError as e:
+            if e.returncode != 1:
+                raise
+
+    return incoming
+
+
+def continue_with_outgoing():
+    """Asks the user if he wants to continue despite facing warnings"""

[Wladimir Palant, 2017/08/16 10:46:09]

Please formulate this in a gender-neutral way, "they want" - we might have
female developers run this as well ;)

[tlucas, 2017/08/16 11:05:18]

Alright - i didn't want to leave anyone out :)

[tlucas, 2017/08/16 11:27:54]

Done.

+
+    print('If you proceed with the release, they will be included in the '
+          'release and pushed.')
+    print('Are you sure about continuing the release-process?')

[Wladimir Palant, 2017/08/16 10:46:08]

Nit: it's "release process"

[tlucas, 2017/08/16 11:05:18]

Acknowledged.

[tlucas, 2017/08/16 11:27:54]

Done.

+
+    while True:
+        choice = raw_input('Please choose (yes / no): ').lower()

[Wladimir Palant, 2017/08/16 10:46:08]

Maybe strip whitespace as well?

[tlucas, 2017/08/16 11:05:18]

Acknowledged.

[tlucas, 2017/08/16 11:27:54]

Done.

+
+        if choice == 'yes':
+            return True
+        if choice == 'no':
+            return False
+
+        print('Please answer "yes" or "no"!')

[Sebastian Noack, 2017/08/16 10:33:29]

This message seems redundant. If we don't return and jump back to the head of
the loop 'Please choose (yes / no): ' is already printed, again.

[tlucas, 2017/08/16 10:40:29]

Right, missed that. Done.

+
+
+def can_safely_release():
+    """Run repository-checks in order to bail out early if necessary"""
+    if repo_has_uncommitted():
+        return False
+    if repo_has_incoming():
+        return False
+    if repo_has_outgoing():
+        return continue_with_outgoing()
+
+
 def run(baseDir, type, version, keyFile, downloadsRepo):
+    if not can_safely_release():
+        print('Aborting release.')
+        return 1
+
     if type == 'gecko':
         import buildtools.packagerGecko as packager
     elif type == 'safari':