dag: diff: check CIDs in base case when comparing nodes

[schomatis]

The base case was assuming that the nodes were different (see issue comment).

Fixes #4591.

[magik6k]

I think that we should check if the data has changed - this is still going to be sort-of broken if there are links in the node and the data is changed.

[schomatis]

@whyrusleeping As you implemented ipfs object diff could you review this please?

[schomatis]

I think that we should check if the data has changed - this is still going to be sort-of broken if there are links in the node and the data is changed.

@magik6k Could you elaborate more on this please? I'm not sure I follow, isn't the CID reflecting the data changes?

[magik6k]

What I mean is that in case where the data changes in top-level node and we have some links, diff won't show any changes:

# Add some data
$ printf '{"Data":"foo"}' | ipfs object put
added QmQeGyS87nyijii7kFt1zbe4n2PsXTFimzsdxyE9qh9TST
$ printf '{"Data":"bar"}' | ipfs object put
added QmNeYRbCibmaMMK6Du6ChfServcLqFvLJF76PzzF76SPrZ

# Diff data without links, this works fine
$ ipfs object diff QmQeGyS87nyijii7kFt1zbe4n2PsXTFimzsdxyE9qh9TST QmNeYRbCibmaMMK6Du6ChfServcLqFvLJF76PzzF76SPrZ
~ QmQeGyS87nyijii7kFt1zbe4n2PsXTFimzsdxyE9qh9TST QmNeYRbCibmaMMK6Du6ChfServcLqFvLJF76PzzF76SPrZ ""

# Add more data, this time with links:
$ printf '{"Data":"abcd", "Links":[{"Hash":"QmQeGyS87nyijii7kFt1zbe4n2PsXTFimzsdxyE9qh9TST"}]}' | ipfs object put
added QmWB7VRDLSweiURLoKMtan2ZCneBXmYAHpsrbmvC8tEZav
$ printf '{"Data":"dcba", "Links":[{"Hash":"QmQeGyS87nyijii7kFt1zbe4n2PsXTFimzsdxyE9qh9TST"}]}' | ipfs object put
added QmdjxDJgR1hvXNSfuwWniB3DsBgv4C1SSYu6RAxwbd38H2

# Diff
$ ipfs object diff QmWB7VRDLSweiURLoKMtan2ZCneBXmYAHpsrbmvC8tEZav QmdjxDJgR1hvXNSfuwWniB3DsBgv4C1SSYu6RAxwbd38H2

# No output, we should get one like in the case without links

Note that this is also the current behaviour, not caused by the changes here, so sorry for the confusion.

[schomatis]

@magik6k Thanks for the clarification, I was under the impression that the data was only reserved for the leaf nodes, not that a node could have both data and links. Yes, this was out of the scope of the original issue, but I could open a new one to discuss this further.

[whyrusleeping]

LGTM, lets merge after 0.4.14 ships.

Thanks for all the PRs @schomatis :) Keep 'em coming!

[whyrusleeping]

Three in a row!