Set data package_uuid to sha256 hash of preloaded contents

[connorjclark]

This PR sets package_uuid to be the sha256 hash of the contents of the preloaded data file, if applicable. This avoids unnecessary cache misses–currently a new is generated per build, which invalidates the cached value in idb even if nothing changed in the data file.

The hash is generated based on the contents of the preloaded datafile post-compression, if applicable.

In #11952 it was suggested to make this be configurable–however, I don't think there's any reason to want this id to be unstable. At least, I can't think of one.

It's a bit weird that package_uuid is only sometimes actually an uuid. Another approach would be to keep package_uuid the same in the metadata, introduce a new content_hash field, and use this for the idb cache key: metadata['content_hash'] || metadata['package_uuid'] (content_hash only being set when the data is preloaded). Is it important to retain current behavior of the metadata's package_uuid property?

Fixes #11952

[kripken]

I don't remember this code well, but after checking the hash do we also check the contents for equality? I think we don't, since the point is to avoid fetching the contents, but I could be wrong.

If we don't check for equality, then there is some risk here. SHA256 collisions are in theory extremely unlikely, so maybe we should just ignore them. But a collision could lead to a very bad experience that is very hard to figure out, which is a little worrying.

Note that you can run the file packager separately from emcc, so you can run it only when your content actually changes, to avoid creating a new uuid each time.

[connorjclark]

You really should give zero weight to the possibility of a hash collision. What does actually happen is network requests failing for random reasons. This patch would reduce that by making network requests happen less often.

[sbc100]

The codesize tests failures should be fixed with a rebase or merge with main.

[sbc100]

I don't think the field name matters too much. lgtm either way.

[kripken]

You really should give zero weight to the possibility of a hash collision

Fair point, but as mentioned there, "Of course, all of the above assumes that SHA-256 is a "perfect" hash function, which is far from being proven. Still, SHA-256 seems quite robust." In practice, you are right - network issues happen far more often - but depending on this troubles me. If I'm the only one, though, then I won't block this.

[sbc100]

I think depending on SHA-256 for this purpose is totally fine. Caching data based on its hash seems perfectly reasonable.

[connorjclark]

Anything else to change on my end?

[sbc100]

this lgtm. @kripken ?

[kripken]

lgtm, since it seems like I'm the only one that slightly prefers the old way.

[kripken]

Before landing, please add a comment to the changelog file, as this is a behavior change that in theory might be noticed.

[connorjclark]

Sorry for delay, should be good now

[kripken]

I still don't see a change to the Changelog file? Perhaps a commit wasn't pushed?

[connorjclark]

Oops, I messed up the git command. pushed it now.