Split BinaryProvider into read vs write

[ras0219-msft]

This is a rewrite of the Binary Caching infrastructure to improve code sharing and reduce coupling.

This will make #908 more tractable because there will be significantly fewer lines to audit.

[autoantwort]

In the past a provider was marked available if the provider pushed a specific package. This was removed here (expand diff required to see the line), resp. in #908 because the call was not thread safe and there is currently no situation were this is needed. It also could be kind of ambiguous because a provider can have multiple destinations and we don't know which destination has a package available.

With #802 the "push_success -> mark_restored flow" will become relevant for the first time because we build packages, will remove them and then maybe need them again. If we don't want to rebuild the package although it is in the cache, we have to mark it as available or remove the unavailable status. In #911 there is now one provider for every destination so that we don't lost the connection between the push and pull providers and are able to mark providers as available for a specific package.

[ras0219-msft]

It's a really good point that we need to invalidate the precheck status of any packages that were pushed -- otherwise we would refuse to try to download them.

However, we SHOULD be running .precheck() on every plan -- which will then correctly re-populate status information. I don't think coupling readers to writers really does what we want, because nothing says that pushing to one reader can't enable pulling from another. I think the correct fix (which you observed) is to completely invalidate the status upon push. Then, when we begin to execute a new plan, we run a .precheck() which will reconsider all read providers (in performance order).

[autoantwort]

However, we SHOULD be running .precheck() on every plan

This is currently already the case :)

I don't think coupling readers to writers really does what we want, because nothing says that pushing to one reader can't enable pulling from another.

That is true (but I think this only happens rarely), but at least for the destinations we pushed a package to, we know we can read it back from there. Invalidating also works and is needed for providers to which we don't have pushed a package, but for the other providers that means unnecessary work (but the overhead is probably negligible).

[BillyONeal]

Most of the CR comments are nits. A few non-nits:

• I'm weakly opposed to RemoveFilesystem.
• The comments describing the interface contracts.
• Localizing vendor().

[BillyONeal]

As an aside, elapsed may be bogus when localized?

[BillyONeal]

I think it's now possible to write tests for this, and that's probably worth it now that this function is doing complicated stuff to map between universes.

[BillyONeal]

LGTM as long as the files.associations thing that looks unintended (?) is fixed.

[BillyONeal]

I'm sorry, I tried https://github.com/microsoft/vcpkg-tool/assets/1544943/3b3f5fd8-254a-4991-ac1f-ab6bdca97eda

[ras0219-msft]

Items left unaddressed:

• ReadOnlyBinaryCache::precheck should be more testable now -- we should add more tests
• Elapsed times are suboptimally localized