diff --git a/internal/backup/backup.go b/internal/backup/backup.go index 5dc959dd2f2bdd360ad050ba67192cef42b3895c..3cbb7eaf397eb639ceeea4e2e1b84925ce51f7eb 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -337,7 +337,7 @@ func (mgr *Manager) sendKnownRefs(ctx context.Context, step *Step, repo *gitalyp } defer reader.Close() - d := NewRefsDecoder(reader) + d := git.NewShowRefDecoder(reader) for { var ref git.Reference diff --git a/internal/backup/refs.go b/internal/backup/refs.go deleted file mode 100644 index cbce8bc2e0edd7151ad352f42435668d2ad3cc14..0000000000000000000000000000000000000000 --- a/internal/backup/refs.go +++ /dev/null @@ -1,50 +0,0 @@ -package backup - -import ( - "bufio" - "fmt" - "io" - "strings" - - "gitlab.com/gitlab-org/gitaly/v14/internal/git" -) - -// RefsDecoder parses the output format of git-show-ref -type RefsDecoder struct { - r *bufio.Reader - err error -} - -// NewRefsDecoder returns a new RefsDecoder -func NewRefsDecoder(r io.Reader) *RefsDecoder { - return &RefsDecoder{ - r: bufio.NewReader(r), - } -} - -// Decode reads and parses the next reference. Returns io.EOF when the end of -// the reader has been reached. -func (d *RefsDecoder) Decode(ref *git.Reference) error { - if d.err != nil { - return d.err - } - - line, err := d.r.ReadString('\n') - d.err = err - - if len(line) > 0 && line[len(line)-1] == '\n' { - line = line[:len(line)-1] - } - - splits := strings.SplitN(line, " ", 2) - if len(splits) != 2 { - if d.err != nil { - return d.err - } - return fmt.Errorf("refs decoder: invalid line: %q", line) - } - - *ref = git.NewReference(git.ReferenceName(splits[1]), splits[0]) - - return nil -} diff --git a/internal/git/decoder.go b/internal/git/decoder.go new file mode 100644 index 0000000000000000000000000000000000000000..e788dede9ad76275d2f307bdd735e6339bce134f --- /dev/null +++ b/internal/git/decoder.go @@ -0,0 +1,70 @@ +package git + +import ( + "bufio" + "fmt" + "io" + "strings" +) + +// RefsDecoder parses git output for git references +type RefsDecoder struct { + r *bufio.Reader + sep string + err error +} + +// NewShowRefDecoder returns a new RefsDecoder to decode the output from git-show-ref +func NewShowRefDecoder(r io.Reader) *RefsDecoder { + return &RefsDecoder{ + r: bufio.NewReader(r), + sep: " ", + } +} + +// ForEachRefFormat can be passed to the `for-each-ref` `--format` flag +// to retrieve references that can be parsed by `NewForEachRefDecoder`. +const ForEachRefFormat = "%(objectname) %(refname)" + +// ForEachRefSymbolicFormat can be passed to the `for-each-ref` `--format` flag +// to retrieve references that can be parsed by `NewForEachRefDecoder` and that +// distinguishes symbolic references. +const ForEachRefSymbolicFormat = "%(objectname) %(refname) %(symref)" + +// NewForEachRefDecoder ... +func NewForEachRefDecoder(r io.Reader) *RefsDecoder { + return &RefsDecoder{ + r: bufio.NewReader(r), + sep: " ", + } +} + +// Decode reads and parses the next reference. Returns io.EOF when the end of +// the reader has been reached. +func (d *RefsDecoder) Decode(ref *Reference) error { + if d.err != nil { + return d.err + } + + line, err := d.r.ReadString('\n') + d.err = err + + if len(line) > 0 && line[len(line)-1] == '\n' { + line = line[:len(line)-1] + } + + splits := strings.SplitN(line, d.sep, 3) + switch { + case len(splits) == 3 && len(splits[2]) > 0: + *ref = NewSymbolicReference(ReferenceName(splits[1]), splits[0]) + case len(splits) >= 2: + *ref = NewReference(ReferenceName(splits[1]), splits[0]) + default: + if d.err != nil { + return d.err + } + return fmt.Errorf("refs decoder: invalid line: %q", line) + } + + return nil +} diff --git a/internal/backup/refs_test.go b/internal/git/decoder_test.go similarity index 82% rename from internal/backup/refs_test.go rename to internal/git/decoder_test.go index 3aa713f4b62a602c280a4b4ac68040d1dc2d6e3b..a31b361bb1c54cf8127d355db15599d6c6b1c524 100644 --- a/internal/backup/refs_test.go +++ b/internal/git/decoder_test.go @@ -1,4 +1,4 @@ -package backup +package git_test import ( "bytes" @@ -6,14 +6,14 @@ import ( "testing" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v14/internal/git" + . "gitlab.com/gitlab-org/gitaly/v14/internal/git" "gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg" ) -func TestRefsDecoder(t *testing.T) { +func TestRefsDecoder_gitShowRef(t *testing.T) { cfg := testcfg.Build(t) ctx, cancel := testhelper.Context() @@ -31,11 +31,11 @@ func TestRefsDecoder(t *testing.T) { output := gittest.Exec(t, cfg, "-C", repoPath, "show-ref") stream := bytes.NewBuffer(output) - d := NewRefsDecoder(stream) + d := NewShowRefDecoder(stream) - var refs []git.Reference + var refs []Reference for { - var ref git.Reference + var ref Reference err := d.Decode(&ref) if err == io.EOF { diff --git a/internal/git/gitpipe/revision.go b/internal/git/gitpipe/revision.go index 0f97a9d8ad0f456c04bac1c93de2642a4259ce51..27472c3db540509459a60ab51d46c035a4bb7c6d 100644 --- a/internal/git/gitpipe/revision.go +++ b/internal/git/gitpipe/revision.go @@ -5,8 +5,8 @@ import ( "bytes" "context" "fmt" + "io" "strconv" - "strings" "time" "gitlab.com/gitlab-org/gitaly/v14/internal/git" @@ -368,7 +368,7 @@ func ForEachRef( // referenced commit's object. It would thus be about 2-3x slower to use the // default format, and instead we move the burden into the next pipeline step by // default. - format: "%(objectname) %(refname)", + format: git.ForEachRefFormat, } for _, opt := range opts { opt(&cfg) @@ -401,33 +401,27 @@ func ForEachRef( return } - scanner := bufio.NewScanner(forEachRef) - for scanner.Scan() { - line := scanner.Text() - - oidAndRef := strings.SplitN(line, " ", 2) - if len(oidAndRef) != 2 { + decoder := git.NewForEachRefDecoder(forEachRef) + for { + var ref git.Reference + err := decoder.Decode(&ref) + if err == io.EOF { + break + } else if err != nil { sendRevisionResult(ctx, resultChan, RevisionResult{ - err: fmt.Errorf("invalid for-each-ref format: %q", line), + err: fmt.Errorf("scanning for-each-ref output: %w", err), }) return } if isDone := sendRevisionResult(ctx, resultChan, RevisionResult{ - OID: git.ObjectID(oidAndRef[0]), - ObjectName: []byte(oidAndRef[1]), + OID: git.ObjectID(ref.Target), + ObjectName: []byte(ref.Name), }); isDone { return } } - if err := scanner.Err(); err != nil { - sendRevisionResult(ctx, resultChan, RevisionResult{ - err: fmt.Errorf("scanning for-each-ref output: %w", err), - }) - return - } - if err := forEachRef.Wait(); err != nil { sendRevisionResult(ctx, resultChan, RevisionResult{ err: fmt.Errorf("for-each-ref pipeline command: %w", err), diff --git a/internal/git/localrepo/refs.go b/internal/git/localrepo/refs.go index 0ac4bf96900e6171766d0f478ffe40d48b6b3d14..eef4b6c0c32a371304e09c26231cf603aa6dcbeb 100644 --- a/internal/git/localrepo/refs.go +++ b/internal/git/localrepo/refs.go @@ -93,7 +93,7 @@ func (repo *Repo) GetReferences(ctx context.Context, patterns ...string) ([]git. } func (repo *Repo) getReferences(ctx context.Context, limit uint, patterns ...string) ([]git.Reference, error) { - flags := []git.Option{git.Flag{Name: "--format=%(refname)%00%(objectname)%00%(symref)"}} + flags := []git.Option{git.ValueFlag{Name: "--format", Value: git.ForEachRefSymbolicFormat}} if limit > 0 { flags = append(flags, git.Flag{Name: fmt.Sprintf("--count=%d", limit)}) } @@ -107,25 +107,20 @@ func (repo *Repo) getReferences(ctx context.Context, limit uint, patterns ...str return nil, err } - scanner := bufio.NewScanner(cmd) + decoder := git.NewForEachRefDecoder(cmd) var refs []git.Reference - for scanner.Scan() { - line := bytes.SplitN(scanner.Bytes(), []byte{0}, 3) - if len(line) != 3 { - return nil, errors.New("unexpected reference format") - } - - if len(line[2]) == 0 { - refs = append(refs, git.NewReference(git.ReferenceName(line[0]), string(line[1]))) - } else { - refs = append(refs, git.NewSymbolicReference(git.ReferenceName(line[0]), string(line[1]))) + for { + var ref git.Reference + err := decoder.Decode(&ref) + if err == io.EOF { + break + } else if err != nil { + return nil, fmt.Errorf("reading standard input: %v", err) } + refs = append(refs, ref) } - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("reading standard input: %v", err) - } if err := cmd.Wait(); err != nil { return nil, err } diff --git a/internal/gitaly/service/ref/find_all_tags.go b/internal/gitaly/service/ref/find_all_tags.go index 47eb787a246754b81f77eddbb4960b2437d4ee3e..849427c1b1130daba596f87923448d066067a932 100644 --- a/internal/gitaly/service/ref/find_all_tags.go +++ b/internal/gitaly/service/ref/find_all_tags.go @@ -50,7 +50,7 @@ func (s *server) findAllTags(ctx context.Context, repo *localrepo.Repo, sortFiel repo, []string{"refs/tags/"}, gitpipe.WithSortField(sortField), - gitpipe.WithForEachRefFormat("%(objectname) %(refname)%(if)%(*objectname)%(then)\n%(objectname)^{} PEELED%(end)"), + gitpipe.WithForEachRefFormat("%(objectname) %(refname)%(if)%(*objectname)%(then)\n%(objectname)^{} peeled%(end)"), ) catfileObjectsIter := gitpipe.CatfileObject(ctx, objectReader, forEachRefIter)