diff --git a/commands/lockverifier.go b/commands/lockverifier.go index f1da8ea1..80fa8855 100644 --- a/commands/lockverifier.go +++ b/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) { - if lv.verifyState == verifyStateDisabled || lv.verifiedRefs[ref] { +func (lv *lockVerifier) Verify(ref *git.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 } diff --git a/commands/refs.go b/commands/refs.go index 988ef899..bc57fff9 100644 --- a/commands/refs.go +++ b/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 { diff --git a/commands/refs_test.go b/commands/refs_test.go index e25fb9c4..cc98b3d8 100644 --- a/commands/refs_test.go +++ b/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()) diff --git a/git/attribs.go b/git/attribs.go index 44419328..067bf42d 100644 --- a/git/attribs.go +++ b/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 diff --git a/git/git.go b/git/git.go index 2870a6c7..bd5b931f 100644 --- a/git/git.go +++ b/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 } } diff --git a/git/git_test.go b/git/git_test.go index 375551d7..afd83b82 100644 --- a/git/git_test.go +++ b/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() -} diff --git a/git/rev_list_scanner.go b/git/rev_list_scanner.go index 294dbe5a..58f28457 100644 --- a/git/rev_list_scanner.go +++ b/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"), ) diff --git a/git/rev_list_scanner_test.go b/git/rev_list_scanner_test.go index ab29417c..70bd9985 100644 --- a/git/rev_list_scanner_test.go +++ b/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)) - 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]) - } + assert.EqualValues(t, c.ExpectedArgs, args) 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) diff --git a/locking/locks.go b/locking/locks.go index 9f4377b1..31b91425 100644 --- a/locking/locks.go +++ b/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, } diff --git a/locking/locks_test.go b/locking/locks_test.go index 3eaa5c97..6cf55ad6 100644 --- a/locking/locks_test.go +++ b/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 diff --git a/script/integration.go b/script/integration.go index 68b1beb8..e67b30a9 100644 --- a/script/integration.go +++ b/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 { diff --git a/test/cmd/lfstest-gitserver.go b/test/cmd/lfstest-gitserver.go index 334a0611..f3aee82e 100644 --- a/test/cmd/lfstest-gitserver.go +++ b/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 } } diff --git a/test/test-pre-push.sh b/test/test-pre-push.sh index eb18e563..071c34f9 100755 --- a/test/test-pre-push.sh +++ b/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"