Merge branch 'master' into migrate-disambiguate-references

commands/lockverifier.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/git-lfs/git-lfs/config"
 	"github.com/git-lfs/git-lfs/errors"
+	"github.com/git-lfs/git-lfs/git"
 	"github.com/git-lfs/git-lfs/lfsapi"
 	"github.com/git-lfs/git-lfs/locking"
 	"github.com/git-lfs/git-lfs/tq"
@@ -23,7 +24,7 @@ const (
 
 func verifyLocksForUpdates(lv *lockVerifier, updates []*refUpdate) {
 	for _, update := range updates {
-		lv.Verify(update.Right().Name)
+		lv.Verify(update.Right())
 	}
 }
 
@@ -44,8 +45,8 @@ type lockVerifier struct {
 	unownedLocks []*refLock
 }
 
-func (lv *lockVerifier) Verify(ref string) {
+func (lv *lockVerifier) Verify(ref *git.Ref) {
-	if lv.verifyState == verifyStateDisabled || lv.verifiedRefs[ref] {
+	if lv.verifyState == verifyStateDisabled || lv.verifiedRefs[ref.Refspec()] {
 		return
 	}
 
@@ -76,10 +77,10 @@ func (lv *lockVerifier) Verify(ref string) {
 
 	lv.addLocks(ref, ours, lv.ourLocks)
 	lv.addLocks(ref, theirs, lv.theirLocks)
-	lv.verifiedRefs[ref] = true
+	lv.verifiedRefs[ref.Refspec()] = true
 }
 
-func (lv *lockVerifier) addLocks(ref string, locks []locking.Lock, set map[string]*refLock) {
+func (lv *lockVerifier) addLocks(ref *git.Ref, locks []locking.Lock, set map[string]*refLock) {
 	for _, l := range locks {
 		if rl, ok := set[l.Path]; ok {
 			if err := rl.Add(ref, l); err != nil {
@@ -136,11 +137,11 @@ func (lv *lockVerifier) Enabled() bool {
 	return lv.verifyState == verifyStateEnabled
 }
 
-func (lv *lockVerifier) newRefLocks(ref string, l locking.Lock) *refLock {
+func (lv *lockVerifier) newRefLocks(ref *git.Ref, l locking.Lock) *refLock {
 	return &refLock{
 		allRefs: lv.verifiedRefs,
 		path:    l.Path,
-		refs:    map[string]locking.Lock{ref: l},
+		refs:    map[*git.Ref]locking.Lock{ref: l},
 	}
 }
 
@@ -169,7 +170,7 @@ func newLockVerifier(m *tq.Manifest) *lockVerifier {
 type refLock struct {
 	path    string
 	allRefs map[string]bool
-	refs    map[string]locking.Lock
+	refs    map[*git.Ref]locking.Lock
 }
 
 // Path returns the locked path.
@@ -189,7 +190,7 @@ func (r *refLock) Owners() string {
 		if _, ok := users[u]; !ok {
 			users[u] = make([]string, 0, len(r.refs))
 		}
-		users[u] = append(users[u], ref)
+		users[u] = append(users[u], ref.Name)
 	}
 
 	owners := make([]string, 0, len(users))
@@ -212,7 +213,7 @@ func (r *refLock) Owners() string {
 	return strings.Join(owners, ", ")
 }
 
-func (r *refLock) Add(ref string, l locking.Lock) error {
+func (r *refLock) Add(ref *git.Ref, l locking.Lock) error {
 	r.refs[ref] = l
 	return nil
 }

commands/refs.go

@@ -57,7 +57,7 @@ func defaultRemoteRef(g config.Environment, remote string, left *git.Ref) *git.R
 
 		// When pushing to a remote that is different from the remote you normally
 		// pull from, work as current.
-		return &git.Ref{Name: left.Name}
+		return left
 	case "upstream", "tracking":
 		// push the current branch back to the branch whose changes are usually
 		// integrated into the current branch
@@ -65,10 +65,10 @@ func defaultRemoteRef(g config.Environment, remote string, left *git.Ref) *git.R
 	case "current":
 		// push the current branch to update a branch with the same name on the
 		// receiving end.
-		return &git.Ref{Name: left.Name}
+		return left
 	default:
 		tracerx.Printf("WARNING: %q push mode not supported", pushMode)
-		return &git.Ref{Name: left.Name}
+		return left
 	}
 }
 
@@ -76,7 +76,7 @@ func trackingRef(g config.Environment, left *git.Ref) *git.Ref {
 	if merge, ok := g.Get(fmt.Sprintf("branch.%s.merge", left.Name)); ok {
 		return git.ParseRef(merge, "")
 	}
-	return &git.Ref{Name: left.Name}
+	return left
 }
 
 func (u *refUpdate) RightCommitish() string {

commands/refs_test.go

@@ -19,8 +19,9 @@ func TestRefUpdateDefault(t *testing.T) {
 			},
 		})
 
-		u := newRefUpdate(cfg.Git, "origin", git.ParseRef("left", ""), nil)
+		u := newRefUpdate(cfg.Git, "origin", git.ParseRef("refs/heads/left", ""), nil)
 		assert.Equal(t, "left", u.Right().Name, "pushmode=%q", pushMode)
+		assert.Equal(t, git.RefTypeLocalBranch, u.Right().Type, "pushmode=%q", pushMode)
 	}
 }
 
@@ -31,12 +32,13 @@ func TestRefUpdateTrackedDefault(t *testing.T) {
 			Git: map[string][]string{
 				"push.default":       []string{pushMode},
 				"branch.left.remote": []string{"origin"},
-				"branch.left.merge":  []string{"tracked"},
+				"branch.left.merge":  []string{"refs/heads/tracked"},
 			},
 		})
 
-		u := newRefUpdate(cfg.Git, "origin", git.ParseRef("left", ""), nil)
+		u := newRefUpdate(cfg.Git, "origin", git.ParseRef("refs/heads/left", ""), nil)
 		assert.Equal(t, "tracked", u.Right().Name, "pushmode=%s", pushMode)
+		assert.Equal(t, git.RefTypeLocalBranch, u.Right().Type, "pushmode=%q", pushMode)
 	}
 }
 
@@ -49,12 +51,13 @@ func TestRefUpdateCurrentDefault(t *testing.T) {
 		},
 	})
 
-	u := newRefUpdate(cfg.Git, "origin", git.ParseRef("left", ""), nil)
+	u := newRefUpdate(cfg.Git, "origin", git.ParseRef("refs/heads/left", ""), nil)
 	assert.Equal(t, "left", u.Right().Name)
+	assert.Equal(t, git.RefTypeLocalBranch, u.Right().Type)
 }
 
 func TestRefUpdateExplicitLeftAndRight(t *testing.T) {
-	u := newRefUpdate(nil, "", git.ParseRef("left", "abc123"), git.ParseRef("right", "def456"))
+	u := newRefUpdate(nil, "", git.ParseRef("refs/heads/left", "abc123"), git.ParseRef("refs/heads/right", "def456"))
 	assert.Equal(t, "left", u.Left().Name)
 	assert.Equal(t, "abc123", u.Left().Sha)
 	assert.Equal(t, "abc123", u.LeftCommitish())
@@ -62,7 +65,7 @@ func TestRefUpdateExplicitLeftAndRight(t *testing.T) {
 	assert.Equal(t, "def456", u.Right().Sha)
 	assert.Equal(t, "def456", u.RightCommitish())
 
-	u = newRefUpdate(nil, "", git.ParseRef("left", ""), git.ParseRef("right", ""))
+	u = newRefUpdate(nil, "", git.ParseRef("refs/heads/left", ""), git.ParseRef("refs/heads/right", ""))
 	assert.Equal(t, "left", u.Left().Name)
 	assert.Equal(t, "", u.Left().Sha)
 	assert.Equal(t, "left", u.LeftCommitish())

git/attribs.go

@@ -36,7 +36,7 @@ func (s *AttributeSource) String() string {
 
 // GetAttributePaths returns a list of entries in .gitattributes which are
 // configured with the filter=lfs attribute
-// workingDIr is the root of the working copy
+// workingDir is the root of the working copy
 // gitDir is the root of the git repo
 func GetAttributePaths(workingDir, gitDir string) []AttributePath {
 	paths := make([]AttributePath, 0)
@@ -56,7 +56,11 @@ func GetAttributePaths(workingDir, gitDir string) []AttributePath {
 		scanner.Split(le.ScanLines)
 
 		for scanner.Scan() {
-			line := scanner.Text()
+			line := strings.TrimSpace(scanner.Text())
+
+			if strings.HasPrefix(line, "#") {
+				continue
+			}
 
 			// Check for filter=lfs (signifying that LFS is tracking
 			// this file) or "lockable", which indicates that the

git/git.go

@@ -54,12 +54,8 @@ func (t RefType) Prefix() (string, bool) {
 		return "refs/tags", true
 	case RefTypeRemoteTag:
 		return "refs/remotes/tags", true
-	case RefTypeHEAD:
-		return "", false
-	case RefTypeOther:
-		return "", false
 	default:
-		panic(fmt.Sprintf("git: unknown RefType %d", t))
+		return "", false
 	}
 }
 

git/git_test.go

@@ -623,16 +623,3 @@ func TestRefTypeKnownPrefixes(t *testing.T) {
 		assert.Equal(t, expected.Ok, ok)
 	}
 }
-
-func TestRefTypeUnknownPrefix(t *testing.T) {
-	defer func() {
-		if err := recover(); err != nil {
-			assert.Equal(t, "git: unknown RefType -1", err)
-		} else {
-			t.Fatal("git: expected panic() from RefType.Prefix()")
-		}
-	}()
-
-	unknown := RefType(-1)
-	unknown.Prefix()
-}

git/rev_list_scanner.go

@@ -225,7 +225,7 @@ func NewRevListScanner(include, excluded []string, opt *ScanRefsOptions) (*RevLi
 // occurred.
 func revListArgs(include, exclude []string, opt *ScanRefsOptions) (io.Reader, []string, error) {
 	var stdin io.Reader
-	args := []string{"rev-list"}
+	args := []string{"rev-list", "--stdin"}
 	if !opt.CommitsOnly {
 		args = append(args, "--objects")
 	}
@@ -246,15 +246,16 @@ func revListArgs(include, exclude []string, opt *ScanRefsOptions) (io.Reader, []
 			args = append(args, "--do-walk")
 		}
 
-		args = append(args, includeExcludeShas(include, exclude)...)
+		stdin = strings.NewReader(strings.Join(
+			includeExcludeShas(include, exclude), "\n"))
 	case ScanAllMode:
 		args = append(args, "--all")
 	case ScanLeftToRemoteMode:
 		if len(opt.SkippedRefs) == 0 {
-			args = append(args, includeExcludeShas(include, exclude)...)
 			args = append(args, "--not", "--remotes="+opt.Remote)
+			stdin = strings.NewReader(strings.Join(
+				includeExcludeShas(include, exclude), "\n"))
 		} else {
-			args = append(args, "--stdin")
 			stdin = strings.NewReader(strings.Join(
 				append(includeExcludeShas(include, exclude), opt.SkippedRefs...), "\n"),
 			)

git/rev_list_scanner_test.go

@@ -4,13 +4,13 @@ import (
 	"bufio"
 	"encoding/hex"
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"strings"
 	"sync/atomic"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 type ArgsTestCase struct {
@@ -32,11 +32,7 @@ func (c *ArgsTestCase) Assert(t *testing.T) {
 		assert.Nil(t, err)
 	}
 
-	require.Equal(t, len(c.ExpectedArgs), len(args))
+	assert.EqualValues(t, c.ExpectedArgs, args)
-	for i := 0; i < len(c.ExpectedArgs); i++ {
-		assert.Equal(t, c.ExpectedArgs[i], args[i],
-			"element #%d not equal: wanted %q, got %q", i, c.ExpectedArgs[i], args[i])
-	}
 
 	if stdin != nil {
 		b, err := ioutil.ReadAll(stdin)
@@ -60,34 +56,38 @@ func TestRevListArgs(t *testing.T) {
 				Mode:             ScanRefsMode,
 				SkipDeletedBlobs: false,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--do-walk", s1, "^" + s2, "--"},
+			ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--do-walk", "--"},
 		},
 		"scan refs not deleted, left and right": {
 			Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
 				Mode:             ScanRefsMode,
 				SkipDeletedBlobs: true,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--no-walk", s1, "^" + s2, "--"},
+			ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--no-walk", "--"},
 		},
 		"scan refs deleted, left only": {
 			Include: []string{s1}, Opt: &ScanRefsOptions{
 				Mode:             ScanRefsMode,
 				SkipDeletedBlobs: false,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--do-walk", s1, "--"},
+			ExpectedStdin: s1,
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--do-walk", "--"},
 		},
 		"scan refs not deleted, left only": {
 			Include: []string{s1}, Opt: &ScanRefsOptions{
 				Mode:             ScanRefsMode,
 				SkipDeletedBlobs: true,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--no-walk", s1, "--"},
+			ExpectedStdin: s1,
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--no-walk", "--"},
 		},
 		"scan all": {
 			Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
 				Mode: ScanAllMode,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--all", "--"},
+			ExpectedArgs: []string{"rev-list", "--stdin", "--objects", "--all", "--"},
 		},
 		"scan left to remote, no skipped refs": {
 			Include: []string{s1}, Opt: &ScanRefsOptions{
@@ -95,7 +95,8 @@ func TestRevListArgs(t *testing.T) {
 				Remote:      "origin",
 				SkippedRefs: []string{},
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", s1, "--not", "--remotes=origin", "--"},
+			ExpectedStdin: s1,
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--not", "--remotes=origin", "--"},
 		},
 		"scan left to remote, skipped refs": {
 			Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
@@ -103,7 +104,7 @@ func TestRevListArgs(t *testing.T) {
 				Remote:      "origin",
 				SkippedRefs: []string{"a", "b", "c"},
 			},
-			ExpectedArgs:  []string{"rev-list", "--objects", "--stdin", "--"},
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--"},
 			ExpectedStdin: s1 + "\n^" + s2 + "\na\nb\nc",
 		},
 		"scan unknown type": {
@@ -117,35 +118,40 @@ func TestRevListArgs(t *testing.T) {
 				Mode:  ScanRefsMode,
 				Order: DateRevListOrder,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--date-order", "--do-walk", s1, "^" + s2, "--"},
+			ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--date-order", "--do-walk", "--"},
 		},
 		"scan author date order": {
 			Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
 				Mode:  ScanRefsMode,
 				Order: AuthorDateRevListOrder,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--author-date-order", "--do-walk", s1, "^" + s2, "--"},
+			ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--author-date-order", "--do-walk", "--"},
 		},
 		"scan topo order": {
 			Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
 				Mode:  ScanRefsMode,
 				Order: TopoRevListOrder,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--topo-order", "--do-walk", s1, "^" + s2, "--"},
+			ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--topo-order", "--do-walk", "--"},
 		},
 		"scan commits only": {
 			Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
 				Mode:        ScanRefsMode,
 				CommitsOnly: true,
 			},
-			ExpectedArgs: []string{"rev-list", "--do-walk", s1, "^" + s2, "--"},
+			ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--do-walk", "--"},
 		},
 		"scan reverse": {
 			Include: []string{s1}, Exclude: []string{s2}, Opt: &ScanRefsOptions{
 				Mode:    ScanRefsMode,
 				Reverse: true,
 			},
-			ExpectedArgs: []string{"rev-list", "--objects", "--reverse", "--do-walk", s1, "^" + s2, "--"},
+			ExpectedStdin: fmt.Sprintf("%s\n^%s", s1, s2),
+			ExpectedArgs:  []string{"rev-list", "--stdin", "--objects", "--reverse", "--do-walk", "--"},
 		},
 	} {
 		t.Run(desc, c.Assert)

locking/locks.go

@@ -206,11 +206,11 @@ func (c *Client) SearchLocks(filter map[string]string, limit int, localOnly bool
 	}
 }
 
-func (c *Client) VerifiableLocks(ref string, limit int) (ourLocks, theirLocks []Lock, err error) {
+func (c *Client) VerifiableLocks(ref *git.Ref, limit int) (ourLocks, theirLocks []Lock, err error) {
 	ourLocks = make([]Lock, 0, limit)
 	theirLocks = make([]Lock, 0, limit)
 	body := &lockVerifiableRequest{
-		Ref:   &lockRef{Name: ref},
+		Ref:   &lockRef{Name: ref.Refspec()},
 		Limit: limit,
 	}
 

locking/locks_test.go

@@ -64,7 +64,7 @@ func TestRefreshCache(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Empty(t, locks)
 
-	_, _, err = client.VerifiableLocks("", 100)
+	_, _, err = client.VerifiableLocks(nil, 100)
 	assert.Nil(t, err)
 
 	locks, err = client.SearchLocks(nil, 0, true)
@@ -130,7 +130,7 @@ func TestGetVerifiableLocks(t *testing.T) {
 	client, err := NewClient("", lfsclient)
 	assert.Nil(t, err)
 
-	ourLocks, theirLocks, err := client.VerifiableLocks("", 0)
+	ourLocks, theirLocks, err := client.VerifiableLocks(nil, 0)
 	assert.Nil(t, err)
 
 	// Need to include zero time in structure for equal to work

script/integration.go

@@ -50,17 +50,24 @@ func mainIntegration() {
 	for _, file := range files {
 		tests <- file
 	}
+	close(tests)
+
+	outputDone := make(chan bool)
+	go func() {
+		for out := range output {
+			fmt.Println(out)
+		}
+		outputDone <- true
+	}()
 
-	go printOutput(output)
 	for i := 0; i < maxprocs; i++ {
 		wg.Add(1)
 		go worker(tests, output, &wg)
 	}
 
-	close(tests)
 	wg.Wait()
 	close(output)
-	printOutput(output)
+	<-outputDone
 
 	if erroring {
 		os.Exit(1)
@@ -116,19 +123,6 @@ func sendTestOutput(output chan string, testname string, buf *bytes.Buffer, err
 	}
 }
 
-func printOutput(output <-chan string) {
-	for {
-		select {
-		case out, ok := <-output:
-			if !ok {
-				return
-			}
-
-			fmt.Println(out)
-		}
-	}
-}
-
 func worker(tests <-chan string, output chan string, wg *sync.WaitGroup) {
 	defer wg.Done()
 	for {

test/cmd/lfstest-gitserver.go

@@ -1101,14 +1101,14 @@ func locksHandler(w http.ResponseWriter, r *http.Request, repo string) {
 				return
 			}
 
-			if strings.HasSuffix(repo, "ref-required") {
+			if strings.HasSuffix(repo, "branch-required") {
 				parts := strings.Split(repo, "-")
 				lenParts := len(parts)
-				if lenParts > 3 && parts[lenParts-3] != reqBody.RefName() {
+				if lenParts > 3 && "refs/heads/"+parts[lenParts-3] != reqBody.RefName() {
 					w.WriteHeader(403)
 					enc.Encode(struct {
 						Message string `json:"message"`
-					}{fmt.Sprintf("Expected ref %q, got %q", parts[lenParts-3], reqBody.RefName())})
+					}{fmt.Sprintf("Expected ref %q, got %q", "refs/heads/"+parts[lenParts-3], reqBody.RefName())})
 					return
 				}
 			}

test/test-pre-push.sh

@@ -781,7 +781,7 @@ begin_test "pre-push locks verify 403 with good ref"
 (
   set -e
 
-  reponame="lock-verify-master-ref-required"
+  reponame="lock-verify-master-branch-required"
   setup_remote_repo "$reponame"
   clone_repo "$reponame" "$reponame"
 
@@ -803,7 +803,7 @@ begin_test "pre-push locks verify 403 with good tracked ref"
 (
   set -e
 
-  reponame="lock-verify-tracked-ref-required"
+  reponame="lock-verify-tracked-branch-required"
   setup_remote_repo "$reponame"
   clone_repo "$reponame" "$reponame"
 
@@ -827,7 +827,7 @@ begin_test "pre-push locks verify 403 with explicit ref"
 (
   set -e
 
-  reponame="lock-verify-explicit-ref-required"
+  reponame="lock-verify-explicit-branch-required"
   setup_remote_repo "$reponame"
   clone_repo "$reponame" "$reponame"
 
@@ -849,7 +849,7 @@ begin_test "pre-push locks verify 403 with bad ref"
 (
   set -e
 
-  reponame="lock-verify-other-ref-required"
+  reponame="lock-verify-other-branch-required"
   setup_remote_repo "$reponame"
   clone_repo "$reponame" "$reponame"