Remove OID (#3275)

Fixes #3121

go/constants/version.go

@@ -11,7 +11,7 @@ import (
 )
 
 // TODO: generate this from some central thing with go generate.
-const NomsVersion = "7.2"
+const NomsVersion = "7.3"
 const NOMS_VERSION_NEXT_ENV_NAME = "NOMS_VERSION_NEXT"
 const NOMS_VERSION_NEXT_ENV_VALUE = "1"
 

go/types/encoding_test.go

@@ -500,7 +500,7 @@ func TestWriteListOfUnion(t *testing.T) {
 	assertEncoding(t,
 		// Note that the order of members in a union is determined based on a hash computation; the particular ordering of Number, Bool, String was determined empirically. This must not change unless deliberately and explicitly revving the persistent format.
 		[]interface{}{
-			uint8(ListKind), uint8(UnionKind), uint32(3) /* len */, uint8(NumberKind), uint8(BoolKind), uint8(StringKind), false,
+			uint8(ListKind), uint8(UnionKind), uint32(3) /* len */, uint8(BoolKind), uint8(NumberKind), uint8(StringKind), false,
 			uint32(4) /* len */, uint8(StringKind), "0", uint8(NumberKind), Number(1), uint8(StringKind), "2", uint8(BoolKind), true,
 		},
 		NewList(

go/types/type.go

@@ -22,7 +22,6 @@ import (
 type Type struct {
 	Desc          TypeDesc
 	h             *hash.Hash
-	oid           *hash.Hash
 	id            uint32
 	serialization []byte
 }
@@ -30,7 +29,7 @@ type Type struct {
 const initialTypeBufferSize = 128
 
 func newType(desc TypeDesc, id uint32) *Type {
-	return &Type{desc, &hash.Hash{}, nil, id, nil}
+	return &Type{desc, &hash.Hash{}, id, nil}
 }
 
 func ensureTypeSerialization(t *Type) {

go/types/type_cache.go

@@ -236,9 +236,6 @@ func checkStructType(t *Type, checkKind checkKindType) {
 		return
 	}
 
-	walkType(t, nil, generateOIDs)
-	walkType(t, nil, checkForUnrolledCycles)
-
 	switch checkKind {
 	case checkKindNormalize:
 		walkType(t, nil, sortUnions)
@@ -249,20 +246,6 @@ func checkStructType(t *Type, checkKind checkKindType) {
 	}
 }
 
-func generateOIDs(t *Type, _ []*Type) {
-	generateOID(t, false)
-}
-
-func checkForUnrolledCycles(t *Type, parentStructTypes []*Type) {
-	if t.Kind() == StructKind {
-		for _, ttt := range parentStructTypes {
-			if *t.oid == *ttt.oid {
-				panic("unrolled cycle types are not supported; ahl owes you a beer")
-			}
-		}
-	}
-}
-
 func sortUnions(t *Type, _ []*Type) {
 	if t.Kind() == UnionKind {
 		sort.Sort(t.Desc.(CompoundDesc).ElemTypes)
@@ -277,7 +260,7 @@ func validateTypes(t *Type, _ []*Type) {
 			panic("Invalid union type")
 		}
 		for i := 1; i < len(elemTypes); i++ {
-			if !elemTypes[i-1].oid.Less(*elemTypes[i].oid) {
+			if !unionLess(elemTypes[i-1], elemTypes[i]) {
 				panic("Invalid union order")
 			}
 		}
@@ -309,95 +292,6 @@ func walkType(t *Type, parentStructTypes []*Type, cb func(*Type, []*Type)) {
 	}
 }
 
-func generateOID(t *Type, allowUnresolvedCycles bool) {
-	if t.oid == nil {
-		buf := newBinaryNomsWriter()
-		encodeForOID(t, buf, allowUnresolvedCycles, t, nil)
-		oid := hash.Of(buf.data())
-		t.oid = &oid
-	}
-}
-
-func encodeForOID(t *Type, buf nomsWriter, allowUnresolvedCycles bool, root *Type, parentStructTypes []*Type) {
-	// Most types are encoded in a straightforward fashion
-	switch desc := t.Desc.(type) {
-	case CycleDesc:
-		if allowUnresolvedCycles {
-			buf.writeUint8(uint8(desc.Kind()))
-			buf.writeUint32(uint32(desc))
-		} else {
-			panic("found an unexpected unresolved cycle")
-		}
-	case PrimitiveDesc:
-		buf.writeUint8(uint8(desc.Kind()))
-	case CompoundDesc:
-		switch k := desc.Kind(); k {
-		case ListKind, MapKind, RefKind, SetKind:
-			buf.writeUint8(uint8(k))
-			buf.writeUint32(uint32(len(desc.ElemTypes)))
-			for _, tt := range desc.ElemTypes {
-				encodeForOID(tt, buf, allowUnresolvedCycles, root, parentStructTypes)
-			}
-		case UnionKind:
-			buf.writeUint8(uint8(k))
-			if t == root {
-				// If this is where we started we don't need to keep going
-				return
-			}
-
-			buf.writeUint32(uint32(len(desc.ElemTypes)))
-
-			// This is the only subtle case: encode each subordinate type, generate the hash, remove duplicates, and xor the results together to form an order independent encoding.
-			mbuf := newBinaryNomsWriter()
-			oids := make(map[hash.Hash]struct{})
-			for _, elemType := range desc.ElemTypes {
-				h := elemType.oid
-				if h == nil {
-
-					mbuf.reset()
-					encodeForOID(elemType, mbuf, allowUnresolvedCycles, root, parentStructTypes)
-					h2 := hash.Of(mbuf.data())
-					if _, found := indexOfType(elemType, parentStructTypes); !found {
-						elemType.oid = &h2
-					}
-					oids[h2] = struct{}{}
-				} else {
-					oids[*h] = struct{}{}
-					if !allowUnresolvedCycles {
-						checkForUnresolvedCycles(elemType, root, parentStructTypes)
-					}
-				}
-			}
-
-			data := make([]byte, hash.ByteLen)
-			for o := range oids {
-				for i := 0; i < len(data); i++ {
-					data[i] ^= o[i]
-				}
-			}
-			buf.writeBytes(data)
-		default:
-			panic("unknown compound type")
-		}
-	case StructDesc:
-		idx, found := indexOfType(t, parentStructTypes)
-		if found {
-			buf.writeUint8(uint8(CycleKind))
-			buf.writeUint32(uint32(len(parentStructTypes)) - 1 - idx)
-			return
-		}
-
-		buf.writeUint8(uint8(StructKind))
-		buf.writeString(desc.Name)
-
-		parentStructTypes = append(parentStructTypes, t)
-		for _, field := range desc.fields {
-			buf.writeString(field.name)
-			encodeForOID(field.t, buf, allowUnresolvedCycles, root, parentStructTypes)
-		}
-	}
-}
-
 func checkForUnresolvedCycles(t, root *Type, parentStructTypes []*Type) {
 	desc := t.Desc
 
@@ -433,9 +327,6 @@ func (tc *TypeCache) makeUnionType(elemTypes ...*Type) *Type {
 	if len(ts) == 1 {
 		return ts[0]
 	}
-	for _, tt := range ts {
-		generateOID(tt, true)
-	}
 	// We sort the contituent types to dedup equivalent types in memory; we may need to sort again after cycles are resolved for final encoding.
 	sort.Sort(ts)
 	return tc.getCompoundType(UnionKind, ts...)

go/types/type_cache_test.go

@@ -197,9 +197,9 @@ func TestTypeCacheCyclicUnions(t *testing.T) {
 		[]*Type{ut},
 	)
 
-	assert.True(ut.Desc.(CompoundDesc).ElemTypes[0].Kind() == CycleKind)
-	// That the Struct / Cycle landed in index 1 was found empirically.
-	assert.True(st == st.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[1])
+	assert.True(ut.Desc.(CompoundDesc).ElemTypes[6].Kind() == CycleKind)
+	// That the Struct / Cycle landed in index 5 was found empirically.
+	assert.Equal(st, st.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[5])
 	// ut contains an explicit Cycle type; noms must not surrepticiously change existing types so we can be sure that the Union within st is different in that the cycle has been resolved.
 	assert.False(ut == st.Desc.(StructDesc).fields[0].t)
 
@@ -209,22 +209,22 @@ func TestTypeCacheCyclicUnions(t *testing.T) {
 		[]string{"foo"},
 		[]*Type{ut2},
 	)
-	assert.True(ut2.Desc.(CompoundDesc).ElemTypes[0].Kind() == CycleKind)
-	assert.True(st2 == st2.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[1])
+	assert.True(ut2.Desc.(CompoundDesc).ElemTypes[6].Kind() == CycleKind)
+	assert.True(st2 == st2.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[5])
 	assert.False(ut2 == st2.Desc.(StructDesc).fields[0].t)
 
 	assert.True(ut == ut2)
 	assert.True(st == st2)
 }
 
-func TestInvalidCyclesAndUnions(t *testing.T) {
+func TestNonNormalizedCycles(t *testing.T) {
 	assert := assert.New(t)
 
-	assert.Panics(func() {
-		MakeStructType("A",
-			[]string{"a"},
-			[]*Type{MakeStructType("A", []string{"a"}, []*Type{MakeCycleType(1)})})
-	})
+	t1 := MakeStructType("A",
+		[]string{"a"},
+		[]*Type{MakeStructType("A", []string{"a"}, []*Type{MakeCycleType(1)})})
+	t2 := t1.Desc.(StructDesc).fields[0].t
+	assert.True(t1.Equals(t2))
 }
 
 func TestMakeStructTypeFromFields(t *testing.T) {

go/types/type_desc.go

@@ -137,6 +137,27 @@ func (c CycleDesc) HasUnresolvedCycle(visited []*Type) bool {
 
 type typeSlice []*Type
 
-func (ts typeSlice) Len() int           { return len(ts) }
-func (ts typeSlice) Less(i, j int) bool { return ts[i].oid.Less(*ts[j].oid) }
-func (ts typeSlice) Swap(i, j int)      { ts[i], ts[j] = ts[j], ts[i] }
+func (ts typeSlice) Len() int { return len(ts) }
+
+func (ts typeSlice) Less(i, j int) bool {
+	return unionLess(ts[i], ts[j])
+}
+
+func (ts typeSlice) Swap(i, j int) { ts[i], ts[j] = ts[j], ts[i] }
+
+func unionLess(ti, tj *Type) bool {
+	if ti == tj {
+		return false
+	}
+
+	ki, kj := ti.Kind(), tj.Kind()
+	if ki == kj {
+		if ki == StructKind {
+			// Due to type simplification, the only thing that matters is the name of the struct.
+			return ti.Desc.(StructDesc).Name < tj.Desc.(StructDesc).Name
+		}
+
+		return false
+	}
+	return ki < kj
+}

go/types/validating_batching_sink_test.go

@@ -71,7 +71,7 @@ func assertPanicsOnInvalidChunk(t *testing.T, data []interface{}) {
 func TestValidatingBatchingSinkDecodeInvalidUnion(t *testing.T) {
 	data := []interface{}{
 		uint8(TypeKind),
-		uint8(UnionKind), uint32(2) /* len */, uint8(BoolKind), uint8(NumberKind),
+		uint8(UnionKind), uint32(2) /* len */, uint8(NumberKind), uint8(BoolKind),
 	}
 	assertPanicsOnInvalidChunk(t, data)
 }

samples/go/hr/test-data/manifest

@@ -1 +1 @@
-2:7.2:jbhniceagao7dapu7q0ftp2mfarcm3kt:bu62ea9gugj7k7jrqc5bjg39r2norrod:2:e7opaki900h5fic5mtdtlmfed3m2jjdg:2
+2:7.3:rop7vcma6nt5sg0m6uk81kdsaiqmuinl:pig8gm2iiv8aefjko2a193hnd42attpr:2:eolvobg44ncok89v1c02vg5cm23t5709:2