From 33e138795260df02f26c4e0d05da49946c7dbf7f Mon Sep 17 00:00:00 2001 From: Neil Macneale IV Date: Fri, 4 Apr 2025 14:30:48 -0700 Subject: [PATCH] Push the amend flag into the transactional updates --- go/libraries/doltcore/doltdb/commit.go | 4 +- go/libraries/doltcore/env/actions/commit.go | 2 +- go/libraries/doltcore/migrate/progress.go | 2 +- go/libraries/doltcore/sqle/dsess/session.go | 41 ++------------------- go/store/datas/commit_options.go | 5 +++ go/store/datas/database_common.go | 4 +- 6 files changed, 15 insertions(+), 43 deletions(-) diff --git a/go/libraries/doltcore/doltdb/commit.go b/go/libraries/doltcore/doltdb/commit.go index 6ff527070e..83829d6918 100644 --- a/go/libraries/doltcore/doltdb/commit.go +++ b/go/libraries/doltcore/doltdb/commit.go @@ -303,12 +303,14 @@ type PendingCommit struct { // commit, once written. // |headRef| is the ref of the HEAD the commit will update // |mergeParentCommits| are any merge parents for this commit +// |amend| is a flag which indicates that additional parents should not be added to the provided |mergeParentCommits|. // |cm| is the metadata for the commit // The current branch head will be automatically filled in as the first parent at commit time. func (ddb *DoltDB) NewPendingCommit( ctx context.Context, roots Roots, mergeParentCommits []*Commit, + amend bool, cm *datas.CommitMeta, ) (*PendingCommit, error) { newstaged, val, err := ddb.writeRootValue(ctx, roots.Staged) @@ -322,7 +324,7 @@ func (ddb *DoltDB) NewPendingCommit( parents = append(parents, pc.dCommit.Addr()) } - commitOpts := datas.CommitOptions{Parents: parents, Meta: cm} + commitOpts := datas.CommitOptions{Parents: parents, Meta: cm, Amend: amend} return &PendingCommit{ Roots: roots, Val: val, diff --git a/go/libraries/doltcore/env/actions/commit.go b/go/libraries/doltcore/env/actions/commit.go index 00a19ac5a7..95ea8fbdee 100644 --- a/go/libraries/doltcore/env/actions/commit.go +++ b/go/libraries/doltcore/env/actions/commit.go @@ -110,5 +110,5 @@ func GetCommitStaged( return nil, err } - return db.NewPendingCommit(ctx, roots, mergeParents, meta) + return db.NewPendingCommit(ctx, roots, mergeParents, props.Amend, meta) } diff --git a/go/libraries/doltcore/migrate/progress.go b/go/libraries/doltcore/migrate/progress.go index 5764be23a3..fa48bef774 100644 --- a/go/libraries/doltcore/migrate/progress.go +++ b/go/libraries/doltcore/migrate/progress.go @@ -273,7 +273,7 @@ func commitRoot( return err } - pcm, err := ddb.NewPendingCommit(ctx, roots, parents, meta) + pcm, err := ddb.NewPendingCommit(ctx, roots, parents, false, meta) if err != nil { return err } diff --git a/go/libraries/doltcore/sqle/dsess/session.go b/go/libraries/doltcore/sqle/dsess/session.go index cdaf169100..e8dd927a93 100644 --- a/go/libraries/doltcore/sqle/dsess/session.go +++ b/go/libraries/doltcore/sqle/dsess/session.go @@ -728,7 +728,6 @@ func (d *DoltSession) NewPendingCommit( // See NewPendingCommit func (d *DoltSession) newPendingCommit(ctx *sql.Context, branchState *branchState, roots doltdb.Roots, props actions.CommitStagedProps) (*doltdb.PendingCommit, error) { headCommit := branchState.headCommit - headHash, _ := headCommit.HashOf() if branchState.WorkingSet() == nil { return nil, doltdb.ErrOperationNotSupportedInDetachedHead @@ -755,52 +754,18 @@ func (d *DoltSession) newPendingCommit(ctx *sql.Context, branchState *branchStat // If the commit message isn't set and we're amending the previous commit, // go ahead and set the commit message from the current HEAD if props.Message == "" && props.Amend { - cs, err := doltdb.NewCommitSpec("HEAD") - if err != nil { - return nil, err - } - - headRef, err := branchState.dbData.Rsr.CWBHeadRef() - if err != nil { - return nil, err - } - optCmt, err := branchState.dbData.Ddb.Resolve(ctx, cs, headRef) - commit, ok := optCmt.ToCommit() - if !ok { - return nil, doltdb.ErrGhostCommitEncountered - } - - meta, err := commit.GetCommitMeta(ctx) + meta, err := headCommit.GetCommitMeta(ctx) if err != nil { return nil, err } props.Message = meta.Description } - - // TODO: This is not the correct way to write this commit as an amend. While this commit is running - // the branch head moves backwards and concurrency control here is not principled. - newRoots, err := actions.ResetSoftToRef(ctx, branchState.dbData, "HEAD~1") - if err != nil { - return nil, err - } - - err = d.SetWorkingSet(ctx, ctx.GetCurrentDatabase(), branchState.WorkingSet().WithStagedRoot(newRoots.Staged)) - if err != nil { - return nil, err - } - - roots.Head = newRoots.Head } pendingCommit, err := actions.GetCommitStaged(ctx, roots, branchState.WorkingSet(), mergeParentCommits, branchState.dbData.Ddb, props) if err != nil { - if props.Amend { - _, err = actions.ResetSoftToRef(ctx, branchState.dbData, headHash.String()) - if err != nil { - return nil, err - } - } - if _, ok := err.(actions.NothingStaged); err != nil && !ok { + // Special case for nothing staged, which is not an error + if _, ok := err.(actions.NothingStaged); !ok { return nil, err } } diff --git a/go/store/datas/commit_options.go b/go/store/datas/commit_options.go index 825a8f3737..ae5ba59b80 100644 --- a/go/store/datas/commit_options.go +++ b/go/store/datas/commit_options.go @@ -32,5 +32,10 @@ type CommitOptions struct { // parent. Parents []hash.Hash + // Amend flag indicates that the commit being build it to amend an existing commit. Generally we add the branch HEAD + // as a parent, in addition to the parent set provided here. When we amend, we want to strictly use the commits + // provided in |Parents|, and no others. + Amend bool + Meta *CommitMeta } diff --git a/go/store/datas/database_common.go b/go/store/datas/database_common.go index 8019ce5943..9502cfa06f 100644 --- a/go/store/datas/database_common.go +++ b/go/store/datas/database_common.go @@ -586,7 +586,7 @@ func (db *database) BuildNewCommit(ctx context.Context, ds Dataset, v types.Valu if ok { opts.Parents = []hash.Hash{headAddr} } - } else { + } else if !opts.Amend { curr, ok := ds.MaybeHeadAddr() if ok { if !hasParentHash(opts, curr) { @@ -901,7 +901,7 @@ func (db *database) CommitWithWorkingSet( // Prepend the current head hash to the list of parents if one was provided. This is only necessary if parents were // provided because we fill it in automatically in buildNewCommit otherwise. - if len(opts.Parents) > 0 { + if len(opts.Parents) > 0 && !opts.Amend { headHash, ok := commitDS.MaybeHeadAddr() if ok { if !hasParentHash(opts, headHash) {