gitclone: let errors on update propagate

Update use to tolerate errors in develop mode, to prevent buildout from
deleting develop working copies if anything happens, then this behavior was
extended to non-develop mode, apparently by mistake.

In non-develop mode, we want error to propagate, if something went wrong with
updating this repository, buildout should delete and retry.

slapos/recipe/build/tests.py

@@ -10,7 +10,7 @@ import zc.buildout.testing
 from zc.buildout.testing import buildoutTearDown
 from contextlib import contextmanager
 from functools import wraps
-from subprocess import check_call, check_output, CalledProcessError
+from subprocess import check_call, check_output, CalledProcessError, STDOUT
 from slapos.recipe.gitclone import GIT_CLONE_ERROR_MESSAGE, \
     GIT_CLONE_CACHE_ERROR_MESSAGE
 from slapos.recipe.downloadunpacked import make_read_only_recursively
@@ -492,6 +492,36 @@ repository = %s
   def test_ignore_cloning_submodules(self):
     self.test_clone_submodules_by_default(ignore_cloning_submodules=True)
 
+  @with_buildout
+  def test_reset_on_update_failure(self, buildout, sample_buildout, write, **kw):
+    self.createRepositoriesAndConnect()
+
+    write(sample_buildout, 'buildout.cfg',
+"""
+[buildout]
+parts = git-clone
+
+[git-clone]
+recipe = slapos.recipe.build:gitclone
+repository = %s
+""" % self.project_dir)
+    check_call([buildout])
+
+    check_call(
+        ['git', 'remote', 'add', 'broken', 'http://git.erp5.org/repos/nowhere'],
+        cwd=os.path.join(sample_buildout, "parts", "git-clone"))
+
+    with self.assertRaises(CalledProcessError) as output:
+      check_output([buildout], stderr=STDOUT)
+    self.assertIn(
+        b"error: Could not fetch broken",
+        output.exception.output)
+
+    # this reset repo
+    self.assertFalse(os.path.exists(os.path.join(sample_buildout, "parts", "git-clone")))
+    # and running buildout again succeeed
+    check_call([buildout])
+
 
 class MakeReadOnlyTests(unittest.TestCase):
 

slapos/recipe/gitclone.py

@@ -280,31 +280,25 @@ class Recipe(object):
     # If develop or revision parameter, no need to update
     if self.develop or self.revision:
       return
-    try:
-      # first cleanup pyc files
-      self.deletePycFiles(self.location)
 
-      # Fetch and reset to the upstream
-      check_call([self.git_command, 'fetch', '--all'], cwd=self.location)
-      self.gitReset('@{upstream}')
+    # first cleanup pyc files
+    self.deletePycFiles(self.location)
 
-      if not self.ignore_cloning_submodules:
-        # Update the submodule to the commit which parent repo points to if
-        # there has been revision is present for the parent repo.
-        # According to man-page of submodule, update also clones missing
-        # submodules and updating the working tree of the submodules. This is
-        # why we do update here only after we are ok with revision of parent
-        # repo being checked out to the desired one.
-        # It will also init a submodule if required
-        # NOTE: This will put the submodule repo in a `Detached` state.
-        check_call([self.git_command, 'submodule', 'update', '--init', '-f',
-                    '--recursive'], cwd=self.location)
+    # Fetch and reset to the upstream
+    check_call([self.git_command, 'fetch', '--all'], cwd=self.location)
+    self.gitReset('@{upstream}')
 
-    except:
-      # Buildout will remove the installed location and mark the part as not
-      # installed if an error occurs during update. If we are developping this
-      # repository we do not want this to happen.
-      print('Unable to update:\n%s' % traceback.format_exc())
+    if not self.ignore_cloning_submodules:
+      # Update the submodule to the commit which parent repo points to if
+      # there has been revision is present for the parent repo.
+      # According to man-page of submodule, update also clones missing
+      # submodules and updating the working tree of the submodules. This is
+      # why we do update here only after we are ok with revision of parent
+      # repo being checked out to the desired one.
+      # It will also init a submodule if required
+      # NOTE: This will put the submodule repo in a `Detached` state.
+      check_call([self.git_command, 'submodule', 'update', '--init', '-f',
+                  '--recursive'], cwd=self.location)
 
 def uninstall(name, options):
   """Keep the working copy, unless develop is set to false.