From 2ea86227524aa59bce5b9672b844f83c46cc00c5 Mon Sep 17 00:00:00 2001 From: Juanjo Alvarez Date: Tue, 29 Oct 2019 14:43:37 +0100 Subject: [PATCH 1/3] Change blame to use file arg Signed-off-by: Juanjo Alvarez --- docs/using-gitbase/examples.md | 11 ++--- docs/using-gitbase/functions.md | 2 +- integration_test.go | 4 +- internal/function/blame.go | 73 ++++++++++----------------------- internal/function/blame_test.go | 31 +++++++++----- internal/function/registry.go | 2 +- 6 files changed, 52 insertions(+), 71 deletions(-) diff --git a/docs/using-gitbase/examples.md b/docs/using-gitbase/examples.md index fe5845eb2..5b54e65b6 100644 --- a/docs/using-gitbase/examples.md +++ b/docs/using-gitbase/examples.md @@ -203,15 +203,15 @@ The output will be similar to this: ## Get miscelaneous information about lines with a "// TODO" comment in HEAD ```sql -SELECT repository_id, - JSON_UNQUOTE(JSON_EXTRACT(bl, "$.file")), +SELECT repository_id, file_path, JSON_UNQUOTE(JSON_EXTRACT(bl, "$.linenum")), JSON_UNQUOTE(JSON_EXTRACT(bl, "$.author")), JSON_UNQUOTE(JSON_EXTRACT(bl, "$.text")) -FROM (SELECT repository_id, - EXPLODE(BLAME(repository_id, commit_hash)) AS bl +FROM (SELECT repository_id, file_path, + EXPLODE(BLAME(repository_id, commit_hash, file_path)) AS bl FROM ref_commits NATURAL JOIN blobs + NATURAL JOIN commit_files WHERE ref_name = 'HEAD' AND NOT IS_BINARY(blob_content) ) as p @@ -225,9 +225,10 @@ SELECT JSON_UNQUOTE(JSON_EXTRACT(bl, "$.author")), COUNT(JSON_UNQUOTE(JSON_EXTRACT(bl, "$.author"))) -FROM (SELECT EXPLODE(BLAME(repository_id, commit_hash)) AS bl +FROM (SELECT EXPLODE(BLAME(repository_id, commit_hash, file_path)) AS bl FROM ref_commits NATURAL JOIN blobs + NATURAL JOIN commit_files WHERE ref_name = 'HEAD' AND NOT IS_BINARY(blob_content) ) AS p diff --git a/docs/using-gitbase/functions.md b/docs/using-gitbase/functions.md index 875a2efce..8da8c1533 100644 --- a/docs/using-gitbase/functions.md +++ b/docs/using-gitbase/functions.md @@ -6,7 +6,7 @@ To make some common tasks easier for the user, there are some functions to inter | Name | Description | |:-------------|:-------------------------------------------------------------------------------------------------------------------------------| -|`blame(repository, commit)`|Returns an array of lines changes and authorship. | +|`blame(repository, commit, file)`|Returns an array of lines changes and authorship for the specific file and commit. |`commit_file_stats(repository_id, [from_commit_hash], to_commit_hash) json array`|returns an array with the stats of each file in `to_commit_hash` since the given `from_commit_hash`. If `from_commit_hash` is not given, the parent commit will be used. Vendored files stats are not included in the result of this function. This function is more thoroughly explained later in this document.| |`commit_stats(repository_id, [from_commit_hash], to_commit_hash) json`|returns the stats between two commits for a repository. If `from_commit_hash` is empty, it will compare the given `to_commit_hash` with its parent commit. Vendored files stats are not included in the result of this function. This function is more thoroughly explained later in this document.| |`is_remote(reference_name)bool`| checks if the given reference name is from a remote one. | diff --git a/integration_test.go b/integration_test.go index ed8680de1..eb91f5085 100644 --- a/integration_test.go +++ b/integration_test.go @@ -514,12 +514,12 @@ func TestIntegration(t *testing.T) { SELECT repository_id, JSON_EXTRACT(bl, "$.author"), COUNT(bl) FROM ( - SELECT repository_id, EXPLODE(BLAME(repository_id, commit_hash)) as bl + SELECT repository_id, EXPLODE(BLAME(repository_id, commit_hash, '.gitignore')) as bl FROM commits WHERE commit_hash = '918c48b83bd081e863dbe1b80f8998f058cd8294' ) as p `, - []sql.Row{{"worktree", "mcuadros@gmail.com", int64(7235)}}, + []sql.Row{{"worktree", "mcuadros@gmail.com", int64(12)}}, }, } diff --git a/internal/function/blame.go b/internal/function/blame.go index 079581c61..aa2a87445 100644 --- a/internal/function/blame.go +++ b/internal/function/blame.go @@ -4,8 +4,6 @@ import ( "fmt" "io" - "github.com/sirupsen/logrus" - "github.com/src-d/gitbase" "github.com/src-d/go-mysql-server/sql" "gopkg.in/src-d/go-git.v4" @@ -15,57 +13,27 @@ import ( ) type BlameGenerator struct { - ctx *sql.Context commit *object.Commit - fIter *object.FileIter + file string curLine int - curFile *object.File lines []*git.Line } -func NewBlameGenerator(ctx *sql.Context, c *object.Commit, f *object.FileIter) (*BlameGenerator, error) { - return &BlameGenerator{ctx: ctx, commit: c, fIter: f, curLine: -1}, nil -} - -func (g *BlameGenerator) loadNewFile() error { - var err error - g.curFile, err = g.fIter.Next() +func NewBlameGenerator(c *object.Commit, f string) (*BlameGenerator, error) { + result, err := git.Blame(c, f) if err != nil { - return err - } - - result, err := git.Blame(g.commit, g.curFile.Name) - if err != nil { - msg := fmt.Sprintf( - "Error in BLAME for file %s: %s", - g.curFile.Name, - err.Error(), - ) - logrus.Warn(msg) - g.ctx.Warn(0, msg) - return io.EOF - } - - if len(result.Lines) == 0 { - return g.loadNewFile() + return nil, err } - - g.lines = result.Lines - g.curLine = 0 - return nil + return &BlameGenerator{commit: c, file: f, curLine: 0, lines: result.Lines}, nil } func (g *BlameGenerator) Next() (interface{}, error) { - if g.curLine == -1 || g.curLine >= len(g.lines) { - err := g.loadNewFile() - if err != nil { - return nil, err - } + if len(g.lines) == 0 || g.curLine >= len(g.lines) { + return nil, io.EOF } l := g.lines[g.curLine] b := BlameLine{ - File: g.curFile.Name, LineNum: g.curLine, Author: l.Author, Text: l.Text, @@ -75,7 +43,6 @@ func (g *BlameGenerator) Next() (interface{}, error) { } func (g *BlameGenerator) Close() error { - g.fIter.Close() return nil } @@ -86,11 +53,11 @@ type ( Blame struct { repo sql.Expression commit sql.Expression + file sql.Expression } // BlameLine represents each line of git blame's output BlameLine struct { - File string `json:"file"` LineNum int `json:"linenum"` Author string `json:"author"` Text string `json:"text"` @@ -98,8 +65,8 @@ type ( ) // NewBlame constructor -func NewBlame(repo, commit sql.Expression) sql.Expression { - return &Blame{repo, commit} +func NewBlame(repo, commit, file sql.Expression) sql.Expression { + return &Blame{repo, commit, file} } func (b *Blame) String() string { @@ -112,26 +79,26 @@ func (*Blame) Type() sql.Type { } func (b *Blame) WithChildren(children ...sql.Expression) (sql.Expression, error) { - if len(children) != 2 { + if len(children) != 3 { return nil, sql.ErrInvalidChildrenNumber.New(b, len(children), 2) } - return NewBlame(children[0], children[1]), nil + return NewBlame(children[0], children[1], children[2]), nil } // Children implements the Expression interface. func (b *Blame) Children() []sql.Expression { - return []sql.Expression{b.repo, b.commit} + return []sql.Expression{b.repo, b.commit, b.file} } // IsNullable implements the Expression interface. func (b *Blame) IsNullable() bool { - return b.repo.IsNullable() || (b.commit.IsNullable()) + return b.repo.IsNullable() || (b.commit.IsNullable()) || (b.file.IsNullable()) } // Resolved implements the Expression interface. func (b *Blame) Resolved() bool { - return b.repo.Resolved() && b.commit.Resolved() + return b.repo.Resolved() && b.commit.Resolved() && b.file.Resolved() } // Eval implements the sql.Expression interface. @@ -151,14 +118,16 @@ func (b *Blame) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { return nil, nil } - fIter, err := commit.Files() + file, err := exprToString(ctx, b.file, row) if err != nil { - return nil, err + ctx.Warn(0, err.Error()) + return nil, nil } - bg, err := NewBlameGenerator(ctx, commit, fIter) + bg, err := NewBlameGenerator(commit, file) if err != nil { - return nil, err + ctx.Warn(0, err.Error()) + return nil, nil } return bg, nil diff --git a/internal/function/blame_test.go b/internal/function/blame_test.go index 4910cf7ed..aacdee15b 100644 --- a/internal/function/blame_test.go +++ b/internal/function/blame_test.go @@ -28,6 +28,7 @@ func TestBlameEval(t *testing.T) { name string repo sql.Expression commit sql.Expression + file sql.Expression row sql.Row expected BlameLine expectedNil bool @@ -38,11 +39,11 @@ func TestBlameEval(t *testing.T) { name: "init commit", repo: expression.NewGetField(0, sql.Text, "repository_id", false), commit: expression.NewGetField(1, sql.Text, "commit_hash", false), - row: sql.NewRow("worktree", "b029517f6300c2da0f4b651b8642506cd6aaf45d"), + file: expression.NewGetField(2, sql.Text, "file", false), + row: sql.NewRow("worktree", "b029517f6300c2da0f4b651b8642506cd6aaf45d", ".gitignore"), testedLine: 0, lineCount: 12, expected: BlameLine{ - ".gitignore", 0, "mcuadros@gmail.com", "*.class", @@ -53,11 +54,11 @@ func TestBlameEval(t *testing.T) { name: "changelog", repo: expression.NewGetField(0, sql.Text, "repository_id", false), commit: expression.NewGetField(1, sql.Text, "commit_hash", false), - row: sql.NewRow("worktree", "b8e471f58bcbca63b07bda20e428190409c2db47"), + file: expression.NewGetField(2, sql.Text, "file", false), + row: sql.NewRow("worktree", "b8e471f58bcbca63b07bda20e428190409c2db47", "CHANGELOG"), testedLine: 0, lineCount: 1, expected: BlameLine{ - "CHANGELOG", 0, "daniel@lordran.local", "Initial changelog", @@ -68,7 +69,8 @@ func TestBlameEval(t *testing.T) { name: "no repo", repo: expression.NewGetField(0, sql.Text, "repository_id", false), commit: expression.NewGetField(1, sql.Text, "commit_hash", false), - row: sql.NewRow("foo", "bar"), + file: expression.NewGetField(2, sql.Text, "file", false), + row: sql.NewRow("foo", "bar", "baz"), testedLine: 0, lineCount: 1, expected: BlameLine{}, @@ -78,7 +80,19 @@ func TestBlameEval(t *testing.T) { name: "no commit", repo: expression.NewGetField(0, sql.Text, "repository_id", false), commit: expression.NewGetField(1, sql.Text, "commit_hash", false), - row: sql.NewRow("worktree", "foo"), + file: expression.NewGetField(2, sql.Text, "file", false), + row: sql.NewRow("worktree", "foo", "bar"), + testedLine: 0, + lineCount: 1, + expected: BlameLine{}, + expectedNil: true, + }, + { + name: "no file", + repo: expression.NewGetField(0, sql.Text, "repository_id", false), + commit: expression.NewGetField(1, sql.Text, "commit_hash", false), + file: expression.NewGetField(2, sql.Text, "file", false), + row: sql.NewRow("worktree", "b8e471f58bcbca63b07bda20e428190409c2db47", "foo"), testedLine: 0, lineCount: 1, expected: BlameLine{}, @@ -88,7 +102,7 @@ func TestBlameEval(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - blame := NewBlame(tc.repo, tc.commit) + blame := NewBlame(tc.repo, tc.commit, tc.file) blameGen, err := blame.Eval(ctx, tc.row) require.NoError(t, err) @@ -105,9 +119,6 @@ func TestBlameEval(t *testing.T) { lineCount := 0 for i, err := bg.Next(); err == nil; i, err = bg.Next() { i := i.(BlameLine) - if i.File != tc.expected.File { - continue - } if lineCount != tc.testedLine { lineCount++ continue diff --git a/internal/function/registry.go b/internal/function/registry.go index 83e2a7bee..a059e674d 100644 --- a/internal/function/registry.go +++ b/internal/function/registry.go @@ -17,5 +17,5 @@ var Functions = []sql.Function{ sql.Function1{Name: "uast_children", Fn: NewUASTChildren}, sql.Function1{Name: "uast_imports", Fn: NewUASTImports}, sql.Function1{Name: "is_vendor", Fn: NewIsVendor}, - sql.Function2{Name: "blame", Fn: NewBlame}, + sql.Function3{Name: "blame", Fn: NewBlame}, } From eb7ee9ba6e413a9f1864487d4ff53d232c6c7322 Mon Sep 17 00:00:00 2001 From: Juanjo Alvarez Date: Tue, 29 Oct 2019 15:00:34 +0100 Subject: [PATCH 2/3] Update changelog Signed-off-by: Juanjo Alvarez --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3aa71974..0d9840eaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Add progress for each partition in SHOW PROCESSLIST ([#855](https://github.com/src-d/go-mysql-server/pull/855)) +- Change BLAME to also take a file parameter. ## [0.24.0-rc3] - 2019-10-23 From 1ab306d2228c06199019e64c17ebd00aca554d17 Mon Sep 17 00:00:00 2001 From: Juanjo Alvarez Date: Tue, 29 Oct 2019 15:12:55 +0100 Subject: [PATCH 3/3] Add context cancellation handling to Blame Signed-off-by: Juanjo Alvarez --- internal/function/blame.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/internal/function/blame.go b/internal/function/blame.go index aa2a87445..0bf9c0a2f 100644 --- a/internal/function/blame.go +++ b/internal/function/blame.go @@ -13,21 +13,34 @@ import ( ) type BlameGenerator struct { + ctx *sql.Context commit *object.Commit file string curLine int lines []*git.Line } -func NewBlameGenerator(c *object.Commit, f string) (*BlameGenerator, error) { +func NewBlameGenerator(ctx *sql.Context, c *object.Commit, f string) (*BlameGenerator, error) { result, err := git.Blame(c, f) if err != nil { return nil, err } - return &BlameGenerator{commit: c, file: f, curLine: 0, lines: result.Lines}, nil + return &BlameGenerator{ + ctx: ctx, + commit: c, + file: f, + curLine: 0, + lines: result.Lines, + }, nil } func (g *BlameGenerator) Next() (interface{}, error) { + select { + case <-g.ctx.Done(): + return nil, io.EOF + default: + } + if len(g.lines) == 0 || g.curLine >= len(g.lines) { return nil, io.EOF } @@ -124,7 +137,7 @@ func (b *Blame) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) { return nil, nil } - bg, err := NewBlameGenerator(commit, file) + bg, err := NewBlameGenerator(ctx, commit, file) if err != nil { ctx.Warn(0, err.Error()) return nil, nil