Do not mark cache generation as local

[DavidANeil]

local=True marks a repository rule to unconditionally re-execute anytime bazel re-evaluates the workspace (almost every invocation) It works well for processes that execute in <10ms, not large ones like the embuilder cache generation.

I have tested this locally and it seems to work fine, but I'll be honest that I don't understand the nuances of repository rules that well, nor do I know much of anything about emscripten or the circumstances in which the cache ought to be rebuilt.

I checked the external/@emscripten_cache.marker file that Bazel uses to store the cache key for if it needs to re-evaluate, and changing any of the options passed into register_emscripten_toolchains resulted in the cache key changing, and it correctly resulted in the cache being regenerated.

The big question I don't have the context to answer: are there files on disk other than these two that need to be watched for changes, so the repository rule should change if they change:

FILE:@@emscripten_bin_linux//BUILD.bazel
FILE:@@emsdk//emscripten_toolchain/default_config

[sbc100]

unconditionally re-execute anytime bazel re-evaluates the workspace

Does that mean the cache was being completely repopulated every time bazel was run? Is that what you are seeing happen?

This code was added in #1405. Perhaps the creator of that PR has some insight here.

[DavidANeil]

Does that mean the cache was being completely repopulated every time bazel was run? Is that what you are seeing happen?

Not necessarily every invocation, but every time the WORKSPACE checks that it is up-to-date (I believe this is called a "fetch" in WORKSPACE-land), which at least happens each time the analysis cache is discarded.
In practice that means I've been seeing it 4-5 times a day during regular development.
Without local = True, then the cache can be vendored by bazel into the cache, which lets it persist until one of the inputs changes.

For us it isn't too bad, because it only takes 10s to create the cache for the few things we need, but it is still annoying.

Good point, maybe @allsey87 knows more about what conditions we would want to regenerate the cache.

[sbc100]

Seems reasonable to me. lgtm if @allsey87 and @walkingeyerobot things this is good idea.

[walkingeyerobot]

This is a piece of bazel I have minimal experience with, but I'm happy to approve. We should give @allsey87 a few days to respond before merging though.

[allsey87]

This is a problem that I have also noticed so I am glad there is a solution to it! I am not sure why I added local in the first place, but perhaps I just misread the documentation. LGTM.