Merge pull request #2120 from arv/commit-no-meta

Commit type should always have a meta field

cmd/noms/noms_log_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/attic-labs/noms/go/spec"
 	"github.com/attic-labs/noms/go/types"
 	"github.com/attic-labs/noms/go/util/clienttest"
+	"github.com/attic-labs/noms/go/util/test"
 	"github.com/attic-labs/testify/assert"
 	"github.com/attic-labs/testify/suite"
 )
@@ -202,32 +203,6 @@ func (s *nomsLogTestSuite) TestNomsGraph2() {
 	s.Equal(diffRes2, res)
 }
 
-func (s *nomsLogTestSuite) TestNoMetaCommit() {
-	str := spec.CreateDatabaseSpecString("ldb", s.LdbDir)
-	db, err := spec.GetDatabase(str)
-	s.NoError(err)
-
-	ds := dataset.NewDataset(db, "ds1")
-
-	meta := types.NewStruct("Meta", map[string]types.Value{
-		"test1": types.String("Yoo"),
-		"test2": types.String("Hoo"),
-	})
-	ds, err = ds.Commit(types.String("1"), dataset.CommitOptions{Meta: meta})
-	s.NoError(err)
-	r1 := ds.HeadRef()
-
-	noMetaCommit := types.NewStruct("Commit", map[string]types.Value{
-		"value":   types.String("2"),
-		"parents": types.NewSet(r1),
-	})
-	ds.Database().Commit("ds1", noMetaCommit)
-	db.Close()
-
-	res, _ := s.Run(main, []string{"log", "-show-value=false", spec.CreateValueSpecString("ldb", s.LdbDir, "ds1")})
-	s.Equal(metaRes1, res)
-}
-
 func (s *nomsLogTestSuite) TestNomsGraph3() {
 	str := spec.CreateDatabaseSpecString("ldb", s.LdbDir)
 	db, err := spec.GetDatabase(str)
@@ -294,19 +269,19 @@ func (s *nomsLogTestSuite) TestTruncation() {
 
 	dsSpec := spec.CreateValueSpecString("ldb", s.LdbDir, "truncate")
 	res, _ := s.Run(main, []string{"log", "-graph", "-show-value=true", dsSpec})
-	s.Equal(truncRes1, res)
+	test.EqualsIgnoreHashes(s.T(), truncRes1, res)
 	res, _ = s.Run(main, []string{"log", "-graph", "-show-value=false", dsSpec})
-	s.Equal(diffTrunc1, res)
+	test.EqualsIgnoreHashes(s.T(), diffTrunc1, res)
 
 	res, _ = s.Run(main, []string{"log", "-graph", "-show-value=true", "-max-lines=-1", dsSpec})
-	s.Equal(truncRes2, res)
+	test.EqualsIgnoreHashes(s.T(), truncRes2, res)
 	res, _ = s.Run(main, []string{"log", "-graph", "-show-value=false", "-max-lines=-1", dsSpec})
-	s.Equal(diffTrunc2, res)
+	test.EqualsIgnoreHashes(s.T(), diffTrunc2, res)
 
 	res, _ = s.Run(main, []string{"log", "-graph", "-show-value=true", "-max-lines=0", dsSpec})
-	s.Equal(truncRes3, res)
+	test.EqualsIgnoreHashes(s.T(), truncRes3, res)
 	res, _ = s.Run(main, []string{"log", "-graph", "-show-value=false", "-max-lines=0", dsSpec})
-	s.Equal(diffTrunc3, res)
+	test.EqualsIgnoreHashes(s.T(), diffTrunc3, res)
 }
 
 func TestBranchlistSplice(t *testing.T) {

cmd/noms/noms_show_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/attic-labs/noms/go/spec"
 	"github.com/attic-labs/noms/go/types"
 	"github.com/attic-labs/noms/go/util/clienttest"
+	"github.com/attic-labs/noms/go/util/test"
 	"github.com/attic-labs/testify/suite"
 )
 
@@ -59,7 +60,7 @@ func (s *nomsShowTestSuite) TestNomsShow() {
 	list := types.NewList(types.String("elem1"), types.Number(2), types.String("elem3"))
 	r = writeTestData(str, list)
 	res, _ = s.Run(main, []string{"show", str})
-	s.Equal(res3, res)
+	test.EqualsIgnoreHashes(s.T(), res3, res)
 
 	str1 = spec.CreateValueSpecString("ldb", s.LdbDir, "#"+r.TargetHash().String())
 	res, _ = s.Run(main, []string{"show", str1})
@@ -67,5 +68,5 @@ func (s *nomsShowTestSuite) TestNomsShow() {
 
 	_ = writeTestData(str, s1)
 	res, _ = s.Run(main, []string{"show", str})
-	s.Equal(res5, res)
+	test.EqualsIgnoreHashes(s.T(), res5, res)
 }

go/constants/version.go

@@ -5,6 +5,6 @@
 package constants
 
 // TODO: generate this from some central thing with go generate, so that JS and Go can be easily kept in sync
-const NomsVersion = "4"
+const NomsVersion = "5"
 
 var NomsGitSHA = "<developer build>"

go/datas/commit.go

@@ -15,7 +15,7 @@ const (
 	MetaField    = "meta"
 )
 
-var valueCommitType = makeValueCommitType()
+var valueCommitType = makeCommitType(types.ValueType)
 
 // NewCommit creates a new commit object. The type of Commit is computed based on the type of the value and the type of the parents.
 // It also includes a Meta field whose type is always the empty struct
@@ -38,6 +38,7 @@ var valueCommitType = makeValueCommitType()
 // struct Commit {
 //   meta: struct {},
 //   parents: Set<Ref<struct Commit {
+//     meta: struct {},
 //     parents: Set<Ref<Cycle<0>>>,
 //     value: T | U
 //   }>>,
@@ -52,39 +53,29 @@ func NewCommit(value types.Value, parents types.Set, meta types.Struct) types.St
 	return types.NewStructWithType(t, types.ValueSlice{meta, parents, value})
 }
 
-func makeValueCommitType() *types.Type {
-	fieldNames := []string{ParentsField, ValueField}
-	return types.MakeStructType("Commit", fieldNames, []*types.Type{
-		types.MakeSetType(types.MakeRefType(types.MakeCycleType(0))),
-		types.ValueType,
-	})
-}
-
 func makeCommitType(valueType *types.Type, parentsValueTypes ...*types.Type) *types.Type {
 	tmp := make([]*types.Type, len(parentsValueTypes)+1)
 	copy(tmp, parentsValueTypes)
 	tmp[len(tmp)-1] = valueType
 	parentsValueUnionType := types.MakeUnionType(tmp...)
 	fieldNames := []string{MetaField, ParentsField, ValueField}
-
+	var parentsType *types.Type
 	if parentsValueUnionType.Equals(valueType) {
-		return types.MakeStructType("Commit", fieldNames, []*types.Type{
-			types.EmptyStructType,
-			types.MakeSetType(types.MakeRefType(types.MakeCycleType(0))),
-			valueType,
-		})
-	}
-
-	fieldTypes := []*types.Type{
-		types.EmptyStructType,
-		types.MakeSetType(types.MakeRefType(
+		parentsType = types.MakeSetType(types.MakeRefType(types.MakeCycleType(0)))
+	} else {
+		parentsType = types.MakeSetType(types.MakeRefType(
 			types.MakeStructType("Commit", fieldNames, []*types.Type{
 				types.EmptyStructType,
 				types.MakeSetType(types.MakeRefType(types.MakeCycleType(0))),
 				parentsValueUnionType,
-			}))),
+			})))
+	}
+	fieldTypes := []*types.Type{
+		types.EmptyStructType,
+		parentsType,
 		valueType,
 	}
+
 	return types.MakeStructType("Commit", fieldNames, fieldTypes)
 }
 

go/datas/commit_test.go

@@ -15,6 +15,7 @@ func TestNewCommit(t *testing.T) {
 	assert := assert.New(t)
 
 	commitFieldNames := []string{MetaField, ParentsField, ValueField}
+
 	assertTypeEquals := func(e, a *types.Type) {
 		assert.True(a.Equals(e), "Actual: %s\nExpected %s", a.Describe(), e.Describe())
 	}
@@ -77,18 +78,5 @@ func TestCommitWithoutMetaField(t *testing.T) {
 		"value":   types.Number(9),
 		"parents": types.NewSet(),
 	})
-	assert.True(IsCommitType(noMetaCommit.Type()))
-
-	badCommit := types.NewStruct("Commit", map[string]types.Value{
-		"value":    types.Number(9),
-		"parents1": types.NewSet(),
-	})
-	assert.False(IsCommitType(badCommit.Type()))
-
-	badMetaCommit := types.NewStruct("Commit", map[string]types.Value{
-		"value":    types.Number(9),
-		"parents1": types.NewSet(),
-		"meta":     types.String("one"),
-	})
-	assert.False(IsCommitType(badMetaCommit.Type()))
+	assert.False(IsCommitType(noMetaCommit.Type()))
 }

go/util/test/equals_ignore_hashes.go

@@ -0,0 +1,22 @@
+package test
+
+import (
+	"regexp"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/attic-labs/noms/go/hash"
+	"github.com/attic-labs/testify/assert"
+)
+
+var pattern = regexp.MustCompile("([0-9a-v]{" + strconv.Itoa(hash.StringLen) + "})")
+
+// EqualsIgnoreHashes compares two strings, ignoring hashes in them.
+func EqualsIgnoreHashes(tt *testing.T, expected, actual string) {
+	expected2 := pattern.ReplaceAllString(expected, strings.Repeat("*", hash.StringLen))
+	actual2 := pattern.ReplaceAllString(actual, strings.Repeat("*", hash.StringLen))
+	if expected2 != actual2 {
+		assert.Equal(tt, expected, actual)
+	}
+}

js/src/commit-test.js

@@ -0,0 +1,92 @@
+// @flow
+
+// Copyright 2016 Attic Labs, Inc. All rights reserved.
+// Licensed under the Apache License, version 2.0:
+// http://www.apache.org/licenses/LICENSE-2.0
+
+import {suite, test} from 'mocha';
+import {assert} from 'chai';
+import {equals} from './compare.js';
+import {
+  makeCycleType,
+  makeRefType,
+  makeSetType,
+  makeStructType,
+  makeUnionType,
+  numberType,
+  stringType,
+} from './type.js';
+import Commit, {isCommitType} from './commit.js';
+import Set from './set.js';
+import Ref from './ref.js';
+import {newStruct} from './struct.js';
+
+suite('commit.js', () => {
+  const emptyStructType = makeStructType('', [], []);
+  test('new Commit', () => {
+    function assertTypeEquals(e, a) {
+      assert.isTrue(equals(a, e), `Actual: ${a.describe()}\nExpected ${e.describe()}`);
+    }
+
+    const commitFieldNames = ['meta', 'parents', 'value'];
+
+    const commit = new Commit(1, new Set());
+    const at = commit.type;
+    const et = makeStructType('Commit', commitFieldNames, [
+      emptyStructType,
+      makeSetType(makeRefType(makeCycleType(0))),
+      numberType,
+    ]);
+    assertTypeEquals(et, at);
+
+    // Commiting another Number
+    const commit2 = new Commit(2, new Set([new Ref(commit)]));
+    const at2 = commit2.type;
+    const et2 = et;
+    assertTypeEquals(et2, at2);
+
+    // Now commit a String
+    const commit3 = new Commit('Hi', new Set([new Ref(commit2)]));
+    const at3 = commit3.type;
+    const et3 = makeStructType('Commit', commitFieldNames, [
+      emptyStructType,
+      makeSetType(makeRefType(makeStructType('Commit', commitFieldNames, [
+        emptyStructType,
+        makeSetType(makeRefType(makeCycleType(0))),
+        makeUnionType([numberType, stringType]),
+      ]))),
+      stringType,
+    ]);
+    assertTypeEquals(et3, at3);
+
+    // Now commit a String with MetaInfo
+    const meta = newStruct('Meta', {date: 'some date', number: 9});
+    const commit4 = new Commit('Hi', new Set([new Ref(commit2)]), meta);
+    const at4 = commit4.type;
+    const et4 = makeStructType('Commit', commitFieldNames, [
+      emptyStructType,
+      makeSetType(makeRefType(makeStructType('Commit', commitFieldNames, [
+        emptyStructType,
+        makeSetType(makeRefType(makeCycleType(0))),
+        makeUnionType([numberType, stringType]),
+      ]))),
+      stringType,
+    ]);
+    assertTypeEquals(et4, at4);
+  });
+
+  test('Commit without meta field', () => {
+    const metaCommit = newStruct('Commit', {
+      value: 9,
+      parents: new Set(),
+      meta: newStruct('', {}),
+    });
+    assert.isTrue(isCommitType(metaCommit.type));
+
+    const noMetaCommit = newStruct('Commit', {
+      value: 9,
+      parents: new Set(),
+    });
+    assert.isFalse(isCommitType(noMetaCommit.type));
+  });
+});

js/src/commit.js

@@ -5,7 +5,7 @@
 // http://www.apache.org/licenses/LICENSE-2.0
 
 import {invariant, notNull} from './assert.js';
-import Struct from './struct.js';
+import Struct, {newStruct} from './struct.js';
 import type Value from './value.js';
 import type Ref from './ref.js';
 import Set from './set.js';
@@ -16,6 +16,7 @@ import {
   makeSetType,
   makeStructType,
   makeUnionType,
+  valueType,
 } from './type.js';
 import {equals} from './compare.js';
 import {Kind} from './noms-kind.js';
@@ -24,17 +25,33 @@ import type {
   StructDesc,
   Type,
 } from './type.js';
+import {isSubtype} from './assert-type.js';
+
+
+// Work around npm cyclic dependencies.
+let emptyStruct;
+
+function getEmptyStruct() {
+  if (emptyStruct) {
+    return emptyStruct;
+  }
+  return emptyStruct = newStruct('', {});
+}
+
+const metaIndex = 0;
+const parentsIndex = 1;
+const valueIndex = 2;
 
 export default class Commit<T: Value> extends Struct {
-  constructor(value: T, parents: Set<Ref<Commit>> = new Set()) {
+  constructor(value: T, parents: Set<Ref<Commit>> = new Set(), meta: Struct = getEmptyStruct()) {
     const t = makeCommitType(getTypeOfValue(value), valueTypesFromParents(parents));
-    super(t, [parents, value]);
+    super(t, [meta, parents, value]);
   }
 
   get value(): T {
-    invariant(this.type.desc.fields[1].name === 'value');
+    invariant(this.type.desc.fields[valueIndex].name === 'value');
     // $FlowIssue: _values is private.
-    const value: T = this._values[1];
+    const value: T = this._values[valueIndex];
     return value;
   }
 
@@ -42,37 +59,48 @@ export default class Commit<T: Value> extends Struct {
     return new Commit(value, this.parents);
   }
 
-  get parents(): Set<Ref<Commit>> {
-    invariant(this.type.desc.fields[0].name === 'parents');
+  get parents(): Set<Ref<Commit<*>>> {
+    invariant(this.type.desc.fields[parentsIndex].name === 'parents');
     // $FlowIssue: _values is private.
-    const parents: Set<Ref<Commit>> = this._values[0];
+    const parents: Set<Ref<Commit>> = this._values[parentsIndex];
     invariant(parents instanceof Set);
     return parents;
   }
 
-  setParents(parents: Set<Ref<Commit>>): Commit<T> {
+  setParents(parents: Set<Ref<Commit<*>>>): Commit<T> {
     return new Commit(this.value, parents);
   }
+
+  get meta(): Struct {
+    invariant(this.type.desc.fields[metaIndex].name === 'meta');
+    // $FlowIssue: _values is private.
+    const meta: Struct = this._values[metaIndex];
+    invariant(meta instanceof Struct);
+    return meta;
+  }
+
+  setMeta(meta: Struct): Commit<T> {
+    return new Commit(this.value, this.parents, meta);
+  }
 }
 
 function makeCommitType(valueType: Type<*>, parentsValueTypes: Type<*>[]): Type<StructDesc> {
+  const fieldNames = ['meta', 'parents', 'value'];
   const tmp = parentsValueTypes.concat(valueType);
   const parentsValueUnionType = makeUnionType(tmp);
+  let parentsType;
   if (equals(parentsValueUnionType, valueType)) {
-    return makeStructType('Commit', [
-      'parents', 'value',
-    ], [
-      makeSetType(makeRefType(makeCycleType(0))),
-      valueType,
-    ]);
-  }
-  return makeStructType('Commit', ['parents', 'value'], [
-    makeSetType(makeRefType(makeStructType('Commit', [
-      'parents', 'value',
-    ], [
+    parentsType = makeSetType(makeRefType(makeCycleType(0)));
+  } else {
+    parentsType = makeSetType(makeRefType(makeStructType('Commit', fieldNames, [
+      getEmptyStruct().type,
       makeSetType(makeRefType(makeCycleType(0))),
       parentsValueUnionType,
-    ]))),
+    ])));
+  }
+  return makeStructType('Commit', fieldNames, [
+    getEmptyStruct().type,
+    parentsType,
     valueType,
   ]);
 }
@@ -105,3 +133,9 @@ function valueTypeFromCommit(t: Type<StructDesc>): Type<*> {
   invariant(t.name === 'Commit');
   return notNull(t.desc.getField('value'));
 }
+
+const valueCommitType = makeCommitType(valueType, []);
+
+export function isCommitType(t: Type<StructDesc>): boolean {
+  return isSubtype(valueCommitType, t);
+}

js/src/encode-human-readable.js

@@ -131,8 +131,10 @@ export class TypeWriter {
 
     const desc = t.desc;
     this._w.write('struct ');
-    this._w.write(desc.name);
-    this._w.write(' {');
+    if (desc.name !== '') {
+      this._w.write(`${desc.name} `);
+    }
+    this._w.write('{');
     this._w.indent();
 
     let first = true;

js/src/version.js

@@ -4,4 +4,4 @@
 // Licensed under the Apache License, version 2.0:
 // http://www.apache.org/licenses/LICENSE-2.0
 
-export default '4';
+export default '5';

samples/go/hr/test-data/LOG

@@ -1,10 +1,10 @@
-=============== Jul 12, 2016 (PDT) ===============
-11:15:59.155834 log@legend F·NumFile S·FileSize N·Entry C·BadEntry B·BadBlock Ke·KeyError D·DroppedEntry L·Level Q·SeqNum T·TimeElapsed
-11:15:59.158102 db@open opening
-11:15:59.158720 journal@recovery F·1
-11:15:59.162707 journal@recovery recovering @1
-11:15:59.165885 memdb@flush created L0@2 N·4 S·530B "/ch..\xb4{\xbc,v3":"/vers,v1"
-11:15:59.173907 db@janitor F·3 G·0
-11:15:59.174126 db@open done T·15.903559ms
-11:15:59.178735 db@close closing
-11:15:59.179415 db@close done T·637.521µs
+=============== Jul 21, 2016 (PDT) ===============
+17:11:33.758856 log@legend F·NumFile S·FileSize N·Entry C·BadEntry B·BadBlock Ke·KeyError D·DroppedEntry L·Level Q·SeqNum T·TimeElapsed
+17:11:33.759581 db@open opening
+17:11:33.759710 journal@recovery F·1
+17:11:33.760572 journal@recovery recovering @1
+17:11:33.762512 memdb@flush created L0@2 N·4 S·597B "/ch..Ut:,v3":"/vers,v1"
+17:11:33.765449 db@janitor F·3 G·0
+17:11:33.765486 db@open done T·5.883286ms
+17:11:33.769811 db@close closing
+17:11:33.771730 db@close done T·1.881224ms

samples/go/hr/test-data/LOG.old

@@ -1,7 +1,7 @@
-=============== Jul 12, 2016 (PDT) ===============
-11:15:59.117309 log@legend F·NumFile S·FileSize N·Entry C·BadEntry B·BadBlock Ke·KeyError D·DroppedEntry L·Level Q·SeqNum T·TimeElapsed
-11:15:59.118173 db@open opening
-11:15:59.120152 db@janitor F·2 G·0
-11:15:59.120196 db@open done T·2.00235ms
-11:15:59.121187 db@close closing
-11:15:59.121293 db@close done T·102.816µs
+=============== Jul 21, 2016 (PDT) ===============
+17:11:33.730161 log@legend F·NumFile S·FileSize N·Entry C·BadEntry B·BadBlock Ke·KeyError D·DroppedEntry L·Level Q·SeqNum T·TimeElapsed
+17:11:33.731468 db@open opening
+17:11:33.738966 db@janitor F·2 G·0
+17:11:33.739028 db@open done T·7.525784ms
+17:11:33.740040 db@close closing
+17:11:33.740180 db@close done T·136.355µs