Slight change in behavior appear here. We could put into our tooling a hash of all the files we're after if we want to allow for caching of the index file across versions (as it use to be). Or we could actually compute the hash (md5?) of the file we're after directly here.

We should restore old behavior. Perhaps:

• hash dir embed.FS on startup and store it somewhere?
  • we dont need to keep the entire hash, only ~7 characters will be more than enough.
  • fyi we use https://github.com/OneOfOne/xxhash in places where we need a fast non-cryptographic hash

I think this is a really interesting aspect anw why I highlighted it. Etag was actually debated as part of the embed implementation here

The way it was before this change was containing an "flaw" (we could actually file it as a separate issue) in that if there are two files foo.html and bar.html, either of the two files being changed would bust the cache of the other one too despite it being unchanged.

I have implemented it using sha256 in the following commit.

It has the same behavior as before, with a slight impact on startup (computing the hash). We can try to optimize it after with some function call, caching and sync.Once to avoid computing the hash until it is needed at a later stage. I believe the impact would be quite minimal given there are not many assets to hash and the efficiency of sha256 so I defer that to a separate concern. This also avoids bringing in another dependency (as xxxhash would).

Another option would also be to use something like https://github.com/benbjohnson/hashfs - this would allow a per-asset caching. (This definitely could be done as another pull-request).

Given the concern of each one, I though just busting the cache between version changes would be an okay solution.

Let me know what you think, happy to remove the commit or attempt another implementation

It has the same behavior as before, with a slight impact on startup (computing the hash). We can try to optimize it after with some function call, caching and sync.Once to avoid computing the hash until it is needed at a later stage.

That sounds like a good idea, however I think it's better to try the easy stuff first (using a cheaper hash function). And see about that later, probably that xxhash is fast enough anyway and that this isn't needed.

An other option similar to sync.Once is we could have init starting a go Hash.Do(hashfunc) so it is still done at init but asynchronously in the background. And then consumers would also try to do Hash.Do(hashfunc) and be stuck until the once return.

@Jorropo Not sure I fully understand the last part of your comment (though I get it's hoping to do that in a goroutine), but I think it's not that big of a bother anyhow as we both think that such an optimization can come later if it is needed and doesn't need to be part of this PR I think.

later if it is needed

I agree with that too, so this point is solved. 🙂

What I proposed, is something like this: (pseudo code)

func init() {
  go etagHashLock.Do(func() {
    etagHash = computeEtagHash()
  })
}

var etagHash string
var etagHashLock sync.Once

func GetETagHash() string {
  etagHashLock.Do(func(){}) // Here we do nothing, basically we just use the once as a way to wait on the goroutine that init started
  return etagHash
}

Ah - got it. Thank for the explanation

We should restore old behavior. Perhaps:

• hash dir embed.FS on startup and store it somewhere?
  • we dont need to keep the entire hash, only ~7 characters will be more than enough.
  • fyi we use https://github.com/OneOfOne/xxhash in places where we need a fast non-cryptographic hash