Cleanup git-backup lock and gitlab-backup tempporary folder in defer-style

So that further `git-backup pull` run can proceed, and many gitlab-backup's left `$tmpd` don't eat up disk space.

Rework of !4.

/reviewed-on !5

contrib/gitlab-backup

 #!/bin/bash -e
 # pull/restore gitlab data into/from git-backup
-# 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
@@ -110,6 +110,8 @@ backup_pull() {
     # 1. dump all gitlab data except repositories & db
     echo " * Dumping gitlab data (except repositories & db)"
     tmpd=$(mktemp -d `pwd`/gitlab-backup.XXXXXX)
+    trap 'rm -rf "$tmpd"' EXIT
+
     gitlab-rake gitlab:backup:create SKIP=repositories,db | tee "$tmpd/gitlab_backup_create.out"
     backup_tar=`grep "^Creating backup archive: .* done" "$tmpd/gitlab_backup_create.out"` || \
         die "E: Cannot detect backup tar"
@@ -218,7 +220,6 @@ backup_pull() {
         # mark backup_tar as pulled and cleanup
         mv "$backup_tar" "$backup_tar.pulled"
     fi
-    rm -rf "$tmpd"
 
     echo OK
 }
@@ -235,6 +236,7 @@ backup_restore() {
     # 1. extract all gitlab data except repositories
     echo " * Extracting gitlab data (except repositories)"
     tmpd=$(mktemp -d `pwd`/gitlab-backup.XXXXXX)
+    trap 'rm -rf "$tmpd"' EXIT
 
     $GIT_BACKUP restore $HEAD gitlab/misc:"$tmpd/gitlab_backup"
     backup_info="$tmpd/gitlab_backup/backup_information.yml"
@@ -308,8 +310,6 @@ backup_restore() {
     test -e "$backup_tar" && die "E: $backup_tar already exists"
     tar -C "$tmpd/gitlab_backup" -cf "$backup_tar" .
 
-    rm -rf "$tmpd"  # tmpd no longer needed
-
     # 4. extract repositories into .../repositories.<timestamp>
     echo " * Extracting repositories"
     reposX="${GITLAB_REPOS_PATH}.${backup_created_at}"

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
@@ -20,6 +20,7 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -68,9 +69,16 @@ func xgittype(s string) git.ObjectType {
 	return type_
 }
 
+// xnoref asserts that git reference ref does not exists.
+func xnoref(ref string) {
+	xgit(context.Background(), "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) {
+	ctx := context.Background()
+
 	// if something raises -> don't let testing panic - report it as proper error with context.
 	here := my.FuncName()
 	defer exc.Catch(func(e *exc.Error) {
@@ -109,7 +117,7 @@ func TestPullRestore(t *testing.T) {
 	}
 
 	// init backup repository
-	xgit("init", "--bare", "backup.git")
+	xgit(ctx, "init", "--bare", "backup.git")
 	xchdir(t, "backup.git")
 	gb, err := git.OpenRepository(".")
 	if err != nil {
@@ -118,10 +126,10 @@ func TestPullRestore(t *testing.T) {
 
 	// pull from testdata
 	my0 := mydir + "/testdata/0"
-	cmd_pull(gb, []string{my0 + ":b0"}) // only empty repo in testdata/0
+	cmd_pull(ctx, gb, []string{my0 + ":b0"}) // only empty repo in testdata/0
 
 	my1 := mydir + "/testdata/1"
-	cmd_pull(gb, []string{my1 + ":b1"})
+	cmd_pull(ctx, gb, []string{my1 + ":b1"})
 
 	// verify tag/tree/blob encoding is 1) consistent and 2) always the same.
 	// we need it be always the same so different git-backup versions can
@@ -153,8 +161,8 @@ func TestPullRestore(t *testing.T) {
 		}
 
 		// encoding original object should give sha1_
-		obj_type := xgit("cat-file", "-t", nc.sha1)
-		sha1_ := obj_represent_as_commit(gb, nc.sha1, xgittype(obj_type))
+		obj_type := xgit(ctx, "cat-file", "-t", nc.sha1)
+		sha1_ := obj_represent_as_commit(ctx, gb, nc.sha1, xgittype(obj_type))
 		if sha1_ != nc.sha1_ {
 			t.Fatalf("encode %s -> %s ;  want %s", sha1, sha1_, nc.sha1_)
 		}
@@ -176,10 +184,10 @@ func TestPullRestore(t *testing.T) {
 		}
 
 		// prune all non-reachable objects (e.g. tags just pulled - they were encoded as commits)
-		xgit("prune")
+		xgit(ctx, "prune")
 
 		// verify backup repo is all ok
-		xgit("fsck")
+		xgit(ctx, "fsck")
 
 		// verify that just pulled tag objects are now gone after pruning -
 		// - they become not directly git-present. The only possibility to
@@ -188,7 +196,7 @@ func TestPullRestore(t *testing.T) {
 			if !nc.istag {
 				continue
 			}
-			gerr, _, _ := ggit("cat-file", "-p", nc.sha1)
+			gerr, _, _ := ggit(ctx, "cat-file", "-p", nc.sha1)
 			if gerr == nil {
 				t.Fatalf("tag %s still present in backup.git after git-prune", nc.sha1)
 			}
@@ -205,14 +213,14 @@ func TestPullRestore(t *testing.T) {
 	afterPull()
 
 	// pull again - it should be noop
-	h1 := xgitSha1("rev-parse", "HEAD")
-	cmd_pull(gb, []string{my1 + ":b1"})
+	h1 := xgitSha1(ctx, "rev-parse", "HEAD")
+	cmd_pull(ctx, gb, []string{my1 + ":b1"})
 	afterPull()
-	h2 := xgitSha1("rev-parse", "HEAD")
+	h2 := xgitSha1(ctx, "rev-parse", "HEAD")
 	if h1 == h2 {
 		t.Fatal("pull: second run did not ajusted HEAD")
 	}
-	δ12 := xgit("diff", h1, h2)
+	δ12 := xgit(ctx, "diff", h1, h2)
 	if δ12 != "" {
 		t.Fatalf("pull: second run was not noop: δ:\n%s", δ12)
 	}
@@ -220,10 +228,10 @@ func TestPullRestore(t *testing.T) {
 
 	// restore backup
 	work1 := workdir + "/1"
-	cmd_restore(gb, []string{"HEAD", "b1:" + work1})
+	cmd_restore(ctx, gb, []string{"HEAD", "b1:" + work1})
 
 	// verify files restored to the same as original
-	gerr, diff, _ := ggit("diff", "--no-index", "--raw", "--exit-code", my1, work1)
+	gerr, diff, _ := ggit(ctx, "diff", "--no-index", "--raw", "--exit-code", my1, work1)
 	// 0 - no diff, 1 - has diff, 2 - problem
 	if gerr != nil && gerr.Sys().(syscall.WaitStatus).ExitStatus() > 1 {
 		t.Fatal(gerr)
@@ -262,12 +270,12 @@ func TestPullRestore(t *testing.T) {
 
 		for _, repo := range R {
 			// fsck just in case
-			xgit("--git-dir="+repo.path, "fsck")
+			xgit(ctx, "--git-dir="+repo.path, "fsck")
 			// NOTE for-each-ref sorts output by refname
-			repo.reflist = xgit("--git-dir="+repo.path, "for-each-ref")
+			repo.reflist = xgit(ctx, "--git-dir="+repo.path, "for-each-ref")
 			// NOTE rev-list emits objects in reverse chronological order,
 			//      starting from refs roots which are also ordered by refname
-			repo.revlist = xgit("--git-dir="+repo.path, "rev-list", "--all", "--objects")
+			repo.revlist = xgit(ctx, "--git-dir="+repo.path, "rev-list", "--all", "--objects")
 		}
 
 		if R[0].reflist != R[1].reflist {
@@ -292,11 +300,11 @@ 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"})
+		cmd_pull(ctx, gb, []string{my2 + ":b2"})
 		t.Fatal("pull corrupt.git: did not complain")
 	}()
 
@@ -318,8 +326,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
@@ -336,7 +344,7 @@ func TestPullRestore(t *testing.T) {
 		err = os.Setenv("HOME", my3+"/incomplete-send-pack.git/"+kind)
 		exc.Raiseif(err)
 
-		cmd_pull(gb, []string{my3 + ":b3"})
+		cmd_pull(ctx, gb, []string{my3 + ":b3"})
 		t.Fatalf("pull incomplete-send-pack.git/%s: did not complain", kind)
 	}
 
@@ -353,7 +361,7 @@ func TestPullRestore(t *testing.T) {
 
 	// pulling incomplete-send-pack.git without pack-objects hook must succeed:
 	// without $HOME tweaks full and complete pack is sent.
-	cmd_pull(gb, []string{my3 + ":b3"})
+	cmd_pull(ctx, gb, []string{my3 + ":b3"})
 }
 
 func TestRepoRefSplit(t *testing.T) {

git.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
@@ -22,6 +22,7 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"os"
 	"os/exec"
@@ -48,18 +49,20 @@ type RunWith struct {
 }
 
 // run `git *argv` -> error, stdout, stderr
-func _git(argv []string, ctx RunWith) (err error, stdout, stderr string) {
+func _git(ctx context.Context, argv []string, rctx RunWith) (err error, stdout, stderr string) {
 	debugf("git %s", strings.Join(argv, " "))
 
-	cmd := exec.Command("git", argv...)
+	// XXX exec.CommandContext does `kill -9` on ctx cancel
+	// XXX -> rework to `kill -TERM` so that spawned process can finish cleanly?
+	cmd := exec.CommandContext(ctx, "git", argv...)
 	stdoutBuf := bytes.Buffer{}
 	stderrBuf := bytes.Buffer{}
 
-	if ctx.stdin != "" {
-		cmd.Stdin = strings.NewReader(ctx.stdin)
+	if rctx.stdin != "" {
+		cmd.Stdin = strings.NewReader(rctx.stdin)
 	}
 
-	switch ctx.stdout {
+	switch rctx.stdout {
 	case PIPE:
 		cmd.Stdout = &stdoutBuf
 	case DontRedirect:
@@ -68,7 +71,7 @@ func _git(argv []string, ctx RunWith) (err error, stdout, stderr string) {
 		panic("git: stdout redirect mode invalid")
 	}
 
-	switch ctx.stderr {
+	switch rctx.stderr {
 	case PIPE:
 		cmd.Stderr = &stderrBuf
 	case DontRedirect:
@@ -77,9 +80,9 @@ func _git(argv []string, ctx RunWith) (err error, stdout, stderr string) {
 		panic("git: stderr redirect mode invalid")
 	}
 
-	if ctx.env != nil {
+	if rctx.env != nil {
 		env := []string{}
-		for k, v := range ctx.env {
+		for k, v := range rctx.env {
 			env = append(env, k+"="+v)
 		}
 		cmd.Env = env
@@ -89,7 +92,7 @@ func _git(argv []string, ctx RunWith) (err error, stdout, stderr string) {
 	stdout = mem.String(stdoutBuf.Bytes())
 	stderr = mem.String(stderrBuf.Bytes())
 
-	if !ctx.raw {
+	if !rctx.raw {
 		// prettify stdout (e.g. so that 'sha1\n' becomes 'sha1' and can be used directly
 		stdout = strings.TrimSpace(stdout)
 		stderr = strings.TrimSpace(stderr)
@@ -138,9 +141,9 @@ func (e *GitErrContext) Error() string {
 	return msg
 }
 
-// argv -> []string, ctx    (for passing argv + RunWith handy - see ggit() for details)
-func _gitargv(argv ...interface{}) (argvs []string, ctx RunWith) {
-	ctx_seen := false
+// ctx, argv -> ctx, []string, rctx  (for passing argv + RunWith handy - see ggit() for details)
+func _gitargv(ctx context.Context, argv ...interface{}) (_ context.Context, argvs []string, rctx RunWith) {
+	rctx_seen := false
 
 	for _, arg := range argv {
 		switch arg := arg.(type) {
@@ -149,47 +152,47 @@ func _gitargv(argv ...interface{}) (argvs []string, ctx RunWith) {
 		default:
 			argvs = append(argvs, fmt.Sprint(arg))
 		case RunWith:
-			if ctx_seen {
+			if rctx_seen {
 				panic("git: multiple RunWith contexts")
 			}
-			ctx, ctx_seen = arg, true
+			rctx, rctx_seen = arg, true
 		}
 	}
 
-	return argvs, ctx
+	return ctx, argvs, rctx
 }
 
 // run `git *argv` -> err, stdout, stderr
 // - arguments are automatically converted to strings
-// - RunWith argument is passed as ctx
+// - RunWith argument is passed as rctx
 // - error is returned only when git command could run and exits with error status
 // - on other errors - exception is raised
 //
 // NOTE err is concrete *GitError, not error
-func ggit(argv ...interface{}) (err *GitError, stdout, stderr string) {
-	return ggit2(_gitargv(argv...))
+func ggit(ctx context.Context, argv ...interface{}) (err *GitError, stdout, stderr string) {
+	return ggit2(_gitargv(ctx, argv...))
 }
 
-func ggit2(argv []string, ctx RunWith) (err *GitError, stdout, stderr string) {
-	e, stdout, stderr := _git(argv, ctx)
+func ggit2(ctx context.Context, argv []string, rctx RunWith) (err *GitError, stdout, stderr string) {
+	e, stdout, stderr := _git(ctx, argv, rctx)
 	eexec, _ := e.(*exec.ExitError)
 	if e != nil && eexec == nil {
 		exc.Raisef("git %s : %s", strings.Join(argv, " "), e)
 	}
 	if eexec != nil {
-		err = &GitError{GitErrContext{argv, ctx.stdin, stdout, stderr}, eexec}
+		err = &GitError{GitErrContext{argv, rctx.stdin, stdout, stderr}, eexec}
 	}
 	return err, stdout, stderr
 }
 
 // run `git *argv` -> stdout
 // on error - raise exception
-func xgit(argv ...interface{}) string {
-	return xgit2(_gitargv(argv...))
+func xgit(ctx context.Context, argv ...interface{}) string {
+	return xgit2(_gitargv(ctx, argv...))
 }
 
-func xgit2(argv []string, ctx RunWith) string {
-	gerr, stdout, _ := ggit2(argv, ctx)
+func xgit2(ctx context.Context, argv []string, rctx RunWith) string {
+	gerr, stdout, _ := ggit2(ctx, argv, rctx)
 	if gerr != nil {
 		exc.Raise(gerr)
 	}
@@ -197,8 +200,8 @@ func xgit2(argv []string, ctx RunWith) string {
 }
 
 // like xgit(), but automatically parse stdout to Sha1
-func xgitSha1(argv ...interface{}) Sha1 {
-	return xgit2Sha1(_gitargv(argv...))
+func xgitSha1(ctx context.Context, argv ...interface{}) Sha1 {
+	return xgit2Sha1(_gitargv(ctx, argv...))
 }
 
 // error when git output is not valid sha1
@@ -212,14 +215,14 @@ func (e *GitSha1Error) Error() string {
 	return msg
 }
 
-func xgit2Sha1(argv []string, ctx RunWith) Sha1 {
-	gerr, stdout, stderr := ggit2(argv, ctx)
+func xgit2Sha1(ctx context.Context, argv []string, rctx RunWith) Sha1 {
+	gerr, stdout, stderr := ggit2(ctx, argv, rctx)
 	if gerr != nil {
 		exc.Raise(gerr)
 	}
 	sha1, err := Sha1Parse(stdout)
 	if err != nil {
-		exc.Raise(&GitSha1Error{GitErrContext{argv, ctx.stdin, stdout, stderr}})
+		exc.Raise(&GitSha1Error{GitErrContext{argv, rctx.stdin, stdout, stderr}})
 	}
 	return sha1
 }

gitobjects.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
@@ -21,6 +21,7 @@ package main
 // Git-backup | Git object: Blob Tree Commit Tag
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"os"
@@ -163,9 +164,9 @@ func (e *InvalidLstreeEntry) Error() string {
 
 // create empty git tree -> tree sha1
 var tree_empty Sha1
-func mktree_empty() Sha1 {
+func mktree_empty(ctx context.Context) Sha1 {
 	if tree_empty.IsNull() {
-		tree_empty = xgitSha1("mktree", RunWith{stdin: ""})
+		tree_empty = xgitSha1(ctx, "mktree", RunWith{stdin: ""})
 	}
 	return tree_empty
 }