pull: Don't leave backup repository locked on error

On pull git-backup locks backup repository to make sure another
concurrent `git-backup pull` process is not running. However until now,
if a pull was failing, the lock was left unreleased, which made followup
pull attempts to fail while acquiring the lock until the lock was
manually removed with `git update-ref -d ...`. Probably originally I
made it like this in 6f237f22 (git-backup: Initial draft) to make sure
that if there is a problem it does not go unnoticed and forces me to
investigate. But in general we do _not_ need to keep the lock on error
return after `git-backup pull` completes even abnormally.

This "lock left unreleased" is causing operational issues on
lab.nexedi.com from time to time: if a pull try fails for some, even
temporary, reason, all next pull tries will fail until a human intervene
and remove the lock ref.

Fix it.

See also: !4

git-backup.go

-// Copyright (C) 2015-2016  Nexedi SA and Contributors.
+// Copyright (C) 2015-2020  Nexedi SA and Contributors.
 //                          Kirill Smelkov <kirr@nexedi.com>
 //
 // This program is free software: you can Use, Study, Modify and Redistribute
@@ -371,10 +371,13 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) {
 	// unique work refs namespace.
 	backup_time := time.Now().Format("20060102-1504")               // %Y%m%d-%H%M
 	backup_refs_work := fmt.Sprintf("refs/backup/%s/", backup_time) // refs/backup/20150820-2109/
-	backup_lock := "refs/backup.locked"
 
-	// make sure another `git-backup pull` is not running
+	// prevent another `git-backup pull` from running simultaneously
+	backup_lock := "refs/backup.locked"
 	xgit("update-ref", backup_lock, mktree_empty(), Sha1{})
+	defer func() {
+		xgit("update-ref", "-d", backup_lock)
+	}()
 
 	// make sure there is root commit
 	var HEAD Sha1
@@ -654,9 +657,6 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) {
 			infof("%s", diffstat)
 		}
 	}
-
-	// we are done - unlock
-	xgit("update-ref", "-d", backup_lock)
 }
 
 // fetch makes sure all objects from a repository are present in backup place.

git-backup_test.go

-// Copyright (C) 2015-2016  Nexedi SA and Contributors.
+// Copyright (C) 2015-2020  Nexedi SA and Contributors.
 //                          Kirill Smelkov <kirr@nexedi.com>
 //
 // This program is free software: you can Use, Study, Modify and Redistribute
@@ -68,6 +68,11 @@ func xgittype(s string) git.ObjectType {
 	return type_
 }
 
+// xnoref asserts that git reference ref does not exists.
+func xnoref(ref string) {
+	xgit("update-ref", "--stdin", RunWith{stdin: fmt.Sprintf("verify refs/%s %s\n", ref, Sha1{})})
+}
+
 
 // verify end-to-end pull-restore
 func TestPullRestore(t *testing.T) {
@@ -292,8 +297,8 @@ func TestPullRestore(t *testing.T) {
 		defer exc.Catch(func(e *exc.Error) {
 			// it ok - pull should raise
 
-			// git-backup leaves backup repo locked on error
-			xgit("update-ref", "-d", "refs/backup.locked")
+			// git-backup should not leave backup repo locked on error
+			xnoref("backup.locked")
 		})
 
 		cmd_pull(gb, []string{my2 + ":b2"})
@@ -318,8 +323,8 @@ func TestPullRestore(t *testing.T) {
 				t.Fatalf("pull incomplete-send-pack.git/%s: complained, but error is wrong:\n%s\nerror: %s", kind, bad, estr)
 			}
 
-			// git-backup leaves backup repo locked on error
-			xgit("update-ref", "-d", "refs/backup.locked")
+			// git-backup should not leave backup repo locked on error
+			xnoref("backup.locked")
 		})
 
 		// for incomplete-send-pack.git to indeed send incomplete pack, its git