attic-labs / noms

The versioned, forkable, syncable database
Apache License 2.0
7.44k stars 266 forks source link

'noms diff' should ignore different parents #3823

Open lak opened 5 years ago

lak commented 5 years ago

I'm experimenting with json-import, and getting some surprising behavior.

I would expect that two datasets would show no diff, but noms seems to consider different parents to indicate different data sets. IMO, the parentage is an aspect of the database, not of the data itself, and thus should not be in diff output.

Note that the text below also shows date in the meta; I don't know whether this is an artifact of json-import, or noms diff. If the latter, then I would also expect that not to be shown. Of course two different commits are going to have different dates and parents, so these should be skipped.


Parent: mpvtg5ng1vjokafas1nrir8c9hpsqovn
Date:   "2019-02-01T17:48:38Z"
Url:    "output.json"

commit mpvtg5ng1vjokafas1nrir8c9hpsqovn
Parent: clah499tslo1cnm2efu242ggu63v2fh8
Date:   "2019-02-01T17:48:30Z"
Url:    "output.json"

commit clah499tslo1cnm2efu242ggu63v2fh8
Parent: None
Date:   "2019-02-01T17:34:12Z"
Url:    "output.json"

[master ~/vobject]
luke@lak-5kimac15-work $ noms diff jsondb::#bc2vr3i95nn47n5dagol2cm0k9543opf jsondb::#mpvtg5ng1vjokafas1nrir8c9hpsqovn
.meta {
-   date: "2019-02-01T17:48:38Z"
+   date: "2019-02-01T17:48:30Z"
  }
.parents {
+   #clah499tslo1cnm2efu242ggu63v2fh8
-   #mpvtg5ng1vjokafas1nrir8c9hpsqovn
  }
[master ~/vobject]
luke@lak-5kimac15-work $ noms diff jsondb::#clah499tslo1cnm2efu242ggu63v2fh8 jsondb::#mpvtg5ng1vjokafas1nrir8c9hpsqovn
.meta {
-   date: "2019-02-01T17:34:12Z"
+   date: "2019-02-01T17:48:30Z"
  }
.parents {
+   #clah499tslo1cnm2efu242ggu63v2fh8
  }```
aboodman commented 5 years ago

Hi @lak - we've gone back and forth about this. You're right it's confusing when you first try it.

You can get the behavior you want, by saying:

noms diff jsondb::#bc2vr3i95nn47n5dagol2cm0k9543opf.value jsondb::#mpvtg5ng1vjokafas1nrir8c9hpsqovn.value

That is, ask noms to just compare the value of the commits, not the commit objects themselves.

I think that when diffing commits, the user typically wants the behavior you ask for. But the challenge is that noms diff can diff any value in the database, not just commits. So making it special case commits is a bit weird, especially when you consider that as a user you can just create a struct with name Commit. Should we special case when you diff that?

Git special-cases commits to make the output prettier in this case, but git doesn't have the problem of users writing arbitrary objects to its database.

aboodman commented 5 years ago

I wonder if we had auto-complete, if this would feel less cumbersome.

lak commented 5 years ago

I expect there are ways to make this less confusing.

But IMO including non-content differences by default is a bad choice. If you claim idempotency, then the core tools should exhibit it by default. Yes, you could get non-idempotent behavior (e.g., showing different parents and dates) but you would have to pick it.

aboodman commented 5 years ago

Yeah, I suppose I could imagine a --compare-commit-values that defaults to true which means to only compare the .value struct of any Commit object.

It's a bit unfortunate to have the special case, but maybe worth it since this is a common case.

aboodman commented 5 years ago

PS: I believe that having json-import auto-commit in the first place was a bug, and it has since been changed. noms commit won't commit a duplicate value by default. So with the current version of both programs, you wouldn't have run into this as easily.