Merge pull request #356 from michael-k/track

Fixed some tracking issues and improved codebase of command track

commands/command_track.go

@@ -47,6 +47,7 @@ func trackCommand(cmd *cobra.Command, args []string) {
 		Print("Error opening .gitattributes file")
 		return
 	}
+	defer attributesFile.Close()
 
 	if addTrailingLinebreak {
 		if _, err := attributesFile.WriteString("\n"); err != nil {
@@ -54,21 +55,26 @@ func trackCommand(cmd *cobra.Command, args []string) {
 		}
 	}
 
+	wd, _ := os.Getwd()
+
+ArgsLoop:
 	for _, t := range args {
-		isKnownPath := false
+		absT, relT := absRelPath(t, wd)
+
+		if !filepath.HasPrefix(absT, lfs.LocalWorkingDir) {
+			Print("%s is outside repository", t)
+			os.Exit(128)
+		}
+
 		for _, k := range knownPaths {
-			if t == k.Path {
-				isKnownPath = true
-				break
+			absK, _ := absRelPath(k.Path, filepath.Join(wd, filepath.Dir(k.Source)))
+			if absT == absK {
+				Print("%s already supported", t)
+				continue ArgsLoop
 			}
 		}
 
-		if isKnownPath {
-			Print("%s already supported", t)
-			continue
-		}
-
-		encodedArg := strings.Replace(t, " ", "[[:space:]]", -1)
+		encodedArg := strings.Replace(relT, " ", "[[:space:]]", -1)
 		_, err := attributesFile.WriteString(fmt.Sprintf("%s filter=lfs diff=lfs merge=lfs -crlf\n", encodedArg))
 		if err != nil {
 			Print("Error adding path %s", t)
@@ -76,8 +82,6 @@ func trackCommand(cmd *cobra.Command, args []string) {
 		}
 		Print("Tracking %s", t)
 	}
-
-	attributesFile.Close()
 }
 
 type mediaPath struct {
@@ -85,11 +89,35 @@ type mediaPath struct {
 	Source string
 }
 
+func findPaths() []mediaPath {
+	paths := make([]mediaPath, 0)
+	wd, _ := os.Getwd()
+
+	for _, path := range findAttributeFiles() {
+		attributes, err := os.Open(path)
+		if err != nil {
+			continue
+		}
+
+		scanner := bufio.NewScanner(attributes)
+		for scanner.Scan() {
+			line := scanner.Text()
+			if strings.Contains(line, "filter=lfs") {
+				fields := strings.Fields(line)
+				relPath, _ := filepath.Rel(wd, path)
+				paths = append(paths, mediaPath{Path: fields[0], Source: relPath})
+			}
+		}
+	}
+
+	return paths
+}
+
 func findAttributeFiles() []string {
 	paths := make([]string, 0)
 
 	repoAttributes := filepath.Join(lfs.LocalGitDir, "info", "attributes")
-	if _, err := os.Stat(repoAttributes); err == nil {
+	if info, err := os.Stat(repoAttributes); err == nil && !info.IsDir() {
 		paths = append(paths, repoAttributes)
 	}
 
@@ -107,34 +135,6 @@ func findAttributeFiles() []string {
 	return paths
 }
 
-func findPaths() []mediaPath {
-	paths := make([]mediaPath, 0)
-	wd, _ := os.Getwd()
-
-	for _, path := range findAttributeFiles() {
-		attributes, err := os.Open(path)
-		if err != nil {
-			return paths
-		}
-
-		scanner := bufio.NewScanner(attributes)
-		for scanner.Scan() {
-			line := scanner.Text()
-			if line == "" {
-				continue
-			}
-
-			if strings.Contains(line, "filter=lfs") {
-				fields := strings.Fields(line)
-				relPath, _ := filepath.Rel(wd, path)
-				paths = append(paths, mediaPath{Path: fields[0], Source: relPath})
-			}
-		}
-	}
-
-	return paths
-}
-
 func needsTrailingLinebreak(filename string) bool {
 	file, err := os.Open(filename)
 	if err != nil {
@@ -157,6 +157,18 @@ func needsTrailingLinebreak(filename string) bool {
 	return !strings.HasSuffix(string(buf[0:bytesRead]), "\n")
 }
 
+// absRelPath takes a path and a working directory and
+// returns an absolute and a relative representation of path based on the working directory
+func absRelPath(path, wd string) (string, string) {
+	if filepath.IsAbs(path) {
+		relPath, _ := filepath.Rel(wd, path)
+		return path, relPath
+	}
+
+	absPath := filepath.Join(wd, path)
+	return absPath, path
+}
+
 func init() {
 	RootCmd.AddCommand(trackCmd)
 }

commands/command_track_test.go

@@ -0,0 +1,35 @@
+package commands
+
+import "testing"
+
+type checker struct {
+	*testing.T
+}
+
+func TestAbsRelPath(t *testing.T) {
+	c := &checker{t}
+
+	ae := "/home/test"
+	re := "test"
+	a, r := absRelPath("test", "/home")
+	c.Check(a, ae)
+	c.Check(r, re)
+
+	ae = "/home/test/some/directory/with/a/file.txt"
+	re = "some/directory/with/a/file.txt"
+	a, r = absRelPath("/home/test/some/directory/with/a/file.txt", "/home/test")
+	c.Check(a, ae)
+	c.Check(r, re)
+
+	ae = "/home/test/some/directory/with/a/file.txt"
+	re = "some/directory/with/a/file.txt"
+	a, r = absRelPath("some/directory/with/a/file.txt", "/home/test")
+	c.Check(a, ae)
+	c.Check(r, re)
+}
+
+func (c *checker) Check(g, e string) {
+	if g != e {
+		c.Errorf("Expected %s got %s", e, g)
+	}
+}

test/test-track.sh

@@ -105,6 +105,86 @@ begin_test "track outside git repo"
     echo "GIT_LFS_TEST_DIR should be set outside of any Git repository"
     exit 1
   fi
+
+  git init track-outside
+  cd track-outside
+
+  git lfs track "*.file"
+
+  git lfs track "../*.foo" || {
+
+    # git itself returns an exit status of 128
+    # $ git add ../test.foo
+    # fatal: ../test.foo: '../test.foo' is outside repository
+    # $ echo "$?"
+    # 128
+
+    [ "$?" = "128" ]
+    exit 0
+  }
+  exit 1
+)
+end_test
+
+begin_test "track representation"
+(
+  set -e
+
+  git init track-representation
+  cd track-representation
+
+  git lfs track "*.jpg"
+  out=$(git lfs track "$PWD/*.jpg")
+
+  if [ "$out" != "$PWD/*.jpg already supported" ]; then
+    echo "Track didn't recognize duplicate path"
+    cat .gitattributes
+    exit 1
+  fi
+
+  out2=$(git lfs track "a/../*.jpg")
+
+  if [ "$out2" != "a/../*.jpg already supported" ]; then
+    echo "Track didn't recognize duplicate path"
+    cat .gitattributes
+    exit 1
+  fi
+
+  mkdir a
+  git lfs track "a/test.file"
+  cd a
+  out3=$(git lfs track "test.file")
+
+  if [ "$out3" != "test.file already supported" ]; then
+    echo "Track didn't recognize duplicate path"
+    cat .gitattributes
+    exit 1
+  fi
+
+  git lfs track "file.bin"
+  cd ..
+  out4=$(git lfs track "a/file.bin")
+  if [ "$out4" != "a/file.bin already supported" ]; then
+    echo "Track didn't recognize duplicate path"
+    cat .gitattributes
+    exit 1
+  fi
+)
+end_test
+
+begin_test "track absolute"
+(
+  set -e
+
+  git init track-absolute
+  cd track-absolute
+
+  git lfs track "$PWD/*.jpg"
+  grep "^*.jpg" .gitattributes || {
+    echo ".gitattributes doesn't contain the expected relative path *.jpg:"
+    cat .gitattributes
+    exit 1
+  }
 )
 end_test