From 50d3ff23fd083002f0fd184c9f63db15f146cf30 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Tue, 1 Nov 2022 09:24:02 -0700 Subject: [PATCH 1/7] Allow multiple refs for dolt log --- go/cmd/dolt/commands/commit.go | 2 +- go/cmd/dolt/commands/log.go | 293 ++++++++---------- go/cmd/dolt/commands/merge_base.go | 1 + go/cmd/dolt/commands/status.go | 2 +- .../env/actions/commitwalk/commitwalk.go | 129 ++++---- .../env/actions/commitwalk/commitwalk_test.go | 24 +- go/libraries/doltcore/sqle/database.go | 5 +- .../doltcore/sqle/dolt_log_table_function.go | 12 +- .../doltcore/sqle/dtables/log_table.go | 5 +- go/store/datas/commit.go | 2 +- integration-tests/bats/log.bats | 13 +- integration-tests/bats/merge-base.bats | 2 +- 12 files changed, 228 insertions(+), 262 deletions(-) diff --git a/go/cmd/dolt/commands/commit.go b/go/cmd/dolt/commands/commit.go index 6866ceaf2d..bf7899164f 100644 --- a/go/cmd/dolt/commands/commit.go +++ b/go/cmd/dolt/commands/commit.go @@ -40,7 +40,7 @@ import ( ) var commitDocs = cli.CommandDocumentationContent{ - ShortDesc: "Record changes to the repository", + ShortDesc: "Record changes to the database", LongDesc: ` Stores the current contents of the staged tables in a new commit along with a log message from the user describing the changes. diff --git a/go/cmd/dolt/commands/log.go b/go/cmd/dolt/commands/log.go index 4d1799dcfd..69a5d5eea8 100644 --- a/go/cmd/dolt/commands/log.go +++ b/go/cmd/dolt/commands/log.go @@ -36,14 +36,14 @@ import ( ) type logOpts struct { - numLines int - showParents bool - minParents int - decoration string - oneLine bool - excludingCommitSpec *doltdb.CommitSpec - commitSpec *doltdb.CommitSpec - tableName string + numLines int + showParents bool + minParents int + decoration string + oneLine bool + excludingCommitSpecs []*doltdb.CommitSpec + commitSpecs []*doltdb.CommitSpec + tableName string } type logNode struct { @@ -114,17 +114,12 @@ func (cmd LogCmd) logWithLoggerFunc(ctx context.Context, commandStr string, args help, usage := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, logDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, help) - if apr.NArg() > 2 { - usage() - return 1 - } - opts, err := parseLogArgs(ctx, dEnv, apr) if err != nil { return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } - if opts.commitSpec == nil { - opts.commitSpec = dEnv.RepoStateReader().CWBHeadSpec() + if len(opts.commitSpecs) == 0 { + opts.commitSpecs = append(opts.commitSpecs, dEnv.RepoStateReader().CWBHeadSpec()) } if len(opts.tableName) > 0 { return handleErrAndExit(logTableCommits(ctx, dEnv, opts)) @@ -152,146 +147,95 @@ func parseLogArgs(ctx context.Context, dEnv *env.DoltEnv, apr *argparser.ArgPars oneLine: apr.Contains(cli.OneLineFlag), decoration: decorateOption, } - cs, notCs, tableName, err := parseRefsAndTable(ctx, apr, dEnv) + + err := opts.parseRefsAndTable(ctx, apr, dEnv) if err != nil { return nil, err } - opts.commitSpec = cs - opts.excludingCommitSpec = notCs - opts.tableName = tableName excludingRef, ok := apr.GetValue(cli.NotFlag) if ok { - if opts.excludingCommitSpec != nil { + if len(opts.excludingCommitSpecs) > 0 { return nil, fmt.Errorf("cannot use --not argument with two dots or ref with ^") } if len(opts.tableName) > 0 { return nil, fmt.Errorf("cannot use --not argument with table") } - cs, err := doltdb.NewCommitSpec(excludingRef) + notCs, err := doltdb.NewCommitSpec(excludingRef) if err != nil { return nil, fmt.Errorf("invalid commit %s\n", excludingRef) } - opts.excludingCommitSpec = cs + opts.excludingCommitSpecs = append(opts.excludingCommitSpecs, notCs) } return opts, nil } -func parseRefsAndTable(ctx context.Context, apr *argparser.ArgParseResults, dEnv *env.DoltEnv) (*doltdb.CommitSpec, *doltdb.CommitSpec, string, error) { - switch apr.NArg() { - // dolt log - case 0: - return nil, nil, "", nil - - // dolt log - case 1: - return getCommitOrTableFromString(ctx, apr.Arg(0), dEnv, true) - - // dolt log - case 2: - firstCs, firstExNotCs, _, err := getCommitOrTableFromString(ctx, apr.Arg(0), dEnv, false) - if err != nil { - return nil, nil, "", err - } - - secondCs, secondExNotCs, tableName, err := getCommitOrTableFromString(ctx, apr.Arg(1), dEnv, false) - if err != nil { - return nil, nil, "", err - } - - if len(tableName) > 0 { - if firstExNotCs != nil { - return nil, nil, "", fmt.Errorf("Providing tableName for two dot log not yet supported") - } - // dolt log - return firstCs, nil, tableName, nil - } - - if firstCs != nil && secondCs != nil { - commit, err := dEnv.DoltDB.Resolve(ctx, firstCs, dEnv.RepoStateReader().CWBHeadRef()) - if err != nil { - return nil, nil, "", err - } - - // Handles table name matching branch name (dolt log
) - exists, err := tableExists(ctx, commit, apr.Arg(1)) - if err != nil { - return nil, nil, "", err - } - - if exists { - return firstCs, nil, apr.Arg(1), nil - } - - return nil, nil, "", fmt.Errorf("Cannot provide two commit refs") // dolt log - } - if firstExNotCs != nil && secondExNotCs != nil { - return nil, nil, "", fmt.Errorf("Cannot exclude two commit refs") // dolt log ^ ^ - } - // dolt log ^ - if firstExNotCs != nil && secondCs != nil { - return secondCs, firstExNotCs, "", nil - } - // dolt log ^ - if firstCs != nil && secondExNotCs != nil { - return firstCs, secondExNotCs, "", nil - } - - return nil, nil, "", nil - - default: - return nil, nil, "", fmt.Errorf("Cannot provide more than 3 arguments") - } -} - -func getCommitOrTableFromString(ctx context.Context, str string, dEnv *env.DoltEnv, canDot bool) (*doltdb.CommitSpec, *doltdb.CommitSpec, string, error) { - // ... - if strings.Contains(str, "...") { - return nil, nil, "", fmt.Errorf("Three dot dolt log not supported") +func (opts *logOpts) parseRefsAndTable(ctx context.Context, apr *argparser.ArgParseResults, dEnv *env.DoltEnv) error { + // `dolt log` + if apr.NArg() == 0 { + return nil } - // .. - if strings.Contains(str, "..") { - if !canDot { - return nil, nil, "", fmt.Errorf("Cannot use two dot when 2 arguments provided") + // `dolt log ..` + if strings.Contains(apr.Arg(0), "..") { + if strings.Contains(apr.Arg(0), "...") { + return fmt.Errorf("three dot log not yet supported") } - refs := strings.Split(str, "..") + if apr.NArg() > 1 { + return fmt.Errorf("Cannot use two dot when 2 or more arguments provided") + } + refs := strings.Split(apr.Arg(0), "..") notCs, err := getCommitSpec(refs[0]) if err != nil { - return nil, nil, "", err + return err } cs, err := getCommitSpec(refs[1]) if err != nil { - return nil, nil, "", err + return err } - return cs, notCs, "", nil + opts.commitSpecs = append(opts.commitSpecs, cs) + opts.excludingCommitSpecs = append(opts.excludingCommitSpecs, notCs) + + return nil } - // ^ - if strings.HasPrefix(str, "^") { - commit := strings.TrimPrefix(str, "^") - notCs, err := getCommitSpec(commit) - if err != nil { - return nil, nil, "", err + seenRefs := make(map[string]bool) + + for _, arg := range apr.Args { + // ^ + if strings.HasPrefix(arg, "^") { + commit := strings.TrimPrefix(arg, "^") + notCs, err := getCommitSpec(commit) + if err != nil { + return err + } + + opts.excludingCommitSpecs = append(opts.excludingCommitSpecs, notCs) + } else { + argIsRef := actions.ValidateIsRef(ctx, arg, dEnv.DoltDB, dEnv.RepoStateReader()) + // + if argIsRef && !seenRefs[arg] { + cs, err := getCommitSpec(arg) + if err != nil { + return err + } + seenRefs[arg] = true + opts.commitSpecs = append(opts.commitSpecs, cs) + } else { + //
+ opts.tableName = arg + } } - return nil, notCs, "", err } - argIsRef := actions.ValidateIsRef(ctx, str, dEnv.DoltDB, dEnv.RepoStateReader()) - // - if argIsRef { - cs, err := getCommitSpec(str) - if err != nil { - return nil, nil, "", err - } - return cs, nil, "", nil + if len(opts.tableName) > 0 && len(opts.excludingCommitSpecs) > 0 { + return fmt.Errorf("Cannot provide table name with excluding refs") } - //
- return nil, nil, str, nil + return nil } func getCommitSpec(commit string) (*doltdb.CommitSpec, error) { @@ -303,10 +247,22 @@ func getCommitSpec(commit string) (*doltdb.CommitSpec, error) { } func logCommits(ctx context.Context, dEnv *env.DoltEnv, opts *logOpts) int { - commit, err := dEnv.DoltDB.Resolve(ctx, opts.commitSpec, dEnv.RepoStateReader().CWBHeadRef()) - if err != nil { - cli.PrintErrln(color.HiRedString("Fatal error: cannot get HEAD commit for current branch.")) - return 1 + hashes := make([]hash.Hash, len(opts.commitSpecs)) + + for i, cs := range opts.commitSpecs { + commit, err := dEnv.DoltDB.Resolve(ctx, cs, dEnv.RepoStateReader().CWBHeadRef()) + if err != nil { + cli.PrintErrln(color.HiRedString("Fatal error: cannot get HEAD commit for current branch.")) + return 1 + } + + h, err := commit.HashOf() + if err != nil { + cli.PrintErrln(color.HiRedString("Fatal error: failed to get commit hash")) + return 1 + } + + hashes[i] = h } cHashToRefs := map[hash.Hash][]string{} @@ -356,39 +312,37 @@ func logCommits(ctx context.Context, dEnv *env.DoltEnv, opts *logOpts) int { cHashToRefs[t.Hash] = append(cHashToRefs[t.Hash], tagName) } - h, err := commit.HashOf() - - if err != nil { - cli.PrintErrln(color.HiRedString("Fatal error: failed to get commit hash")) - return 1 - } - - matchFunc := func(commit *doltdb.Commit) (bool, error) { - return commit.NumParents() >= opts.minParents, nil + matchFunc := func(c *doltdb.Commit) (bool, error) { + return c.NumParents() >= opts.minParents, nil } var commits []*doltdb.Commit - if opts.excludingCommitSpec == nil { - commits, err = commitwalk.GetTopNTopoOrderedCommitsMatching(ctx, dEnv.DoltDB, h, opts.numLines, matchFunc) + if len(opts.excludingCommitSpecs) == 0 { + commits, err = commitwalk.GetTopNTopoOrderedCommitsMatching(ctx, dEnv.DoltDB, hashes, opts.numLines, matchFunc) } else { - excludingCommit, err := dEnv.DoltDB.Resolve(ctx, opts.excludingCommitSpec, dEnv.RepoStateReader().CWBHeadRef()) - if err != nil { - cli.PrintErrln(color.HiRedString("Fatal error: cannot get excluding commit for current branch.")) - return 1 + excludingHashes := make([]hash.Hash, len(opts.excludingCommitSpecs)) + + for i, excludingSpec := range opts.excludingCommitSpecs { + excludingCommit, err := dEnv.DoltDB.Resolve(ctx, excludingSpec, dEnv.RepoStateReader().CWBHeadRef()) + if err != nil { + cli.PrintErrln(color.HiRedString("Fatal error: cannot get excluding commit for current branch.")) + return 1 + } + + excludingHash, err := excludingCommit.HashOf() + if err != nil { + cli.PrintErrln(color.HiRedString("Fatal error: failed to get commit hash")) + return 1 + } + + excludingHashes[i] = excludingHash } - excludingHash, err := excludingCommit.HashOf() - - if err != nil { - cli.PrintErrln(color.HiRedString("Fatal error: failed to get commit hash")) - return 1 - } - - commits, err = commitwalk.GetDotDotRevisions(ctx, dEnv.DoltDB, h, dEnv.DoltDB, excludingHash, opts.numLines) + commits, err = commitwalk.GetDotDotRevisions(ctx, dEnv.DoltDB, hashes, dEnv.DoltDB, excludingHashes, opts.numLines) } if err != nil { - cli.PrintErrln("Error retrieving commit.") + cli.PrintErrln(err) return 1 } @@ -417,7 +371,7 @@ func logCommits(ctx context.Context, dEnv *env.DoltEnv, opts *logOpts) int { commitHash: cmHash, parentHashes: pHashes, branchNames: cHashToRefs[cmHash], - isHead: cmHash == h}) + isHead: hashIsHead(cmHash, hashes)}) } logToStdOut(opts, commitsInfo) @@ -425,6 +379,13 @@ func logCommits(ctx context.Context, dEnv *env.DoltEnv, opts *logOpts) int { return 0 } +func hashIsHead(cmHash hash.Hash, hashes []hash.Hash) bool { + if len(hashes) > 1 || len(hashes) == 0 { + return false + } + return cmHash == hashes[0] +} + func tableExists(ctx context.Context, commit *doltdb.Commit, tableName string) (bool, error) { rv, err := commit.GetRootValue(ctx) if err != nil { @@ -440,31 +401,37 @@ func tableExists(ctx context.Context, commit *doltdb.Commit, tableName string) ( } func logTableCommits(ctx context.Context, dEnv *env.DoltEnv, opts *logOpts) error { - commit, err := dEnv.DoltDB.Resolve(ctx, opts.commitSpec, dEnv.RepoStateReader().CWBHeadRef()) - if err != nil { - return err - } + hashes := make([]hash.Hash, len(opts.commitSpecs)) - // Check that the table exists in the head commit - exists, err := tableExists(ctx, commit, opts.tableName) - if err != nil { - return err - } + for i, cs := range opts.commitSpecs { + commit, err := dEnv.DoltDB.Resolve(ctx, cs, dEnv.RepoStateReader().CWBHeadRef()) + if err != nil { + return err + } - if !exists { - return fmt.Errorf("error: table %s does not exist", opts.tableName) - } + h, err := commit.HashOf() + if err != nil { + return err + } - h, err := commit.HashOf() - if err != nil { - return err + // Check that the table exists in the head commits + exists, err := tableExists(ctx, commit, opts.tableName) + if err != nil { + return err + } + + if !exists { + return fmt.Errorf("error: table %s does not exist", opts.tableName) + } + + hashes[i] = h } matchFunc := func(commit *doltdb.Commit) (bool, error) { return commit.NumParents() >= opts.minParents, nil } - itr, err := commitwalk.GetTopologicalOrderIterator(ctx, dEnv.DoltDB, h, matchFunc) + itr, err := commitwalk.GetTopologicalOrderIterator(ctx, dEnv.DoltDB, hashes, matchFunc) if err != nil && err != io.EOF { return err } diff --git a/go/cmd/dolt/commands/merge_base.go b/go/cmd/dolt/commands/merge_base.go index 6d4bc628f9..622baf4576 100644 --- a/go/cmd/dolt/commands/merge_base.go +++ b/go/cmd/dolt/commands/merge_base.go @@ -86,6 +86,7 @@ func (cmd MergeBaseCmd) Exec(ctx context.Context, commandStr string, args []stri } cli.Println(mergeBaseStr) + return 0 } diff --git a/go/cmd/dolt/commands/status.go b/go/cmd/dolt/commands/status.go index ffdf03f638..b747bbc5ba 100644 --- a/go/cmd/dolt/commands/status.go +++ b/go/cmd/dolt/commands/status.go @@ -219,7 +219,7 @@ func printRemoteRefTrackingInfo(ctx context.Context, dEnv *env.DoltEnv) error { // countCommitsInRange returns the number of commits between the given starting point to trace back to the given target point. // The starting commit must be a descendant of the target commit. Target commit must be a common ancestor commit. func countCommitsInRange(ctx context.Context, ddb *doltdb.DoltDB, startCommitHash, targetCommitHash hash.Hash) (int, error) { - itr, iErr := commitwalk.GetTopologicalOrderIterator(ctx, ddb, startCommitHash, nil) + itr, iErr := commitwalk.GetTopologicalOrderIterator(ctx, ddb, []hash.Hash{startCommitHash}, nil) if iErr != nil { return 0, iErr } diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go index 1daa9abda2..8db050cb18 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go @@ -158,71 +158,54 @@ func newQueue() *q { // ties are broken by timestamp; newer commits appear first. // // Roughly mimics `git log main..feature`. -func GetDotDotRevisions(ctx context.Context, includedDB *doltdb.DoltDB, includedHead hash.Hash, excludedDB *doltdb.DoltDB, excludedHead hash.Hash, num int) ([]*doltdb.Commit, error) { +func GetDotDotRevisions(ctx context.Context, includedDB *doltdb.DoltDB, includedHeads []hash.Hash, excludedDB *doltdb.DoltDB, excludedHeads []hash.Hash, num int) ([]*doltdb.Commit, error) { + itr, err := GetDotDotRevisionsIterator(ctx, includedDB, includedHeads, excludedDB, excludedHeads, nil) + if err != nil { + return nil, err + } + var commitList []*doltdb.Commit - q := newQueue() - if err := q.SetInvisible(ctx, excludedDB, excludedHead); err != nil { - return nil, err - } - if err := q.AddPendingIfUnseen(ctx, excludedDB, excludedHead); err != nil { - return nil, err - } - if err := q.AddPendingIfUnseen(ctx, includedDB, includedHead); err != nil { - return nil, err - } - for q.NumVisiblePending() > 0 { - nextC := q.PopPending() - parents, err := nextC.commit.ParentHashes(ctx) - if err != nil { + for num < 0 || len(commitList) < num { + _, commit, err := itr.Next(ctx) + if err == io.EOF { + break + } else if err != nil { return nil, err } - for _, parentID := range parents { - if nextC.invisible { - if err := q.SetInvisible(ctx, nextC.ddb, parentID); err != nil { - return nil, err - } - } - if err := q.AddPendingIfUnseen(ctx, nextC.ddb, parentID); err != nil { - return nil, err - } - } - if !nextC.invisible { - commitList = append(commitList, nextC.commit) - if len(commitList) == num { - return commitList, nil - } - } + + commitList = append(commitList, commit) } + return commitList, nil } -// GetTopologicalOrderCommits returns the commits reachable from the commit at hash `startCommitHash` +// GetTopologicalOrderCommits returns the commits reachable from the commits in `startCommitHashes` // in reverse topological order, with tiebreaking done by the height of the commit graph -- higher commits // appear first. Remaining ties are broken by timestamp; newer commits appear first. -func GetTopologicalOrderCommits(ctx context.Context, ddb *doltdb.DoltDB, startCommitHash hash.Hash) ([]*doltdb.Commit, error) { - return GetTopNTopoOrderedCommitsMatching(ctx, ddb, startCommitHash, -1, nil) +func GetTopologicalOrderCommits(ctx context.Context, ddb *doltdb.DoltDB, startCommitHashes []hash.Hash) ([]*doltdb.Commit, error) { + return GetTopNTopoOrderedCommitsMatching(ctx, ddb, startCommitHashes, -1, nil) } // GetTopologicalOrderCommitIterator returns an iterator for commits generated with the same semantics as // GetTopologicalOrderCommits -func GetTopologicalOrderIterator(ctx context.Context, ddb *doltdb.DoltDB, startCommitHash hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (doltdb.CommitItr, error) { - return newCommiterator(ctx, ddb, startCommitHash, matchFn) +func GetTopologicalOrderIterator(ctx context.Context, ddb *doltdb.DoltDB, startCommitHashes []hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (doltdb.CommitItr, error) { + return newCommiterator(ctx, ddb, startCommitHashes, matchFn) } type commiterator struct { - ddb *doltdb.DoltDB - startCommitHash hash.Hash - matchFn func(*doltdb.Commit) (bool, error) - q *q + ddb *doltdb.DoltDB + startCommitHashes []hash.Hash + matchFn func(*doltdb.Commit) (bool, error) + q *q } var _ doltdb.CommitItr = (*commiterator)(nil) -func newCommiterator(ctx context.Context, ddb *doltdb.DoltDB, startCommitHash hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (*commiterator, error) { +func newCommiterator(ctx context.Context, ddb *doltdb.DoltDB, startCommitHashes []hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (*commiterator, error) { itr := &commiterator{ - ddb: ddb, - startCommitHash: startCommitHash, - matchFn: matchFn, + ddb: ddb, + startCommitHashes: startCommitHashes, + matchFn: matchFn, } err := itr.Reset(ctx) @@ -270,17 +253,19 @@ func (i *commiterator) Next(ctx context.Context) (hash.Hash, *doltdb.Commit, err // Reset implements doltdb.CommitItr func (i *commiterator) Reset(ctx context.Context) error { i.q = newQueue() - if err := i.q.AddPendingIfUnseen(ctx, i.ddb, i.startCommitHash); err != nil { - return err + for _, startCommitHash := range i.startCommitHashes { + if err := i.q.AddPendingIfUnseen(ctx, i.ddb, startCommitHash); err != nil { + return err + } } return nil } -// GetTopNTopoOrderedCommitsMatching returns the first N commits (If N <= 0 then all commits) reachable from the commit at hash -// `startCommitHash` in reverse topological order, with tiebreaking done by the height of the commit graph -- higher +// GetTopNTopoOrderedCommitsMatching returns the first N commits (If N <= 0 then all commits) reachable from the commits in +// `startCommitHashes` in reverse topological order, with tiebreaking done by the height of the commit graph -- higher // commits appear first. Remaining ties are broken by timestamp; newer commits appear first. -func GetTopNTopoOrderedCommitsMatching(ctx context.Context, ddb *doltdb.DoltDB, startCommitHash hash.Hash, n int, matchFn func(*doltdb.Commit) (bool, error)) ([]*doltdb.Commit, error) { - itr, err := GetTopologicalOrderIterator(ctx, ddb, startCommitHash, matchFn) +func GetTopNTopoOrderedCommitsMatching(ctx context.Context, ddb *doltdb.DoltDB, startCommitHashes []hash.Hash, n int, matchFn func(*doltdb.Commit) (bool, error)) ([]*doltdb.Commit, error) { + itr, err := GetTopologicalOrderIterator(ctx, ddb, startCommitHashes, matchFn) if err != nil { return nil, err } @@ -302,26 +287,28 @@ func GetTopNTopoOrderedCommitsMatching(ctx context.Context, ddb *doltdb.DoltDB, // GetDotDotRevisionsIterator returns an iterator for commits generated with the same semantics as // GetDotDotRevisions -func GetDotDotRevisionsIterator(ctx context.Context, ddb *doltdb.DoltDB, startCommitHash, excludingCommitHash hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (doltdb.CommitItr, error) { - return newDotDotCommiterator(ctx, ddb, startCommitHash, excludingCommitHash, matchFn) +func GetDotDotRevisionsIterator(ctx context.Context, includedDdb *doltdb.DoltDB, startCommitHashes []hash.Hash, excludedDdb *doltdb.DoltDB, excludingCommitHashes []hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (doltdb.CommitItr, error) { + return newDotDotCommiterator(ctx, includedDdb, startCommitHashes, excludedDdb, excludingCommitHashes, matchFn) } type dotDotCommiterator struct { - ddb *doltdb.DoltDB - startCommitHash hash.Hash - excludingCommitHash hash.Hash - matchFn func(*doltdb.Commit) (bool, error) - q *q + includedDdb *doltdb.DoltDB + excludedDdb *doltdb.DoltDB + startCommitHashes []hash.Hash + excludingCommitHashes []hash.Hash + matchFn func(*doltdb.Commit) (bool, error) + q *q } var _ doltdb.CommitItr = (*dotDotCommiterator)(nil) -func newDotDotCommiterator(ctx context.Context, ddb *doltdb.DoltDB, startCommitHash, excludingCommitHash hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (*dotDotCommiterator, error) { +func newDotDotCommiterator(ctx context.Context, includedDdb *doltdb.DoltDB, startCommitHashes []hash.Hash, excludedDdb *doltdb.DoltDB, excludingCommitHashes []hash.Hash, matchFn func(*doltdb.Commit) (bool, error)) (*dotDotCommiterator, error) { itr := &dotDotCommiterator{ - ddb: ddb, - startCommitHash: startCommitHash, - excludingCommitHash: excludingCommitHash, - matchFn: matchFn, + includedDdb: includedDdb, + excludedDdb: excludedDdb, + startCommitHashes: startCommitHashes, + excludingCommitHashes: excludingCommitHashes, + matchFn: matchFn, } err := itr.Reset(ctx) @@ -373,14 +360,18 @@ func (i *dotDotCommiterator) Next(ctx context.Context) (hash.Hash, *doltdb.Commi // Reset implements doltdb.CommitItr func (i *dotDotCommiterator) Reset(ctx context.Context) error { i.q = newQueue() - if err := i.q.SetInvisible(ctx, i.ddb, i.excludingCommitHash); err != nil { - return err + for _, excludingCommitHash := range i.excludingCommitHashes { + if err := i.q.SetInvisible(ctx, i.excludedDdb, excludingCommitHash); err != nil { + return err + } + if err := i.q.AddPendingIfUnseen(ctx, i.excludedDdb, excludingCommitHash); err != nil { + return err + } } - if err := i.q.AddPendingIfUnseen(ctx, i.ddb, i.excludingCommitHash); err != nil { - return err - } - if err := i.q.AddPendingIfUnseen(ctx, i.ddb, i.startCommitHash); err != nil { - return err + for _, startCommitHash := range i.startCommitHashes { + if err := i.q.AddPendingIfUnseen(ctx, i.includedDdb, startCommitHash); err != nil { + return err + } } return nil } diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go index a3a96dfd8d..1539edf9ef 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go @@ -121,7 +121,7 @@ func TestGetDotDotRevisions(t *testing.T) { mainHash := mustGetHash(t, mainCommits[6]) featurePreMergeHash := mustGetHash(t, featureCommits[3]) - res, err := GetDotDotRevisions(context.Background(), dEnv.DoltDB, featureHash, dEnv.DoltDB, mainHash, 100) + res, err := GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featureHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 100) require.NoError(t, err) assert.Len(t, res, 7) @@ -133,25 +133,25 @@ func TestGetDotDotRevisions(t *testing.T) { assertEqualHashes(t, featureCommits[2], res[5]) assertEqualHashes(t, featureCommits[1], res[6]) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, mainHash, dEnv.DoltDB, featureHash, 100) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{mainHash}, dEnv.DoltDB, []hash.Hash{featureHash}, 100) require.NoError(t, err) assert.Len(t, res, 0) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, featureHash, dEnv.DoltDB, mainHash, 3) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featureHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 3) require.NoError(t, err) assert.Len(t, res, 3) assertEqualHashes(t, featureCommits[7], res[0]) assertEqualHashes(t, featureCommits[6], res[1]) assertEqualHashes(t, featureCommits[5], res[2]) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, featurePreMergeHash, dEnv.DoltDB, mainHash, 3) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featurePreMergeHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 3) require.NoError(t, err) assert.Len(t, res, 3) assertEqualHashes(t, featureCommits[3], res[0]) assertEqualHashes(t, featureCommits[2], res[1]) assertEqualHashes(t, featureCommits[1], res[2]) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, featurePreMergeHash, dEnv.DoltDB, mainHash, 3) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featurePreMergeHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 3) require.NoError(t, err) assert.Len(t, res, 3) assertEqualHashes(t, featureCommits[3], res[0]) @@ -170,9 +170,9 @@ func TestGetDotDotRevisions(t *testing.T) { mainHash = mustGetHash(t, mainCommits[6]) featurePreMergeHash = mustGetHash(t, featureCommits[3]) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, featureHash, dEnv.DoltDB, mainHash, 100) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featureHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 100) require.Error(t, err) - res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, featureHash, dEnv.DoltDB, mainHash, 100) + res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, []hash.Hash{featureHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 100) require.NoError(t, err) assert.Len(t, res, 7) assertEqualHashes(t, featureCommits[7], res[0]) @@ -183,27 +183,27 @@ func TestGetDotDotRevisions(t *testing.T) { assertEqualHashes(t, featureCommits[2], res[5]) assertEqualHashes(t, featureCommits[1], res[6]) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, mainHash, dEnv.DoltDB, featureHash, 100) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{mainHash}, dEnv.DoltDB, []hash.Hash{featureHash}, 100) require.Error(t, err) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, mainHash, forkEnv.DoltDB, featureHash, 100) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{mainHash}, forkEnv.DoltDB, []hash.Hash{featureHash}, 100) require.NoError(t, err) assert.Len(t, res, 0) - res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, featureHash, dEnv.DoltDB, mainHash, 3) + res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, []hash.Hash{featureHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 3) require.NoError(t, err) assert.Len(t, res, 3) assertEqualHashes(t, featureCommits[7], res[0]) assertEqualHashes(t, featureCommits[6], res[1]) assertEqualHashes(t, featureCommits[5], res[2]) - res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, featurePreMergeHash, dEnv.DoltDB, mainHash, 3) + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featurePreMergeHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 3) require.NoError(t, err) assert.Len(t, res, 3) assertEqualHashes(t, featureCommits[3], res[0]) assertEqualHashes(t, featureCommits[2], res[1]) assertEqualHashes(t, featureCommits[1], res[2]) - res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, featurePreMergeHash, dEnv.DoltDB, mainHash, 3) + res, err = GetDotDotRevisions(context.Background(), forkEnv.DoltDB, []hash.Hash{featurePreMergeHash}, dEnv.DoltDB, []hash.Hash{mainHash}, 3) require.NoError(t, err) assert.Len(t, res, 3) assertEqualHashes(t, featureCommits[3], res[0]) diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index fe825f42ad..6aaa30df67 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -37,6 +37,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/sqle/globalstate" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/sqlutil" "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" + "github.com/dolthub/dolt/go/store/hash" ) var ErrInvalidTableName = errors.NewKind("Invalid table name %s. Table names must match the regular expression " + doltdb.TableNameRegexStr) @@ -511,12 +512,12 @@ func resolveAsOfTime(ctx *sql.Context, ddb *doltdb.DoltDB, head ref.DoltRef, asO return nil, nil, err } - hash, err := cm.HashOf() + h, err := cm.HashOf() if err != nil { return nil, nil, err } - cmItr, err := commitwalk.GetTopologicalOrderIterator(ctx, ddb, hash, nil) + cmItr, err := commitwalk.GetTopologicalOrderIterator(ctx, ddb, []hash.Hash{h}, nil) if err != nil { return nil, nil, err } diff --git a/go/libraries/doltcore/sqle/dolt_log_table_function.go b/go/libraries/doltcore/sqle/dolt_log_table_function.go index b0bf57dacd..7e6ed6bd0f 100644 --- a/go/libraries/doltcore/sqle/dolt_log_table_function.go +++ b/go/libraries/doltcore/sqle/dolt_log_table_function.go @@ -547,12 +547,12 @@ type logTableFunctionRowIter struct { } func (ltf *LogTableFunction) NewLogTableFunctionRowIter(ctx *sql.Context, ddb *doltdb.DoltDB, commit *doltdb.Commit, matchFn func(*doltdb.Commit) (bool, error), cHashToRefs map[hash.Hash][]string) (*logTableFunctionRowIter, error) { - hash, err := commit.HashOf() + h, err := commit.HashOf() if err != nil { return nil, err } - child, err := commitwalk.GetTopologicalOrderIterator(ctx, ddb, hash, matchFn) + child, err := commitwalk.GetTopologicalOrderIterator(ctx, ddb, []hash.Hash{h}, matchFn) if err != nil { return nil, err } @@ -562,12 +562,12 @@ func (ltf *LogTableFunction) NewLogTableFunctionRowIter(ctx *sql.Context, ddb *d showParents: ltf.showParents, decoration: ltf.decoration, cHashToRefs: cHashToRefs, - headHash: hash, + headHash: h, }, nil } func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, ddb *doltdb.DoltDB, commit, excludingCommit *doltdb.Commit, matchFn func(*doltdb.Commit) (bool, error), cHashToRefs map[hash.Hash][]string) (*logTableFunctionRowIter, error) { - hash, err := commit.HashOf() + h, err := commit.HashOf() if err != nil { return nil, err } @@ -577,7 +577,7 @@ func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, return nil, err } - child, err := commitwalk.GetDotDotRevisionsIterator(ctx, ddb, hash, exHash, matchFn) + child, err := commitwalk.GetDotDotRevisionsIterator(ctx, ddb, []hash.Hash{h}, ddb, []hash.Hash{exHash}, matchFn) if err != nil { return nil, err } @@ -587,7 +587,7 @@ func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, showParents: ltf.showParents, decoration: ltf.decoration, cHashToRefs: cHashToRefs, - headHash: hash, + headHash: h, }, nil } diff --git a/go/libraries/doltcore/sqle/dtables/log_table.go b/go/libraries/doltcore/sqle/dtables/log_table.go index cb1418e194..d4aab1c506 100644 --- a/go/libraries/doltcore/sqle/dtables/log_table.go +++ b/go/libraries/doltcore/sqle/dtables/log_table.go @@ -18,6 +18,7 @@ import ( "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/libraries/doltcore/env/actions/commitwalk" + "github.com/dolthub/dolt/go/store/hash" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/index" @@ -81,12 +82,12 @@ type LogItr struct { // NewLogItr creates a LogItr from the current environment. func NewLogItr(ctx *sql.Context, ddb *doltdb.DoltDB, head *doltdb.Commit) (*LogItr, error) { - hash, err := head.HashOf() + h, err := head.HashOf() if err != nil { return nil, err } - child, err := commitwalk.GetTopologicalOrderIterator(ctx, ddb, hash, nil) + child, err := commitwalk.GetTopologicalOrderIterator(ctx, ddb, []hash.Hash{h}, nil) if err != nil { return nil, err } diff --git a/go/store/datas/commit.go b/go/store/datas/commit.go index 61f5f04d35..417e017040 100644 --- a/go/store/datas/commit.go +++ b/go/store/datas/commit.go @@ -363,7 +363,7 @@ func findCommonAncestorUsingParentsList(ctx context.Context, c1, c2 *Commit, vr1 // FindCommonAncestor returns the most recent common ancestor of c1 and c2, if // one exists, setting ok to true. If there is no common ancestor, ok is set // to false. Refs of |c1| are dereferenced through |vr1|, while refs of |c2| -// are dereference through |vr2|. +// are dereferenced through |vr2|. // // This implementation makes use of the parents_closure field on the commit // struct. If the commit does not have a materialized parents_closure, this diff --git a/integration-tests/bats/log.bats b/integration-tests/bats/log.bats index 64b6295a1c..211a0dfcec 100755 --- a/integration-tests/bats/log.bats +++ b/integration-tests/bats/log.bats @@ -105,6 +105,13 @@ teardown() { [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false [[ "$output" =~ "BRANCH1" ]] || false + run dolt log ^main ^branch1 + [ $status -eq 0 ] + run dolt log main branch1 + [ $status -eq 0 ] + [[ "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCH1" ]] || false # Invalid two dot run dolt log main..branch1 testtable @@ -119,12 +126,10 @@ teardown() { [ $status -eq 1 ] run dolt log main main..branch1 [ $status -eq 1 ] - run dolt log ^main ^branch1 - [ $status -eq 1 ] - run dolt log main branch1 - [ $status -eq 1 ] run dolt log ^main testtable [ $status -eq 1 ] + run dolt log testtable ^main + [ $status -eq 1 ] run dolt log ^branch1 --not main [ $status -eq 1 ] run dolt log main..branch1 --not main diff --git a/integration-tests/bats/merge-base.bats b/integration-tests/bats/merge-base.bats index 92d3d1cf7e..f9b18c5b15 100644 --- a/integration-tests/bats/merge-base.bats +++ b/integration-tests/bats/merge-base.bats @@ -27,7 +27,7 @@ setup() { # / # # / <-- (one) # # / / # - # (init) -- (A) -- (B) -- (C) <-- (main) # + # (init) -- (A) -- (B) -- (C) <-- (main) # # \ # # -- (D) <-- (two) # # # From f7246f854b10d1b510e23b93c1ceeffd583e155a Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Tue, 1 Nov 2022 17:03:12 -0700 Subject: [PATCH 2/7] Supply multiple refs to --not --- go/cmd/dolt/cli/arg_parser_helpers.go | 2 +- go/cmd/dolt/commands/log.go | 13 +- .../env/actions/commitwalk/commitwalk.go | 8 +- go/libraries/utils/argparser/args_test.go | 14 +- go/libraries/utils/argparser/option.go | 2 + go/libraries/utils/argparser/parser.go | 43 +++++- go/libraries/utils/argparser/results.go | 6 + integration-tests/bats/log.bats | 139 +++++++++++++----- 8 files changed, 167 insertions(+), 60 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 0f39ab788c..b4447d7378 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -302,7 +302,7 @@ func CreateLogArgParser() *argparser.ArgParser { ap.SupportsFlag(ParentsFlag, "", "Shows all parents of each commit in the log.") ap.SupportsString(DecorateFlag, "", "decorate_fmt", "Shows refs next to commits. Valid options are short, full, no, and auto") ap.SupportsFlag(OneLineFlag, "", "Shows logs in a compact format.") - ap.SupportsString(NotFlag, "", "revision", "Excludes commits from revision.") + ap.SupportsStringList(NotFlag, "", "revision", "Excludes commits from revision.") return ap } diff --git a/go/cmd/dolt/commands/log.go b/go/cmd/dolt/commands/log.go index 69a5d5eea8..294f86ad45 100644 --- a/go/cmd/dolt/commands/log.go +++ b/go/cmd/dolt/commands/log.go @@ -153,7 +153,7 @@ func parseLogArgs(ctx context.Context, dEnv *env.DoltEnv, apr *argparser.ArgPars return nil, err } - excludingRef, ok := apr.GetValue(cli.NotFlag) + excludingRefs, ok := apr.GetValueList(cli.NotFlag) if ok { if len(opts.excludingCommitSpecs) > 0 { return nil, fmt.Errorf("cannot use --not argument with two dots or ref with ^") @@ -161,11 +161,14 @@ func parseLogArgs(ctx context.Context, dEnv *env.DoltEnv, apr *argparser.ArgPars if len(opts.tableName) > 0 { return nil, fmt.Errorf("cannot use --not argument with table") } - notCs, err := doltdb.NewCommitSpec(excludingRef) - if err != nil { - return nil, fmt.Errorf("invalid commit %s\n", excludingRef) + for _, excludingRef := range excludingRefs { + notCs, err := doltdb.NewCommitSpec(excludingRef) + if err != nil { + return nil, fmt.Errorf("invalid commit %s\n", excludingRef) + } + + opts.excludingCommitSpecs = append(opts.excludingCommitSpecs, notCs) } - opts.excludingCommitSpecs = append(opts.excludingCommitSpecs, notCs) } return opts, nil diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go index 8db050cb18..dd16802ce7 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go @@ -149,10 +149,10 @@ func newQueue() *q { return &q{loaded: make(map[hash.Hash]*c)} } -// GetDotDotRevisions returns the commits reachable from commit at hash -// `includedHead` that are not reachable from hash `excludedHead`. -// `includedHead` and `excludedHead` must be commits in `ddb`. Returns up -// to `num` commits, in reverse topological order starting at `includedHead`, +// GetDotDotRevisions returns the commits reachable from commit at hashes +// `includedHeads` that are not reachable from hashes `excludedHeads`. +// `includedHeads` and `excludedHeads` must be commits in `ddb`. Returns up +// to `num` commits, in reverse topological order starting at `includedHeads`, // with tie breaking based on the height of commit graph between // concurrent commits --- higher commits appear first. Remaining // ties are broken by timestamp; newer commits appear first. diff --git a/go/libraries/utils/argparser/args_test.go b/go/libraries/utils/argparser/args_test.go index 2dd7f5cad2..5fd7d8a23f 100644 --- a/go/libraries/utils/argparser/args_test.go +++ b/go/libraries/utils/argparser/args_test.go @@ -22,9 +22,10 @@ import ( "github.com/stretchr/testify/require" ) -var forceOpt = &Option{"force", "f", "", OptionalFlag, "force desc", nil} -var messageOpt = &Option{"message", "m", "msg", OptionalValue, "msg desc", nil} -var fileTypeOpt = &Option{"file-type", "", "", OptionalValue, "file type", nil} +var forceOpt = &Option{"force", "f", "", OptionalFlag, "force desc", nil, false} +var messageOpt = &Option{"message", "m", "msg", OptionalValue, "msg desc", nil, false} +var fileTypeOpt = &Option{"file-type", "", "", OptionalValue, "file type", nil, false} +var notOpt = &Option{"not", "", "", OptionalValue, "not desc", nil, true} func TestParsing(t *testing.T) { tests := []struct { @@ -158,6 +159,13 @@ func TestParsing(t *testing.T) { expectedOpts: map[string]string{"message": "f"}, expectedArgs: []string{"value"}, }, + { + name: "--not string list value", + options: []*Option{forceOpt, messageOpt, notOpt}, + args: []string{"-mf", "value", "--not", "main", "branch"}, + expectedOpts: map[string]string{"message": "f", "not": "main,branch"}, + expectedArgs: []string{"value"}, + }, { name: "-fm value", options: []*Option{forceOpt, messageOpt}, diff --git a/go/libraries/utils/argparser/option.go b/go/libraries/utils/argparser/option.go index 0434e23b2b..eaadf9511d 100644 --- a/go/libraries/utils/argparser/option.go +++ b/go/libraries/utils/argparser/option.go @@ -63,4 +63,6 @@ type Option struct { Desc string // Function to validate an Option after parsing, returning any error. Validator ValidationFunc + // Allows more than one arg to an Option. + AllowMultipleOptions bool } diff --git a/go/libraries/utils/argparser/parser.go b/go/libraries/utils/argparser/parser.go index 2ea2d9bb0e..0c2912c33b 100644 --- a/go/libraries/utils/argparser/parser.go +++ b/go/libraries/utils/argparser/parser.go @@ -89,7 +89,7 @@ func (ap *ArgParser) SupportOption(opt *Option) { // SupportsFlag adds support for a new flag (argument with no value). See SupportOpt for details on params. func (ap *ArgParser) SupportsFlag(name, abbrev, desc string) *ArgParser { - opt := &Option{name, abbrev, "", OptionalFlag, desc, nil} + opt := &Option{name, abbrev, "", OptionalFlag, desc, nil, false} ap.SupportOption(opt) return ap @@ -97,7 +97,15 @@ func (ap *ArgParser) SupportsFlag(name, abbrev, desc string) *ArgParser { // SupportsString adds support for a new string argument with the description given. See SupportOpt for details on params. func (ap *ArgParser) SupportsString(name, abbrev, valDesc, desc string) *ArgParser { - opt := &Option{name, abbrev, valDesc, OptionalValue, desc, nil} + opt := &Option{name, abbrev, valDesc, OptionalValue, desc, nil, false} + ap.SupportOption(opt) + + return ap +} + +// SupportsStringList adds support for a new string list argument with the description given. See SupportOpt for details on params. +func (ap *ArgParser) SupportsStringList(name, abbrev, valDesc, desc string) *ArgParser { + opt := &Option{name, abbrev, valDesc, OptionalValue, desc, nil, true} ap.SupportOption(opt) return ap @@ -105,7 +113,7 @@ func (ap *ArgParser) SupportsString(name, abbrev, valDesc, desc string) *ArgPars // SupportsOptionalString adds support for a new string argument with the description given and optional empty value. func (ap *ArgParser) SupportsOptionalString(name, abbrev, valDesc, desc string) *ArgParser { - opt := &Option{name, abbrev, valDesc, OptionalEmptyValue, desc, nil} + opt := &Option{name, abbrev, valDesc, OptionalEmptyValue, desc, nil, false} ap.SupportOption(opt) return ap @@ -113,7 +121,7 @@ func (ap *ArgParser) SupportsOptionalString(name, abbrev, valDesc, desc string) // SupportsValidatedString adds support for a new string argument with the description given and defined validation function. func (ap *ArgParser) SupportsValidatedString(name, abbrev, valDesc, desc string, validator ValidationFunc) *ArgParser { - opt := &Option{name, abbrev, valDesc, OptionalValue, desc, validator} + opt := &Option{name, abbrev, valDesc, OptionalValue, desc, validator, false} ap.SupportOption(opt) return ap @@ -121,7 +129,7 @@ func (ap *ArgParser) SupportsValidatedString(name, abbrev, valDesc, desc string, // SupportsUint adds support for a new uint argument with the description given. See SupportOpt for details on params. func (ap *ArgParser) SupportsUint(name, abbrev, valDesc, desc string) *ArgParser { - opt := &Option{name, abbrev, valDesc, OptionalValue, desc, isUintStr} + opt := &Option{name, abbrev, valDesc, OptionalValue, desc, isUintStr, false} ap.SupportOption(opt) return ap @@ -129,7 +137,7 @@ func (ap *ArgParser) SupportsUint(name, abbrev, valDesc, desc string) *ArgParser // SupportsInt adds support for a new int argument with the description given. See SupportOpt for details on params. func (ap *ArgParser) SupportsInt(name, abbrev, valDesc, desc string) *ArgParser { - opt := &Option{name, abbrev, valDesc, OptionalValue, desc, isIntStr} + opt := &Option{name, abbrev, valDesc, OptionalValue, desc, isIntStr, false} ap.SupportOption(opt) return ap @@ -280,9 +288,14 @@ func (ap *ArgParser) Parse(args []string) (*ArgParseResults, error) { return nil, errors.New("error: no value for option `" + opt.Name + "'") } } else { - valueStr = args[i] + if opt.AllowMultipleOptions { + list := getListValues(args[i:]) + valueStr = strings.Join(list, ",") + i += len(list) - 1 + } else { + valueStr = args[i] + } } - value = &valueStr } @@ -303,3 +316,17 @@ func (ap *ArgParser) Parse(args []string) (*ArgParseResults, error) { return &ArgParseResults{results, list, ap}, nil } + +func getListValues(args []string) []string { + var values []string + + for _, arg := range args { + // Stop if another option found + if arg[0] == '-' || arg == "--" { + return values + } + values = append(values, arg) + } + + return values +} diff --git a/go/libraries/utils/argparser/results.go b/go/libraries/utils/argparser/results.go index 9288495b39..b553e0e10e 100644 --- a/go/libraries/utils/argparser/results.go +++ b/go/libraries/utils/argparser/results.go @@ -17,6 +17,7 @@ package argparser import ( "math" "strconv" + "strings" "github.com/dolthub/dolt/go/libraries/utils/set" ) @@ -96,6 +97,11 @@ func (res *ArgParseResults) GetValue(name string) (string, bool) { return val, ok } +func (res *ArgParseResults) GetValueList(name string) ([]string, bool) { + val, ok := res.options[name] + return strings.Split(val, ","), ok +} + func (res *ArgParseResults) GetValues(names ...string) map[string]string { vals := make(map[string]string) diff --git a/integration-tests/bats/log.bats b/integration-tests/bats/log.bats index 211a0dfcec..5afdff0548 100755 --- a/integration-tests/bats/log.bats +++ b/integration-tests/bats/log.bats @@ -45,101 +45,162 @@ teardown() { dolt add . dolt commit -m "commit 1 MAIN" dolt commit --allow-empty -m "commit 2 MAIN" - dolt checkout -b branch1 - dolt commit --allow-empty -m "commit 1 BRANCH1" - dolt commit --allow-empty -m "commit 2 BRANCH1" - dolt commit --allow-empty -m "commit 3 BRANCH1" + dolt checkout -b branchA + dolt commit --allow-empty -m "commit 1 BRANCHA" + dolt commit --allow-empty -m "commit 2 BRANCHA" + dolt checkout -b branchB + dolt commit --allow-empty -m "commit 1 BRANCHB" + dolt checkout branchA + dolt commit --allow-empty -m "commit 3 BRANCHA" - run dolt log branch1 + run dolt log branchA [ $status -eq 0 ] [[ "$output" =~ "MAIN" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false run dolt log main [ $status -eq 0 ] [[ "$output" =~ "MAIN" ]] || false - [[ ! "$output" =~ "BRANCH1" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false dolt checkout main dolt commit --allow-empty -m "commit 3 AFTER" + # # # # # # # # # # # # # # # # # # # # # # # + # # + # 1B (branchB) # + # / # + # 1A - 2A - 3A (branchA) # + # / # + # (init) - 1M - 2M - 3M (main) # + # # + # # # # # # # # # # # # # # # # # # # # # # # + # Valid two dot - run dolt log main..branch1 + run dolt log main..branchA [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false - run dolt log ^main branch1 + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log ^main branchA [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false - run dolt log branch1 ^main + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log branchA ^main [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false - run dolt log branch1 --not main + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log branchA --not main [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false - run dolt log branch1..main + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log branchA..main [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ "$output" =~ "AFTER" ]] || false - [[ ! "$output" =~ "BRANCH1" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false run dolt log main..main [ $status -eq 0 ] - run dolt log main^..branch1 + run dolt log main^..branchA [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false - run dolt log ^main^ branch1 + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log ^main^ branchA [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false - run dolt log branch1 --not main^ + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log branchA --not main^ [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false [[ ! "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false - run dolt log ^main ^branch1 + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log ^main ^branchA [ $status -eq 0 ] - run dolt log main branch1 + run dolt log main branchA [ $status -eq 0 ] [[ "$output" =~ "MAIN" ]] || false [[ "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCH1" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false - # Invalid two dot - run dolt log main..branch1 testtable + # Multiple refs + run dolt log branchB branchA + [ $status -eq 0 ] + [[ "$output" =~ "MAIN" ]] || false + [[ ! "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + run dolt log main branchA + [ $status -eq 0 ] + [[ "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false + [[ ! "$output" =~ "BRANCHB" ]] || false + run dolt log branchB main ^branchA + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + run dolt log branchB main --not branchA + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + run dolt log branchB main --not branchA --oneline + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + run dolt log branchB main ^branchA ^main + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ ! "$output" =~ "AFTER" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + run dolt log branchB main --not branchA main + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ ! "$output" =~ "AFTER" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + run dolt log branchB main --not branchA main --oneline + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ ! "$output" =~ "AFTER" ]] || false + [[ ! "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + + # Invalid + run dolt log main..branchA testtable [ $status -eq 1 ] - run dolt log testtable main..branch1 + run dolt log testtable main..branchA [ $status -eq 1 ] - run dolt log ^main branch1 testtable + run dolt log ^main branchA testtable [ $status -eq 1 ] - run dolt log branch1 testtable --not main + run dolt log branchA testtable --not main [ $status -eq 1 ] - run dolt log main..branch1 main + run dolt log main..branchA main [ $status -eq 1 ] - run dolt log main main..branch1 + run dolt log main main..branchA [ $status -eq 1 ] run dolt log ^main testtable [ $status -eq 1 ] run dolt log testtable ^main [ $status -eq 1 ] - run dolt log ^branch1 --not main + run dolt log ^branchA --not main [ $status -eq 1 ] - run dolt log main..branch1 --not main + run dolt log main..branchA --not main [ $status -eq 1 ] - run dolt log branch1 --not main branch1 - [ $status -eq 1 ] - run dolt log branch1 --not main --not branch1 + run dolt log main..branchA --not ^main [ $status -eq 1 ] - run dolt log main...branch1 + run dolt log main...branchA [ $status -eq 1 ] } From 328a5f6e76e469299b19302c20df4d743a3a748d Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Wed, 2 Nov 2022 14:51:33 -0700 Subject: [PATCH 3/7] Docs --- go/cmd/dolt/commands/log.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/cmd/dolt/commands/log.go b/go/cmd/dolt/commands/log.go index 294f86ad45..b65d1a82e0 100644 --- a/go/cmd/dolt/commands/log.go +++ b/go/cmd/dolt/commands/log.go @@ -63,11 +63,11 @@ The command takes options to control what is shown and how. {{.EmphasisLeft}}dolt log{{.EmphasisRight}} Lists commit logs from current HEAD when no options provided. -{{.EmphasisLeft}}dolt log {{.EmphasisRight}} - Lists commit logs starting from revision. +{{.EmphasisLeft}}dolt log [...]{{.EmphasisRight}} + Lists commit logs starting from revision. If multiple revisions provided, lists logs reachable by all revisions. -{{.EmphasisLeft}}dolt log
{{.EmphasisRight}} - Lists commit logs starting from revision, only including commits with changes to table. +{{.EmphasisLeft}}dolt log [
{{.EmphasisRight}} + Lists commit logs starting from revisions, only including commits with changes to table. {{.EmphasisLeft}}dolt log ..{{.EmphasisRight}} {{.EmphasisLeft}}dolt log --not {{.EmphasisRight}} From a271c4dc20a4fe7153d9c8d3dc1446cbf31724ea Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Wed, 2 Nov 2022 13:55:05 -0700 Subject: [PATCH 4/7] Three dot log CLI --- go/cmd/dolt/commands/log.go | 39 ++++++++++++++++++++++----- integration-tests/bats/log.bats | 47 ++++++++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/go/cmd/dolt/commands/log.go b/go/cmd/dolt/commands/log.go index b65d1a82e0..ca4d900c39 100644 --- a/go/cmd/dolt/commands/log.go +++ b/go/cmd/dolt/commands/log.go @@ -72,7 +72,11 @@ The command takes options to control what is shown and how. {{.EmphasisLeft}}dolt log ..{{.EmphasisRight}} {{.EmphasisLeft}}dolt log --not {{.EmphasisRight}} {{.EmphasisLeft}}dolt log ^ {{.EmphasisRight}} - Different ways to list two dot logs. These will list commit logs for revisionA, while excluding commits from revisionB. The table option is not supported for two dot log.`, + Different ways to list two dot logs. These will list commit logs for revisionA, while excluding commits from revisionB. The table option is not supported for two dot log. + +{{.EmphasisLeft}}dolt log ...{{.EmphasisRight}} +{{.EmphasisLeft}}dolt log --not $(dolt merge-base ){{.EmphasisRight}} + Different ways to list three dot logs. These will list commit logs reachable by revisionA OR revisionB, while excluding commits reachable by BOTH revisionA AND revisionB.`, Synopsis: []string{ `[-n {{.LessThan}}num_commits{{.GreaterThan}}] [{{.LessThan}}revision-range{{.GreaterThan}}] [[--] {{.LessThan}}table{{.GreaterThan}}]`, }, @@ -180,14 +184,37 @@ func (opts *logOpts) parseRefsAndTable(ctx context.Context, apr *argparser.ArgPa return nil } - // `dolt log ..` if strings.Contains(apr.Arg(0), "..") { - if strings.Contains(apr.Arg(0), "...") { - return fmt.Errorf("three dot log not yet supported") - } if apr.NArg() > 1 { - return fmt.Errorf("Cannot use two dot when 2 or more arguments provided") + return fmt.Errorf("Cannot use two or three dot syntax when 2 or more arguments provided") } + + // `dolt log ...` + if strings.Contains(apr.Arg(0), "...") { + refs := strings.Split(apr.Arg(0), "...") + + for _, ref := range refs { + cs, err := getCommitSpec(ref) + if err != nil { + return err + } + opts.commitSpecs = append(opts.commitSpecs, cs) + } + + mergeBase, verr := getMergeBaseFromStrings(ctx, dEnv, refs[0], refs[1]) + if verr != nil { + return verr + } + notCs, err := getCommitSpec(mergeBase) + if err != nil { + return err + } + opts.excludingCommitSpecs = append(opts.excludingCommitSpecs, notCs) + + return nil + } + + // `dolt log ..` refs := strings.Split(apr.Arg(0), "..") notCs, err := getCommitSpec(refs[0]) if err != nil { diff --git a/integration-tests/bats/log.bats b/integration-tests/bats/log.bats index 5afdff0548..c0c441c149 100755 --- a/integration-tests/bats/log.bats +++ b/integration-tests/bats/log.bats @@ -40,7 +40,7 @@ teardown() { [[ ! "$output" =~ "BRANCH1" ]] || false } -@test "log: two dot log" { +@test "log: two and three dot log" { dolt sql -q "create table testtable (pk int PRIMARY KEY)" dolt add . dolt commit -m "commit 1 MAIN" @@ -73,6 +73,9 @@ teardown() { # (init) - 1M - 2M - 3M (main) # # # # # # # # # # # # # # # # # # # # # # # # # # + + + # Valid two dot run dolt log main..branchA @@ -126,6 +129,29 @@ teardown() { [[ "$output" =~ "AFTER" ]] || false [[ "$output" =~ "BRANCHA" ]] || false + # Valid three dot + run dolt log main...branchA + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log branchA...main + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log main branchA --not $(dolt merge-base main branchA) + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false + run dolt log branchB...branchA + [ $status -eq 0 ] + [[ ! "$output" =~ "MAIN" ]] || false + [[ ! "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false + # Multiple refs run dolt log branchB branchA [ $status -eq 0 ] @@ -139,6 +165,12 @@ teardown() { [[ "$output" =~ "AFTER" ]] || false [[ "$output" =~ "BRANCHA" ]] || false [[ ! "$output" =~ "BRANCHB" ]] || false + run dolt log main branchB branchA + [ $status -eq 0 ] + [[ "$output" =~ "MAIN" ]] || false + [[ "$output" =~ "AFTER" ]] || false + [[ "$output" =~ "BRANCHA" ]] || false + [[ "$output" =~ "BRANCHB" ]] || false run dolt log branchB main ^branchA [ $status -eq 0 ] [[ ! "$output" =~ "MAIN" ]] || false @@ -181,6 +213,10 @@ teardown() { [ $status -eq 1 ] run dolt log testtable main..branchA [ $status -eq 1 ] + run dolt log main...branchA testtable + [ $status -eq 1 ] + run dolt log testtable main...branchA + [ $status -eq 1 ] run dolt log ^main branchA testtable [ $status -eq 1 ] run dolt log branchA testtable --not main @@ -189,6 +225,10 @@ teardown() { [ $status -eq 1 ] run dolt log main main..branchA [ $status -eq 1 ] + run dolt log main...branchA main + [ $status -eq 1 ] + run dolt log main main...branchA + [ $status -eq 1 ] run dolt log ^main testtable [ $status -eq 1 ] run dolt log testtable ^main @@ -199,8 +239,9 @@ teardown() { [ $status -eq 1 ] run dolt log main..branchA --not ^main [ $status -eq 1 ] - - run dolt log main...branchA + run dolt log main...branchA --not main + [ $status -eq 1 ] + run dolt log main...branchA --not ^main [ $status -eq 1 ] } From 9d0b7884f366eee89696520dea0ea40652e429f2 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Wed, 2 Nov 2022 14:45:39 -0700 Subject: [PATCH 5/7] dolt_log table function supports three dot --- .../doltcore/sqle/dolt_log_table_function.go | 105 ++++++++++++------ .../doltcore/sqle/enginetest/dolt_queries.go | 58 ++++++++-- integration-tests/bats/log.bats | 3 - 3 files changed, 120 insertions(+), 46 deletions(-) diff --git a/go/libraries/doltcore/sqle/dolt_log_table_function.go b/go/libraries/doltcore/sqle/dolt_log_table_function.go index 7e6ed6bd0f..16566b1a8e 100644 --- a/go/libraries/doltcore/sqle/dolt_log_table_function.go +++ b/go/libraries/doltcore/sqle/dolt_log_table_function.go @@ -24,6 +24,7 @@ import ( "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env/actions/commitwalk" + "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" "github.com/dolthub/dolt/go/store/hash" ) @@ -303,7 +304,7 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error { return ltf.invalidArgDetailsErr(ltf.revisionExpr, "second revision must exist if first revision contains '^'") } if strings.Contains(revisionStr, "..") && strings.HasPrefix(revisionStr, "^") { - return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain both '..' and '^'") + return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain both '..' or '...' and '^'") } } @@ -313,10 +314,10 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error { return sql.ErrInvalidArgumentDetails.New(ltf.FunctionName(), ltf.secondRevisionExpr.String()) } if strings.Contains(secondRevisionStr, "..") { - return ltf.invalidArgDetailsErr(ltf.secondRevisionExpr, "second revision cannot contain '..'") + return ltf.invalidArgDetailsErr(ltf.secondRevisionExpr, "second revision cannot contain '..' or '...'") } if strings.Contains(revisionStr, "..") { - return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain '..' if second revision exists") + return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain '..' or '...' if second revision exists") } } @@ -334,7 +335,7 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error { return ltf.invalidArgDetailsErr(ltf.revisionExpr, "must have revision in order to use --not") } if ltf.revisionExpr != nil && (strings.Contains(revisionStr, "..") || strings.HasPrefix(revisionStr, "^")) { - return ltf.invalidArgDetailsErr(ltf.revisionExpr, "cannot use --not if '..' or '^' present in revision") + return ltf.invalidArgDetailsErr(ltf.revisionExpr, "cannot use --not if dots or '^' present in revision") } if ltf.secondRevisionExpr != nil && strings.HasPrefix(secondRevisionStr, "^") { return ltf.invalidArgDetailsErr(ltf.secondRevisionExpr, "cannot use --not if '^' present in second revision") @@ -352,7 +353,7 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error { // RowIter implements the sql.Node interface func (ltf *LogTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error) { - revisionVal, excludingRevisionVal, err := ltf.evaluateArguments() + revisionVal, secondRevisionVal, threeDot, err := ltf.evaluateArguments() if err != nil { return nil, err } @@ -392,18 +393,40 @@ func (ltf *LogTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter return nil, err } - // Two dot log - if len(excludingRevisionVal) > 0 { - exCs, err := doltdb.NewCommitSpec(excludingRevisionVal) + // Two and three dot log + if len(secondRevisionVal) > 0 { + secondCs, err := doltdb.NewCommitSpec(secondRevisionVal) if err != nil { return nil, err } - excludingCommit, err := sqledb.ddb.Resolve(ctx, exCs, nil) + secondCommit, err := sqledb.ddb.Resolve(ctx, secondCs, nil) if err != nil { return nil, err } - return ltf.NewDotDotLogTableFunctionRowIter(ctx, sqledb.ddb, commit, excludingCommit, matchFunc, cHashToRefs) + + if threeDot { + mergeBase, err := merge.MergeBase(ctx, commit, secondCommit) + if err != nil { + return nil, err + } + + mergeCs, err := doltdb.NewCommitSpec(mergeBase.String()) + if err != nil { + return nil, err + } + + // Use merge base as excluding commit + mergeCommit, err := sqledb.ddb.Resolve(ctx, mergeCs, nil) + if err != nil { + return nil, err + } + + return ltf.NewDotDotLogTableFunctionRowIter(ctx, sqledb.ddb, []*doltdb.Commit{commit, secondCommit}, mergeCommit, matchFunc, cHashToRefs) + } + + return ltf.NewDotDotLogTableFunctionRowIter(ctx, sqledb.ddb, []*doltdb.Commit{commit}, secondCommit, matchFunc, cHashToRefs) + } return ltf.NewLogTableFunctionRowIter(ctx, sqledb.ddb, commit, matchFunc, cHashToRefs) @@ -455,39 +478,40 @@ func getCommitHashToRefs(ctx *sql.Context, ddb *doltdb.DoltDB, decoration string return cHashToRefs, nil } -// evaluateArguments returns revisionValStr and excludingRevisionValStr. +// evaluateArguments returns revisionValStr, secondRevisionValStr, and three dot boolean. // It evaluates the argument expressions to turn them into values this LogTableFunction // can use. Note that this method only evals the expressions, and doesn't validate the values. -func (ltf *LogTableFunction) evaluateArguments() (string, string, error) { +func (ltf *LogTableFunction) evaluateArguments() (string, string, bool, error) { var revisionValStr string - var excludingRevisionValStr string + var secondRevisionValStr string var err error + threeDot := false if ltf.revisionExpr != nil { - revisionValStr, excludingRevisionValStr, err = getRevisionsFromExpr(ltf.ctx, ltf.revisionExpr, true) + revisionValStr, secondRevisionValStr, threeDot, err = getRevisionsFromExpr(ltf.ctx, ltf.revisionExpr, true) if err != nil { - return "", "", err + return "", "", false, err } } if ltf.secondRevisionExpr != nil { - rvs, ervs, err := getRevisionsFromExpr(ltf.ctx, ltf.secondRevisionExpr, false) + rvs, srvs, _, err := getRevisionsFromExpr(ltf.ctx, ltf.secondRevisionExpr, false) if err != nil { - return "", "", err + return "", "", false, err } if len(rvs) > 0 { revisionValStr = rvs } - if len(ervs) > 0 { - excludingRevisionValStr = ervs + if len(srvs) > 0 { + secondRevisionValStr = srvs } } if len(ltf.notRevision) > 0 { - excludingRevisionValStr = ltf.notRevision + secondRevisionValStr = ltf.notRevision } - return revisionValStr, excludingRevisionValStr, nil + return revisionValStr, secondRevisionValStr, threeDot, nil } func mustExpressionToString(ctx *sql.Context, expr sql.Expression) string { @@ -512,23 +536,27 @@ func expressionToString(ctx *sql.Context, expr sql.Expression) (string, error) { return valStr, nil } -// Gets revisionName and/or excludingRevisionName from sql expression -func getRevisionsFromExpr(ctx *sql.Context, expr sql.Expression, canDot bool) (string, string, error) { +// Gets revisionName and/or secondRevisionName from sql expression +func getRevisionsFromExpr(ctx *sql.Context, expr sql.Expression, canDot bool) (string, string, bool, error) { revisionValStr, err := expressionToString(ctx, expr) if err != nil { - return "", "", err + return "", "", false, err } if canDot && strings.Contains(revisionValStr, "..") { + if strings.Contains(revisionValStr, "...") { + refs := strings.Split(revisionValStr, "...") + return refs[0], refs[1], true, nil + } refs := strings.Split(revisionValStr, "..") - return refs[1], refs[0], nil + return refs[1], refs[0], false, nil } if strings.HasPrefix(revisionValStr, "^") { - return "", strings.TrimPrefix(revisionValStr, "^"), nil + return "", strings.TrimPrefix(revisionValStr, "^"), false, nil } - return revisionValStr, "", nil + return revisionValStr, "", false, nil } //------------------------------------ @@ -566,10 +594,15 @@ func (ltf *LogTableFunction) NewLogTableFunctionRowIter(ctx *sql.Context, ddb *d }, nil } -func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, ddb *doltdb.DoltDB, commit, excludingCommit *doltdb.Commit, matchFn func(*doltdb.Commit) (bool, error), cHashToRefs map[hash.Hash][]string) (*logTableFunctionRowIter, error) { - h, err := commit.HashOf() - if err != nil { - return nil, err +func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, ddb *doltdb.DoltDB, commits []*doltdb.Commit, excludingCommit *doltdb.Commit, matchFn func(*doltdb.Commit) (bool, error), cHashToRefs map[hash.Hash][]string) (*logTableFunctionRowIter, error) { + hashes := make([]hash.Hash, len(commits)) + + for i, commit := range commits { + h, err := commit.HashOf() + if err != nil { + return nil, err + } + hashes[i] = h } exHash, err := excludingCommit.HashOf() @@ -577,17 +610,23 @@ func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, return nil, err } - child, err := commitwalk.GetDotDotRevisionsIterator(ctx, ddb, []hash.Hash{h}, ddb, []hash.Hash{exHash}, matchFn) + child, err := commitwalk.GetDotDotRevisionsIterator(ctx, ddb, hashes, ddb, []hash.Hash{exHash}, matchFn) if err != nil { return nil, err } + var headHash hash.Hash + + if len(hashes) == 1 { + headHash = hashes[0] + } + return &logTableFunctionRowIter{ child: child, showParents: ltf.showParents, decoration: ltf.decoration, cHashToRefs: cHashToRefs, - headHash: h, + headHash: headHash, }, nil } diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go index 50532439da..5cd35cf296 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go @@ -5198,14 +5198,26 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{ Query: "SELECT * from dolt_log('^main..branch1');", ExpectedErr: sql.ErrInvalidArgumentDetails, }, + { + Query: "SELECT * from dolt_log('^main...branch1');", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, { Query: "SELECT * from dolt_log(@Commit1, 'main..branch1');", ExpectedErr: sql.ErrInvalidArgumentDetails, }, + { + Query: "SELECT * from dolt_log(@Commit1, 'main...branch1');", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, { Query: "SELECT * from dolt_log('main..branch1', '--not', @Commit1);", ExpectedErr: sql.ErrInvalidArgumentDetails, }, + { + Query: "SELECT * from dolt_log('main...branch1', '--not', @Commit1);", + ExpectedErr: sql.ErrInvalidArgumentDetails, + }, { Query: "SELECT * from dolt_log('^main', '--not', @Commit1);", ExpectedErr: sql.ErrInvalidArgumentDetails, @@ -5366,30 +5378,39 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{ "insert into t values (5, 'five', 'six');", "set @Commit5 = dolt_commit('-am', 'inserting into t 5');", }, + /* Commit graph: + 3 - 4 (new-branch) + / + 0 - 1 - 2 - 5 (main) + */ Assertions: []queries.ScriptTestAssertion{ { Query: "SELECT count(*) from dolt_log('^main', 'new-branch');", - Expected: []sql.Row{{2}}, + Expected: []sql.Row{{2}}, // 4, 3 }, { Query: "SELECT count(*) from dolt_log('main..new-branch');", - Expected: []sql.Row{{2}}, + Expected: []sql.Row{{2}}, // 4, 3 + }, + { + Query: "SELECT count(*) from dolt_log('main...new-branch');", + Expected: []sql.Row{{3}}, // 5, 4, 3 }, { Query: "SELECT count(*) from dolt_log('new-branch', '--not', 'main');", - Expected: []sql.Row{{2}}, + Expected: []sql.Row{{2}}, // 4, 3 }, { Query: "SELECT count(*) from dolt_log('new-branch', '^main');", - Expected: []sql.Row{{2}}, + Expected: []sql.Row{{2}}, // 4, 3 }, { Query: "SELECT count(*) from dolt_log('^new-branch', 'main');", - Expected: []sql.Row{{1}}, + Expected: []sql.Row{{1}}, // 5 }, { Query: "SELECT count(*) from dolt_log('main', '--not', 'new-branch');", - Expected: []sql.Row{{1}}, + Expected: []sql.Row{{1}}, // 5 }, { Query: "SELECT count(*) from dolt_log('^main', 'main');", @@ -5399,6 +5420,10 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{ Query: "SELECT count(*) from dolt_log('main..main');", Expected: []sql.Row{{0}}, }, + { + Query: "SELECT count(*) from dolt_log('main...main');", + Expected: []sql.Row{{0}}, + }, { Query: "SELECT count(*) from dolt_log('main', '--not', 'main');", Expected: []sql.Row{{0}}, @@ -5409,7 +5434,7 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{ }, { Query: "SELECT count(*) from dolt_log('^main^', 'main');", - Expected: []sql.Row{{1}}, + Expected: []sql.Row{{1}}, // 5 }, { Query: "SELECT count(*) from dolt_log('^main', 'main^');", @@ -5421,15 +5446,15 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{ }, { Query: "SELECT count(*) from dolt_log('^new-branch', @Commit5);", - Expected: []sql.Row{{1}}, + Expected: []sql.Row{{1}}, // 5 }, { Query: "SELECT count(*) from dolt_log(@Commit3, '--not', @Commit2);", - Expected: []sql.Row{{1}}, + Expected: []sql.Row{{1}}, // 3 }, { Query: "SELECT count(*) from dolt_log(@Commit4, '--not', @Commit2);", - Expected: []sql.Row{{2}}, + Expected: []sql.Row{{2}}, // 4, 3 }, }, }, @@ -5492,6 +5517,11 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{ "insert into t values (5, 'five', 'six');", "set @Commit5 = dolt_commit('-am', 'inserting into t 5');", }, + /* Commit graph: + 3 - 4 (new-branch) + / + 0 - 1 - 2 - 5 (main) + */ Assertions: []queries.ScriptTestAssertion{ { Query: "SELECT commit_hash = @Commit4, commit_hash = @Commit3, committer, email, message from dolt_log('^main', 'new-branch');", @@ -5507,6 +5537,14 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{ {false, true, "John Doe", "johndoe@example.com", "inserting into t 3"}, }, }, + { + Query: "SELECT commit_hash = @Commit5, commit_hash = @Commit4, commit_hash = @Commit3, committer, email, message from dolt_log('main...new-branch');", + Expected: []sql.Row{ + {true, false, false, "billy bob", "bigbillieb@fake.horse", "inserting into t 5"}, + {false, true, false, "John Doe", "johndoe@example.com", "inserting into t 4"}, + {false, false, true, "John Doe", "johndoe@example.com", "inserting into t 3"}, + }, + }, { Query: "SELECT commit_hash = @Commit4, commit_hash = @Commit3, committer, email, message from dolt_log('new-branch', '--not', 'main');", Expected: []sql.Row{ diff --git a/integration-tests/bats/log.bats b/integration-tests/bats/log.bats index c0c441c149..542fe1649d 100755 --- a/integration-tests/bats/log.bats +++ b/integration-tests/bats/log.bats @@ -73,9 +73,6 @@ teardown() { # (init) - 1M - 2M - 3M (main) # # # # # # # # # # # # # # # # # # # # # # # # # # - - - # Valid two dot run dolt log main..branchA From a9289c0c4ea0f673db908105861dc8d50c7bcbe6 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Thu, 3 Nov 2022 13:55:59 -0700 Subject: [PATCH 6/7] Fix getRevisionsFromExpr docs --- go/libraries/doltcore/sqle/dolt_log_table_function.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/libraries/doltcore/sqle/dolt_log_table_function.go b/go/libraries/doltcore/sqle/dolt_log_table_function.go index 16566b1a8e..8b1c5b2d95 100644 --- a/go/libraries/doltcore/sqle/dolt_log_table_function.go +++ b/go/libraries/doltcore/sqle/dolt_log_table_function.go @@ -536,7 +536,8 @@ func expressionToString(ctx *sql.Context, expr sql.Expression) (string, error) { return valStr, nil } -// Gets revisionName and/or secondRevisionName from sql expression +// getRevisionsFromExpr returns the revisionName and/or secondRevisionName, as +// well as a threeDot boolean from sql expression func getRevisionsFromExpr(ctx *sql.Context, expr sql.Expression, canDot bool) (string, string, bool, error) { revisionValStr, err := expressionToString(ctx, expr) if err != nil { From 20c0ae3010d68d82adb6196f1ff0caa9c2917772 Mon Sep 17 00:00:00 2001 From: Taylor Bantle Date: Mon, 7 Nov 2022 10:27:37 -0800 Subject: [PATCH 7/7] Feedback --- go/cmd/dolt/commands/log.go | 2 +- .../env/actions/commitwalk/commitwalk.go | 3 +- .../env/actions/commitwalk/commitwalk_test.go | 33 +++++++++++++++++-- integration-tests/bats/log.bats | 5 --- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/go/cmd/dolt/commands/log.go b/go/cmd/dolt/commands/log.go index b65d1a82e0..ed4e2e69a3 100644 --- a/go/cmd/dolt/commands/log.go +++ b/go/cmd/dolt/commands/log.go @@ -66,7 +66,7 @@ The command takes options to control what is shown and how. {{.EmphasisLeft}}dolt log [...]{{.EmphasisRight}} Lists commit logs starting from revision. If multiple revisions provided, lists logs reachable by all revisions. -{{.EmphasisLeft}}dolt log [
{{.EmphasisRight}} +{{.EmphasisLeft}}dolt log [...]
{{.EmphasisRight}} Lists commit logs starting from revisions, only including commits with changes to table. {{.EmphasisLeft}}dolt log ..{{.EmphasisRight}} diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go index dd16802ce7..c954746a95 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk.go @@ -157,7 +157,8 @@ func newQueue() *q { // concurrent commits --- higher commits appear first. Remaining // ties are broken by timestamp; newer commits appear first. // -// Roughly mimics `git log main..feature`. +// Roughly mimics `git log main..feature` or `git log main...feature` (if +// more than one `includedHead` is provided). func GetDotDotRevisions(ctx context.Context, includedDB *doltdb.DoltDB, includedHeads []hash.Hash, excludedDB *doltdb.DoltDB, excludedHeads []hash.Hash, num int) ([]*doltdb.Commit, error) { itr, err := GetDotDotRevisionsIterator(ctx, includedDB, includedHeads, excludedDB, excludedHeads, nil) if err != nil { diff --git a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go index 1539edf9ef..94850500a9 100644 --- a/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go +++ b/go/libraries/doltcore/env/actions/commitwalk/commitwalk_test.go @@ -24,6 +24,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" + "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/ref" "github.com/dolthub/dolt/go/libraries/utils/filesys" "github.com/dolthub/dolt/go/store/datas" @@ -113,9 +114,9 @@ func TestGetDotDotRevisions(t *testing.T) { // Branches look like this: // - // feature: *--*--*--*--*--*--* - // / / - // main: --*--*--*--*--*--------*--*--*--* + // feature: F1--F2--F3--F4--F5--F6--F7 + // / / + // main: M0--M1--M2--M3--M4--M5/F0------------M6--M7--M8--M9 featureHash := mustGetHash(t, featureCommits[7]) mainHash := mustGetHash(t, mainCommits[6]) @@ -158,6 +159,32 @@ func TestGetDotDotRevisions(t *testing.T) { assertEqualHashes(t, featureCommits[2], res[1]) assertEqualHashes(t, featureCommits[1], res[2]) + // Three dot + mergeBaseHash, err := merge.MergeBase(context.Background(), mainCommits[6], featureCommits[7]) + require.NoError(t, err) + + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featureHash, mainHash}, dEnv.DoltDB, []hash.Hash{mergeBaseHash}, -1) + require.NoError(t, err) + assert.Len(t, res, 7) + assertEqualHashes(t, featureCommits[7], res[0]) + assertEqualHashes(t, featureCommits[6], res[1]) + assertEqualHashes(t, featureCommits[5], res[2]) + assertEqualHashes(t, featureCommits[4], res[3]) + assertEqualHashes(t, featureCommits[3], res[4]) + assertEqualHashes(t, featureCommits[2], res[5]) + assertEqualHashes(t, featureCommits[1], res[6]) + + mergeBaseHash, err = merge.MergeBase(context.Background(), mainCommits[6], featureCommits[3]) + require.NoError(t, err) + + res, err = GetDotDotRevisions(context.Background(), dEnv.DoltDB, []hash.Hash{featurePreMergeHash, mainHash}, dEnv.DoltDB, []hash.Hash{mergeBaseHash}, -1) + require.NoError(t, err) + assert.Len(t, res, 4) + assertEqualHashes(t, featureCommits[3], res[0]) + assertEqualHashes(t, featureCommits[2], res[1]) + assertEqualHashes(t, mainCommits[6], res[2]) + assertEqualHashes(t, featureCommits[1], res[3]) + // Create a similar branch to "feature" on a forked repository and GetDotDotRevisions using that as well. forkEnv := mustForkDB(t, dEnv.DoltDB, "feature", featureCommits[4]) diff --git a/integration-tests/bats/log.bats b/integration-tests/bats/log.bats index 5afdff0548..a0b04339c4 100755 --- a/integration-tests/bats/log.bats +++ b/integration-tests/bats/log.bats @@ -120,11 +120,6 @@ teardown() { [[ "$output" =~ "BRANCHA" ]] || false run dolt log ^main ^branchA [ $status -eq 0 ] - run dolt log main branchA - [ $status -eq 0 ] - [[ "$output" =~ "MAIN" ]] || false - [[ "$output" =~ "AFTER" ]] || false - [[ "$output" =~ "BRANCHA" ]] || false # Multiple refs run dolt log branchB branchA