attic-labs / noms

The versioned, forkable, syncable database
Apache License 2.0
7.45k stars 267 forks source link

Is fast-forward correct? #3852

Open aboodman opened 4 years ago

aboodman commented 4 years ago

It doesn't look like it to me.

It's only checking whether the incoming commit is taller than the existing commit. It doesn't check the ancestry at all. So committing x<-y<-z on top of a<-b would succeed. Worse, the ref height takes into account the entire chunk hierarchy including values, not just the commits. So committing something like x on top of a<-b<-c would also work if x contains a chunk hierarchy that is bigger than c's.

https://github.com/attic-labs/noms/blob/master/go/datas/database_common.go#L121

    if ok && newHeadRef.Height() <= currentHeadRef.Height() {
        return ErrMergeNeeded
    }

In order to implement fast-forward correctly, we must know all commits refs that are reachable from a given commit.