switch to the gogo protobuf package in the filestore

[Stebalien]

No description provided.

[ghost]

What's the story of this, just a leftover from moving everything to gogo protobuf?

[Stebalien]

No idea. However, we're using one version of protobuf here, the same in prometheus, and a different version in badger (needs a newer version). However, I need to update the version in prometheus to the one that we use in badger so I figured we should just kill this one off (@whyrusleeping had no idea why we were using this here either).

[whyrusleeping]

Its worth noting though, that the version of gogo protobuf we are using has a serialization order bug. If we try to update gogo, it will break all of our hashes. This is no bueno

[Stebalien]

I'd like to vendor that old version of gogo and rename it to something like gogo-for-ipfs.

[ghost]

If we try to update gogo, it will break all of our hashes. This is no bueno

Maybe that's something to coordinate with the base32 changes.

[kevina]

@lgierth

Maybe that's something to coordinate with the base32 changes.

The base32 change will not break change hashes, Just the way there are presented.

[magik6k]

Its worth noting though, that the version of gogo protobuf we are using has a serialization order bug

Is it not possible to rewrite the protobufs in a way that is preserving the 'broken' order of things?

[Stebalien]

Is it not possible to rewrite the protobufs in a way that is preserving the 'broken' order of things?

No. In "canonical" protobuf, fields are serialized in tag order. In the buggy gogo, fields were being serialized in the order in which they appeared in the protobuf schema. Unfortunately, we list the Links field first and the Data field second while the Links field has a tag of 2 and the Data field has a tag of 1.

[Stebalien]

However... we can update the protobuf library as long as we don't update the generated protobuf marshaler in merkledag/pb/merkledag.pb.go.

[kevina]

However... we can update the protobuf library as long as we don't update the generated protobuf marshaler in merkledag/pb/merkledag.pb.go.

That strikes me as just asking for trouble down the road. What is to keep some other developer from regenerating this file using the update protobuf library?

[Stebalien]

That strikes me as just asking for trouble down the road. What is to keep some other developer from regenerating this file using the update protobuf library?

My plan is to add a constant (DoNotUpgradeFileEverItWillChangeYourHashes) to the generated file (checking it elsewhere). I was also planning on renaming it.

Note: this is already a problem as this file is generated by a command (where the version of the command isn't enforced by gx).

[kevina]

My plan is to add a constant (DoNotUpgradeFileEverItWillChangeYourHashes) to the generated file (checking it elsewhere). I was also planning on renaming it.

Okay that can work as long we make it clear why we can't regenerate the file so developers not familiar with the issue won't be tempted to override the check.

[Stebalien]

Yep. It'll have a big comment. Maybe I'll even set the constant to a multi-line string describing the issue.

[Kubuxu]

We recently had a PR that was reapplying the old bug on fresh gogoprotobuf. Maybe we should use that?

[Stebalien]

We recently had a PR that was reapplying the old bug on fresh gogoprotobuf. Maybe we should use that?

We don't actually need that. Really, we just need the one generated function in the generated go file.

[Stebalien]

Done in a different PR.