Merge pull request #13 from grdl/bugfix/9

Cleanup empty directories created by a failed git clone

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/cobra v1.0.0
 	github.com/spf13/viper v1.7.0
-	github.com/stretchr/testify v1.4.0
+	github.com/stretchr/testify v1.6.0
 	github.com/xlab/treeprint v1.0.0
 	golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 // indirect
 	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect

go.sum

@@ -194,8 +194,8 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
-github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
+github.com/stretchr/testify v1.6.0 h1:jlIyCplCJFULU/01vCkhKuTyc3OorI3bJFuw6obfgho=
-github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+github.com/stretchr/testify v1.6.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
@@ -335,6 +335,8 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

pkg/git/repo.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"git-get/pkg/run"
 	"net/url"
+	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 )
@@ -54,6 +56,7 @@ func Clone(opts *CloneOpts) (*Repo, error) {
 	}
 
 	if err != nil {
+		cleanupFailedClone(opts.Path)
 		return nil, err
 	}
 
@@ -183,3 +186,21 @@ func (r *Repo) Remote() (string, error) {
 func (r *Repo) Path() string {
 	return r.path
 }
+
+// cleanupFailedClone removes empty directories created by git when coning failed.
+// Git itself will delete the final repo directory if a clone failed,
+// but it won't delete all the parent dirs that it created when cloning the repo.
+// eg:
+// When operation like `git clone https://github.com/grdl/git-get /tmp/some/temp/dir/git-get` fails,
+// git will only delete the final `git-get` dir in the path an will leave /tmp/some/temp/dir even if it just created them.
+//
+// os.Remove will only delete an empty dir so we traverse the path "upwards" and delete all directories
+// until a non-empty one is reached.
+func cleanupFailedClone(path string) {
+	for {
+		path = filepath.Dir(path)
+		if err := os.Remove(path); err != nil {
+			return
+		}
+	}
+}

pkg/git/repo_test.go

@@ -1,9 +1,14 @@
 package git
 
 import (
+	"fmt"
 	"git-get/pkg/git/test"
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestUncommitted(t *testing.T) {
@@ -299,3 +304,70 @@ func TestAheadBehind(t *testing.T) {
 		})
 	}
 }
+
+func TestCleanupFailedClone(t *testing.T) {
+	// Test dir structure:
+	// root
+	// └── a/
+	//     ├── b/
+	//     │   └── c/
+	//     └── x/
+	//         └── y/
+	//        	   └── file.txt
+
+	tests := []struct {
+		path     string // path to cleanup
+		wantGone string // this path should be deleted, if empty - nothing should be deleted
+		wantStay string // this path shouldn't be deleted
+	}{
+		{
+			path:     "a/b/c/repo",
+			wantGone: "a/b/c/repo",
+			wantStay: "a",
+		}, {
+			path:     "a/b/c/repo",
+			wantGone: "a/b",
+			wantStay: "a",
+		}, {
+			path:     "a/b/repo",
+			wantGone: "",
+			wantStay: "a/b/c",
+		}, {
+			path:     "a/x/y/repo",
+			wantGone: "",
+			wantStay: "a/x/y",
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			root := createTestDirTree(t)
+
+			path := filepath.Join(root, test.path)
+			cleanupFailedClone(path)
+
+			if test.wantGone != "" {
+				wantGone := filepath.Join(root, test.wantGone)
+				assert.NoDirExists(t, wantGone, "%s dir should be deleted during the cleanup", wantGone)
+			}
+
+			if test.wantStay != "" {
+				wantLeft := filepath.Join(root, test.wantStay)
+				assert.DirExists(t, wantLeft, "%s dir should not be deleted during the cleanup", wantLeft)
+			}
+		})
+	}
+}
+
+func createTestDirTree(t *testing.T) string {
+	root := test.TempDir(t, "")
+	err := os.MkdirAll(filepath.Join(root, "a", "b", "c"), os.ModePerm)
+	err = os.MkdirAll(filepath.Join(root, "a", "x", "y"), os.ModePerm)
+	_, err = os.Create(filepath.Join(root, "a", "x", "y", "file.txt"))
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return root
+}