Support unixfs 1.5 inline metadata

[iand]

This brings go-unixfs in line with the unixfs 1.5 specification.

It is an alternative to #85 that is stricter regarding updates to file mode and mtime, following the requirements in UNIXFS.md#metadata. It supports explicitly setting file metadata via the DagBuilderParams and also reads it from any embedded stat info from Filestore nodes. Tests have been added for both sources on balanced and trickle importers. Many thanks to @kstuart for the groundwork on #85.

[welcome]

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

• The context for this PR is clear, with relevant discussion, decisions
  and stakeholders linked/mentioned.

• Your contribution itself is clear (code comments, self-review for the
  rest) and in its best form. Follow the code contribution
  guidelines
  if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

• A maintainer will triage and assign priority to this PR, commenting on
  any missing things and potentially assigning a reviewer for high
  priority items.

• The PR gets reviews, discussed and approvals as needed.

• The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

[johnnymatthews]

Looks sound from my side but we likely need an actual SWE to take a look at this too.

[aschmahmann]

Thanks @iand. @rvagg or @achingbrain do you have any time to look at this since I think you've both spent more time with the Unixfs (1.5) spec than I have.

[BigLep]

2022-03-29 conversation during IPFS steward standup: the main use case that drives this is wanting to do things like rsync over IPFS data.

Go IPFS Stewards aren't prioritizing this work amongst their other reviews in the "Best Effort Track". We don't have the review list organized there yet to say where it sits (which we recognize is not helpful for those waiting on review).

It would be helpful if could get help from others with expertise here like @rvagg or @achingbrain (given that js-ipfs has implemented this).

Question for reviewer: how problematic will it be to merge this without a corresponding PR in go-unixfsnode?

[iand]

Question for reviewer: how problematic will it be to merge this without a corresponding PR in go-unixfsnode?

Note that this PR is bringing this repo up to par with go-unixfsnode and js-ipfs-unixfs. In particular go-unixfsnode already supports the mode and mtime protobuf fields added in this PR. See https://github.com/ipfs/go-unixfsnode/blob/main/data/gen/main.go#L60

[rvagg]

Looks good to me (from an admittedly fairly quick review); the important bits line up with what's supposed to be where so from a compatibility standpoint this seems good.
It would be interesting to see some CIDs lining up between the implementations with the same data to check that they are actually correct.
Also, probably obvious to everyone here, this is going to be breaking for go-ipfs users in the sense that the same directories imported will generate new CIDs and therefore duplicate blocks to the same directory that may have already been imported. But we've broken that contract plenty over time so I don't suppose it's a major concern, just not something we'd want to do without bumping the version significantly? (it'd be nice if we had a semver-major that wasn't 0 to bump!)

[aschmahmann]

Also, probably obvious to everyone here, this is going to be breaking for go-ipfs users in the sense that the same directories imported will generate new CIDs and therefore duplicate blocks to the same directory that may have already been imported.

Is this true? IIUC we shouldn't be changing the CIDs unless the user actively chooses to pass in metadata and therefore no CIDs should change upstream unless the new features are explicitly opted in to.

I'd hope that by default we choose not to include any metadata since for the most part people expect UnixFS-ifying some data to produce the same CID when passed the same parameters on the same binary/library version. Some folks will need the metadata (biggest use cases I recall are folks relying on rsync and timestamps), but those should be in the minority and be able to opt into this.

[achingbrain]

Metadata was an opt-in thing for js-ipfs too, it shouldn't result in different CIDs out of the box.

It would be interesting to see some CIDs lining up between the implementations with the same data to check that they are actually correct.

Yes, that would be really helpful. Tests for this can be added to the files section of the interop suite.

[BigLep]

@iand : are you able to add the requested tests?

@rvagg : can you followup on Adin and Alex's comments about metadata being opt-in?

[iand]

@BigLep yes I will add the tests

[kstuart]

@rvagg if you don't elect to store the attributes (or conversely remove them) the same CIDs will be generated as before, the CIDs will change if you add or update them. There are explicit sharness tests that check this, and numerous previously existing tests that would fail as a side effect if this were not the case.

@iand thank you for your contribution, taking a quick look at the code please be aware that os.FileMode and unix mode permissions are not directly interchangeable, you'd need to convert between the two.

Obviously I'd prefer the focus to be on my own PR as it's developed alongside the full feature, I will be pushing updates to the feature as a whole either this weekend or next and would value your feedback, maybe we could take stock and review then.

[rvagg]

There are explicit sharness tests that check this

Then 👍 from me

[BigLep]

So what are the next steps here?

@iand : are you going to be adding test ?
@kstuart : are you pushing more changes to #85 ?

[iand]

Quick update: I am working on https://github.com/ipfs/interop testing this week

[iand]

Quick update: I am working on https://github.com/ipfs/interop testing this week

Currently blocked by ipfs/interop#480

[Jorropo]

Replaced by ipfs/boxo#317