fix(dsfs): set script paths before generating commit messages

split the "updatePaths" function into two separate purposes: setting script
file paths, and converting component data to references, apply these at
different times in the commit component write hook

base/dsfs/commit.go

@@ -14,7 +14,6 @@ import (
 	"github.com/qri-io/dataset/dsio"
 	"github.com/qri-io/deepdiff"
 	"github.com/qri-io/qfs"
-	"github.com/qri-io/qfs/localfs"
 	"github.com/qri-io/qri/base/friendly"
 	"github.com/qri-io/qri/base/toqtype"
 )
@@ -69,14 +68,17 @@ func commitFileAddFunc(privKey crypto.PrivKey) addWriteFileFunc {
 		}
 
 		hook := func(ctx context.Context, f qfs.File, added map[string]string) (io.Reader, error) {
+
 			if cff, ok := wfs.body.(*computeFieldsFile); ok {
+				updateScriptPaths(ds, added)
+
 				if err := generateCommitTitleAndMessage(ctx, cff.fs, cff.pk, cff.ds, cff.prev, cff.bodyAct, cff.sw.FileHint, cff.sw.ForceIfNoChanges); err != nil {
 					log.Debugf("generateCommitTitleAndMessage: %s", err)
 					return nil, err
 				}
 			}
 
-			updatePaths(ds, added, wfs.body.FullPath())
+			replaceComponentsWithRefs(ds, added, wfs.body.FullPath())
 
 			signedBytes, err := privKey.Sign(ds.SigningBytes())
 			if err != nil {
@@ -161,12 +163,6 @@ func generateCommitDescriptions(ctx context.Context, fs qfs.Filesystem, ds, prev
 	}
 	if ds.Transform != nil && ds.Transform.ScriptPath != "" {
 		log.Debugf("inlining next transform ScriptPath=%q", ds.Transform.ScriptPath)
-		// TODO(dustmop): The ipfs filestore won't recognize local filepaths, we need to use
-		// local here. Is there some way to have a cafs store that works with both?
-		fs, err := localfs.NewFS(nil)
-		if err != nil {
-			log.Errorf("error setting up local fs: %s", err)
-		}
 		err = ds.Transform.OpenScriptFile(ctx, fs)
 		if err != nil {
 			log.Errorf("ds.Transform.ScriptPath %q open err: %s", ds.Transform.ScriptPath, err)
@@ -200,12 +196,6 @@ func generateCommitDescriptions(ctx context.Context, fs qfs.Filesystem, ds, prev
 	}
 	if ds.Readme != nil && ds.Readme.ScriptPath != "" {
 		log.Debugf("inlining next readme ScriptPath=%q", ds.Readme.ScriptPath)
-		// TODO(dustmop): The ipfs filestore won't recognize local filepaths, we need to use
-		// local here. Is there some way to have a cafs store that works with both?
-		fs, err := localfs.NewFS(nil)
-		if err != nil {
-			log.Error("localfs.NewFS err: %s", err)
-		}
 		err = ds.Readme.OpenScriptFile(ctx, fs)
 		if err != nil {
 			log.Debugf("ds.Readme.ScriptPath %q open err: %s", ds.Readme.ScriptPath, err)

base/dsfs/dataset.go

@@ -158,16 +158,16 @@ func CreateDataset(
 	}
 	log.Debugf("CreateDataset ds.Peername=%q ds.Name=%q writeDestType=%s", ds.Peername, ds.Name, destination.Type())
 
-	if prev != nil {
+	if prev != nil && !prev.IsEmpty() {
 		log.Debugw("dereferencing previous dataset", "prevPath", prev.Path)
 		if err := DerefDataset(ctx, source, prev); err != nil {
 			log.Debug(err.Error())
 			return "", err
 		}
-		// if err := validate.Dataset(prev); err != nil {
-		// 	log.Debug(err.Error())
-		// 	return "", err
-		// }
+		if err := validate.Dataset(prev); err != nil {
+			log.Debug(err.Error())
+			return "", err
+		}
 	}
 
 	var (
@@ -315,8 +315,8 @@ func addDatasetFile(ds *dataset.Dataset, wfs *writeFiles) error {
 
 	hook := func(ctx context.Context, f qfs.File, added map[string]string) (io.Reader, error) {
 		ds.DropTransientValues()
-
-		updatePaths(ds, added, wfs.body.FullPath())
+		updateScriptPaths(ds, added)
+		replaceComponentsWithRefs(ds, added, wfs.body.FullPath())
 
 		if path, ok := added[PackageFileCommit.Filename()]; ok {
 			ds.Commit = dataset.NewCommitRef(path)
@@ -346,7 +346,7 @@ func jsonWriteHook(filename string, data json.Marshaler) qfs.WriteHook {
 	}
 }
 
-func updatePaths(ds *dataset.Dataset, added map[string]string, bodyPathName string) {
+func updateScriptPaths(ds *dataset.Dataset, added map[string]string) {
 	for filepath, addr := range added {
 		switch filepath {
 		case PackageFileVizScript.Filename():
@@ -355,6 +355,13 @@ func updatePaths(ds *dataset.Dataset, added map[string]string, bodyPathName stri
 			ds.Viz.RenderedPath = addr
 		case PackageFileReadmeScript.Filename():
 			ds.Readme.ScriptPath = addr
+		}
+	}
+}
+
+func replaceComponentsWithRefs(ds *dataset.Dataset, added map[string]string, bodyPathName string) {
+	for filepath, addr := range added {
+		switch filepath {
 		case PackageFileStructure.Filename():
 			ds.Structure = dataset.NewStructureRef(addr)
 		case PackageFileViz.Filename():
@@ -367,8 +374,4 @@ func updatePaths(ds *dataset.Dataset, added map[string]string, bodyPathName stri
 			ds.BodyPath = addr
 		}
 	}
-
-	if vizScriptPath, ok := added[PackageFileVizScript.Filename()]; ok {
-		ds.Viz.ScriptPath = vizScriptPath
-	}
 }

base/dsfs/meta.go

@@ -30,7 +30,9 @@ func loadMeta(ctx context.Context, fs qfs.Filesystem, path string) (md *dataset.
 		log.Debug(err.Error())
 		return nil, fmt.Errorf("loading metadata file: %w", err)
 	}
-	return dataset.UnmarshalMeta(data)
+	md = &dataset.Meta{}
+	err = md.UnmarshalJSON(data)
+	return md, err
 }
 
 func addMetaFile(ds *dataset.Dataset, wfs *writeFiles) error {

cmd/cmd_test.go

@@ -498,9 +498,9 @@ func TestSaveTransformModifiedButSameBody(t *testing.T) {
     Storage: local
     Size:    7 B
 
-    transform removed scriptBytes
+    transform updated scriptBytes
     transform:
-    	removed scriptBytes
+    	updated scriptBytes
 
 2   Commit:  {{ .commit2 }}
     Date:    Sun Dec 31 20:01:01 EST 2000
@@ -510,7 +510,7 @@ func TestSaveTransformModifiedButSameBody(t *testing.T) {
     created dataset from tf_123.star
 
 `, map[string]string{
-		"commit1": "/ipfs/QmNnmosunfY62w5rUwtGYLAWT8wZBJ22da5TWrjR84vgAd",
+		"commit1": "/ipfs/QmQjJjAGF9mD4ArnBHCLmDT8AEProLbmpbF3VdBMZFL5yE",
 		"commit2": "/ipfs/QmWfeoPUpus3YiSHoE6qHnDP4jyvC7gNVFuJFi6M2Y2iD7",
 	})
 	if diff := cmp.Diff(expect, output); diff != "" {

cmd/fsi_integration_test.go

@@ -52,6 +52,7 @@ func (run *FSITestRunner) MustExec(t *testing.T, cmdText string) string {
 
 // MustExec runs a command, returning combined standard output and standard err
 func (run *FSITestRunner) MustExecCombinedOutErr(t *testing.T, cmdText string) string {
+	t.Helper()
 	var shutdown func() <-chan error
 
 	run.CmdR, shutdown = run.CreateCommandRunnerCombinedOutErr(context.Background())
@@ -1307,7 +1308,7 @@ run ` + "`qri save`" + ` to commit this dataset
 	output = run.MustExecCombinedOutErr(t, "qri diff")
 	expect = `for linked dataset [test_peer_diff_after_change/diff_change]
 
--23 elements. 5 inserts. 5 deletes.
+-22 elements. 6 inserts. 5 deletes.
 
  body: 
    0: 
@@ -1323,7 +1324,7 @@ run ` + "`qri save`" + ` to commit this dataset
     -2: 6
     +2: 321
  meta: 
-   qri: "md:0"
+  +qri: "md:0"
   +title: "hello"
  qri: "ds:0"
 -stats: {"qri":"sa:0","stats":[{"count":2,"frequencies":{},"maxLength":4,"minLength":3,"type":"string"},{"count":2,"frequencies":{},"maxLength":4,"minLength":3,"type":"string"},{"count":2,"histogram":{"bins":null,"frequencies":[]},"max":6,"mean":9,"min":3,"type":"numeric"}]}
@@ -1483,7 +1484,7 @@ func TestMoveWorkingDirectory(t *testing.T) {
 
 `, map[string]string{
 		"profileID": "QmeL2mdVka1eahKENjehK6tBxkkpk5dNQ1qMcgWi7Hrb4B",
-		"path":      "/ipfs/QmWUUi1u5hM9k3s5vicfXVqJvAtXLB3NQvpChcB86nX7kg",
+		"path":      "/ipfs/QmZkcwvx5akGMQ9xUXEDZAo3ZZYmeVbBGsu6noHpmsEaXq",
 	})
 	if diff := cmp.Diff(expect, output); diff != "" {
 		t.Errorf("unexpected (-want +got):\n%s", diff)
@@ -1524,7 +1525,7 @@ func TestRemoveWorkingDirectory(t *testing.T) {
 
 `, map[string]string{
 		"profileID": "QmeL2mdVka1eahKENjehK6tBxkkpk5dNQ1qMcgWi7Hrb4B",
-		"path":      "/ipfs/QmWUUi1u5hM9k3s5vicfXVqJvAtXLB3NQvpChcB86nX7kg",
+		"path":      "/ipfs/QmZkcwvx5akGMQ9xUXEDZAo3ZZYmeVbBGsu6noHpmsEaXq",
 	})
 
 	if diff := cmp.Diff(expect, output); diff != "" {

cmd/testdata/expect/TestSaveThenOverrideMetaAndTransformAndViz.json

@@ -16,7 +16,7 @@
     "qri": "md:0",
     "title": "different title"
   },
-  "path": "/ipfs/QmQJqCA55NUYF3XEkSkWaJFhD5csiNKqPnHJF9cdfDCTok",
+  "path": "/ipfs/QmTkZuXUCxcGsuhUbnfG4QHiDqhtVaTDnzStsd6PsnHxxV",
   "previousPath": "/ipfs/QmPY4xPQSSGSYZny4QLPn574VdeP84L7Nf3oCY1DxYAv5n",
   "qri": "ds:0",
   "structure": {
@@ -75,6 +75,7 @@
   },
   "transform": {
     "qri": "tf:0",
+    "scriptBytes": "IyB0cmFuc2Zvcm0gdGhhdCBkb2Vzbid0IGNoYW5nZSBhbnl0aGluZy4KZGVmIHRyYW5zZm9ybShkcywgY3R4KToKICBwcmludCgiaGVsbG8gUXJpISBcbiIp",
     "scriptPath": "/ipfs/Qmb69tx5VCL7q7EfkGKpDgESBysmDbohoLvonpbgri48NN",
     "syntax": "starlark",
     "syntaxVersion": "test_version"

cmd/testdata/expect/TestSaveThenOverrideTransform.json

@@ -16,7 +16,7 @@
     "qri": "md:0",
     "title": "example movie data"
   },
-  "path": "/ipfs/QmXDCMMF8sjVk8SLeWo7cdaYq2K5FaspBTmDrZkxmZhEMq",
+  "path": "/ipfs/QmPuSQE6da9HtgBcF8BgAkUZhkA4BBzrBpJkmZWrYAnypb",
   "previousPath": "/ipfs/QmPY4xPQSSGSYZny4QLPn574VdeP84L7Nf3oCY1DxYAv5n",
   "qri": "ds:0",
   "structure": {
@@ -75,6 +75,7 @@
   },
   "transform": {
     "qri": "tf:0",
+    "scriptBytes": "IyB0cmFuc2Zvcm0gdGhhdCBkb2Vzbid0IGNoYW5nZSBhbnl0aGluZy4KZGVmIHRyYW5zZm9ybShkcywgY3R4KToKICBwcmludCgiaGVsbG8gUXJpISBcbiIp",
     "scriptPath": "/ipfs/Qmb69tx5VCL7q7EfkGKpDgESBysmDbohoLvonpbgri48NN",
     "syntax": "starlark",
     "syntaxVersion": "test_version"

go.mod

@@ -43,7 +43,7 @@ require (
 	github.com/pkg/profile v1.5.0
 	github.com/qri-io/apiutil v0.2.0
 	github.com/qri-io/dag v0.2.2-0.20201113214519-3219603c4d1a
-	github.com/qri-io/dataset v0.2.1-0.20201117033713-f588ddee464f
+	github.com/qri-io/dataset v0.2.1-0.20201118223538-aec8f29a5a2c
 	github.com/qri-io/deepdiff v0.2.1-0.20200807143746-d02d9f531f5b
 	github.com/qri-io/doggos v0.1.0
 	github.com/qri-io/ioes v0.1.1