fix(save): Save change detection with readme works correctly

base/dsfs/dataset.go

@@ -618,6 +618,37 @@ func generateCommitDescriptions(store cafs.Filestore, prev, ds *dataset.Dataset,
 		_ = ds.Transform.OpenScriptFile(ctx, localfs.NewFS())
 	}
 
+	// Read the readme files to see if they changed.
+	// TODO(dustmop): Would be better to get a line-by-line diff
+	if prev.Readme != nil && prev.Readme.ScriptPath != "" {
+		err := prev.Readme.OpenScriptFile(ctx, store)
+		if err != nil {
+			log.Error("prev.Readme.ScriptPath %q open err: %s", prev.Readme.ScriptPath, err)
+		} else {
+			tfFile := prev.Readme.ScriptFile()
+			prev.Readme.ScriptBytes, err = ioutil.ReadAll(tfFile)
+			if err != nil {
+				log.Error("prev.Readme.ScriptPath %q read err: %s", prev.Readme.ScriptPath, err)
+			}
+		}
+	}
+	if ds.Readme != nil && 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?
+		err := ds.Readme.OpenScriptFile(ctx, localfs.NewFS())
+		if err != nil {
+			log.Error("ds.Readme.ScriptPath %q open err: %s", ds.Readme.ScriptPath, err)
+		} else {
+			tfFile := ds.Readme.ScriptFile()
+			ds.Readme.ScriptBytes, err = ioutil.ReadAll(tfFile)
+			if err != nil {
+				log.Error("ds.Readme.ScriptPath %q read err: %s", ds.Readme.ScriptPath, err)
+			}
+		}
+		// Reopen the readme file so that WriteDataset will be able to write it to the store.
+		_ = ds.Readme.OpenScriptFile(ctx, localfs.NewFS())
+	}
+
 	var prevData map[string]interface{}
 	prevData, err = toqtype.StructToMap(prev)
 	if err != nil {

cmd/fsi_integration_test.go

@@ -1651,6 +1651,64 @@ func TestUnlinkDirectoryButRefNotFound(t *testing.T) {
 	}
 }
 
+// Test that saving with readme changes work correctly
+func TestSaveWithReadmeChange(t *testing.T) {
+	run := NewFSITestRunner(t, "qri_test_save_readme_change")
+	defer run.Delete()
+
+	_ = run.CreateAndChdirToWorkDir("readme_change")
+
+	// Init as a linked directory, save default dataset.
+	run.MustExec(t, "qri init --name readme_change --format csv")
+	run.MustExec(t, "qri save")
+
+	// Write a readme and save
+	run.MustWriteFile(t, "readme.md", `# Title\n\ncontent\n`)
+	output := run.MustExecCombinedOutErr(t, "qri save")
+	if !strings.Contains(output, "dataset saved") {
+		t.Errorf("expected save to succeed, creating the second commit")
+	}
+
+	output = run.MustExecCombinedOutErr(t, "qri status")
+	if diff := cmpTextLines(cleanStatusMessage("test_peer/readme_change"), output); diff != "" {
+		t.Errorf("qri status (-want +got):\n%s", diff)
+	}
+
+	// Write a second readme and save
+	run.MustWriteFile(t, "readme.md", `# Title\n\nmore content\n`)
+	output = run.MustExecCombinedOutErr(t, "qri save")
+	if !strings.Contains(output, "dataset saved") {
+		t.Errorf("expected save to succeed, creating the second commit")
+	}
+
+	output = run.MustExecCombinedOutErr(t, "qri status")
+	if diff := cmpTextLines(cleanStatusMessage("test_peer/readme_change"), output); diff != "" {
+		t.Errorf("qri status (-want +got):\n%s", diff)
+	}
+
+	// Write a third readme and save
+	run.MustWriteFile(t, "readme.md", `# Title\n\neven more content\n`)
+	output = run.MustExecCombinedOutErr(t, "qri save")
+	if !strings.Contains(output, "dataset saved") {
+		t.Errorf("expected save to succeed, creating the second commit")
+	}
+
+	output = run.MustExecCombinedOutErr(t, "qri status")
+	if diff := cmpTextLines(cleanStatusMessage("test_peer/readme_change"), output); diff != "" {
+		t.Errorf("qri status (-want +got):\n%s", diff)
+	}
+
+	// Should fail because there's no changes
+	err := run.ExecCommand("qri save")
+	if err == nil {
+		t.Fatal("expected error trying to save, did not get an error")
+	}
+	expect := "error saving: no changes"
+	if err.Error() != expect {
+		t.Errorf("error mismatch, expect: %s, got: %s", expect, err.Error())
+	}
+}
+
 func parseRefFromSave(output string) string {
 	pos := strings.Index(output, "saved: ")
 	if pos == -1 {

cmd/qri.go

@@ -105,7 +105,7 @@ type QriOptions struct {
 	// Whether to log all activity by enabling logging for all packages
 	LogAll bool
 	// inst is the Instance that holds state needed by qri's methods
-	inst        *lib.Instance
+	inst *lib.Instance
 }
 
 // NewQriOptions creates an options object

cmd/save_test.go

@@ -698,3 +698,29 @@ func TestSaveTwiceWithTransform(t *testing.T) {
 //	// Save with a transform that results in a different shape (non tabular)
 //	run.MustExec(t, "qri save --file testdata/movies/tf_123.star test_peer/my_ds")
 //}
+
+// Test that saving with only a readme change will succeed
+func TestSaveWithReadmeFiles(t *testing.T) {
+	run := NewFSITestRunner(t, "qri_test_save_readme_files")
+	defer run.Delete()
+
+	err := run.ExecCommand("qri save --body testdata/movies/body_ten.csv me/with_readme")
+	if err != nil {
+		t.Errorf("expected save to succeed, got %s", err)
+	}
+
+	err = run.ExecCommand("qri save --file testdata/movies/about_movies.md me/with_readme")
+	if err != nil {
+		t.Errorf("expected save to succeed, got %s", err)
+	}
+
+	err = run.ExecCommand("qri save --file testdata/movies/more_movies.md me/with_readme")
+	if err != nil {
+		t.Errorf("expected save to succeed, got %s", err)
+	}
+
+	err = run.ExecCommand("qri save --file testdata/movies/even_more_movies.md me/with_readme")
+	if err != nil {
+		t.Errorf("expected save to succeed, got %s", err)
+	}
+}

cmd/testdata/movies/even_more_movies.md

@@ -0,0 +1,3 @@
+# Title
+
+This is a dataset with even more movies

cmd/testdata/movies/more_movies.md

@@ -0,0 +1,3 @@
+# Title
+
+This is a dataset with more movies

dsref/generate_test.go

@@ -118,8 +118,8 @@ func TestGenerateNameCutoffWord(t *testing.T) {
 	NameMaxLength = 18
 
 	cases := []struct {
-		input       string
-		expect      string
+		input  string
+		expect string
 	}{
 		{
 			"the quick brown fox jumped",

go.sum

@@ -756,6 +756,8 @@ github.com/qri-io/compare v0.1.0 h1:A/MRx3uEnJ/iMjfJY1VOqH9CYs9zFSEYaFVeXuGfmis=
 github.com/qri-io/compare v0.1.0/go.mod h1:i/tVuDGRXVxhuZ8ZUieF23u6rQ6wLGJl7KKWpoMRaTE=
 github.com/qri-io/dag v0.2.1-0.20191025201336-254aa177fbd7 h1:hujyzdufAz3Hi6I0I0yKyJTLCM9eoLamKhlpwaqE7ec=
 github.com/qri-io/dag v0.2.1-0.20191025201336-254aa177fbd7/go.mod h1:gMpEjalFfB6tGDCiOx1bmqsKhKhqnPHOnEycsqY4lqc=
+github.com/qri-io/dag v0.2.1-0.20200317231253-5cd938b03caf h1:akfYjBZY3yqM1PHvYOwuoi7zxGfcSF+kD6eB8bYtkEU=
+github.com/qri-io/dag v0.2.1-0.20200317231253-5cd938b03caf/go.mod h1:gMpEjalFfB6tGDCiOx1bmqsKhKhqnPHOnEycsqY4lqc=
 github.com/qri-io/dataset v0.1.5-0.20200324184139-108a69072ede h1:CcN+fr18CZ4NZkBR4pHLdLQD1Mh59pYQeI2ZLZHelq0=
 github.com/qri-io/dataset v0.1.5-0.20200324184139-108a69072ede/go.mod h1:efJPKDyyGItDnI9CzKuQYmKiEP0xp4mIxIut7o/bG2g=
 github.com/qri-io/deepdiff v0.1.1-0.20200305020550-8173efebcaa1 h1:LR8poNGx/qkhcNU0eWmsPexQ69s/MK+kvFNfAhTTlK0=