Insert "--head" defenses into the binary cache and block "default" wh…

…en calculating feature ABIs. (#336)

* Insert defenses into BinaryCache as reasonable and add tests.

<expected.h>: Add the missing T& overloads used elsewhere.

<binarycaching.h>:
* Remove VcpkgPaths from the IBinaryProvider interface; add it as a member of the concrete implementations.
* Hoist BinaryConfigParserState (formerly State in binarycaching.cpp) here. It would be nice to make this more shiny, but hiding the definition of this struct wasn't buying us too much, and being unable to test stuff was a worse evil.
* create_binary_providers_from_configs_pure now returns ExpectedS<BinaryConfigParserState>; this lets the tests of the parser machinery continue to be unchanged while still moving paths in as a member.

binarycaching.cpp:
* Push vcpkgpaths as a member into the concrete implementations.
* Change/audit all IBinaryProvider::prefetch to support nullptr. We pass in the underlying action plan vector here, and preparing a vector with the ABI-less packages removed would require extra indirection or flattening of the data we don't want to do in a bugfix.
* Change/audit all IBinaryProvider::precheck to not support nullptr. That is used only by CI and everything there should have a package ABI.
* Change BinaryCache::prefetch to explicitly document that nullptr package_abi is acceptable.
* Change BinaryCache::precheck to explicitly error out on nullptr package_abi.

vcpkg-test/binarycaching.cpp:
* Add tests for nullptr package ABIs passed to BinaryCache.

* Block "default" in package ABI as it indicates that the default features have not yet been resolved.

* Enforce that "core" is included before calculating ABI hashes.

Co-authored-by: Robert Schumacher <[email protected]>

include/vcpkg/base/expected.h

@@ -132,12 +132,24 @@ namespace vcpkg
         explicit constexpr operator bool() const noexcept { return !m_s.has_error(); }
         constexpr bool has_value() const noexcept { return !m_s.has_error(); }
 
+        const T&& value_or_exit(const LineInfo& line_info) const&&
+        {
+            exit_if_error(line_info);
+            return std::move(*this->m_t.get());
+        }
+
         T&& value_or_exit(const LineInfo& line_info) &&
         {
             exit_if_error(line_info);
             return std::move(*this->m_t.get());
         }
 
+        T& value_or_exit(const LineInfo& line_info) &
+        {
+            exit_if_error(line_info);
+            return *this->m_t.get();
+        }
+
         const T& value_or_exit(const LineInfo& line_info) const&
         {
             exit_if_error(line_info);

include/vcpkg/binarycaching.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <vcpkg/fwd/binarycaching.h>
 #include <vcpkg/fwd/dependencies.h>
 #include <vcpkg/fwd/vcpkgpaths.h>
 
@@ -16,27 +17,6 @@
 
 namespace vcpkg
 {
-    enum class RestoreResult
-    {
-        unavailable,
-        restored,
-    };
-
-    enum class CacheAvailability
-    {
-        unavailable,
-        available,
-    };
-
-    enum class CacheStatusState
-    {
-        unknown,   // the cache status of the indicated package ABI is unknown
-        available, // the cache is known to contain the package ABI, but it has not been restored
-        restored,  // the cache contains the ABI and it has been restored to the packages tree
-    };
-
-    struct IBinaryProvider;
-
     struct CacheStatus
     {
         bool should_attempt_precheck(const IBinaryProvider* sender) const noexcept;
@@ -67,57 +47,84 @@ namespace vcpkg
 
         /// Attempts to restore the package referenced by `action` into the packages directory.
         /// Prerequisite: action has a package_abi()
-        virtual RestoreResult try_restore(const VcpkgPaths& paths,
-                                          const Dependencies::InstallPlanAction& action) const = 0;
+        virtual RestoreResult try_restore(const Dependencies::InstallPlanAction& action) const = 0;
 
         /// Called upon a successful build of `action` to store those contents in the binary cache.
         /// Prerequisite: action has a package_abi()
-        virtual void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) const = 0;
+        virtual void push_success(const Dependencies::InstallPlanAction& action) const = 0;
 
         /// Gives the IBinaryProvider an opportunity to batch any downloading or server communication for
         /// executing `actions`.
         /// `cache_status` is a vector with the same number of entries of actions, where each index corresponds
         /// to the action at the same index in `actions`. The provider must mark the cache status as appropriate.
-        virtual void prefetch(const VcpkgPaths& paths,
-                              View<Dependencies::InstallPlanAction> actions,
-                              View<CacheStatus*> cache_status) const = 0;
+        /// Note: `actions` *might not* have package ABIs (for example, a --head package)!
+        /// Prerequisite: if `actions[i]` has no package ABI, `cache_status[i]` is nullptr.
+        virtual void prefetch(View<Dependencies::InstallPlanAction> actions, View<CacheStatus*> cache_status) const = 0;
 
         /// Checks whether the `actions` are present in the cache, without restoring them. Used by CI to determine
         /// missing packages.
         /// `cache_status` is a view with the same number of entries of actions, where each index corresponds
         /// to the action at the same index in `actions`. The provider must mark the cache status as appropriate.
-        virtual void precheck(const VcpkgPaths& paths,
-                              View<Dependencies::InstallPlanAction> actions,
-                              View<CacheStatus*> cache_status) const = 0;
+        /// Prerequisite: `actions` have package ABIs.
+        virtual void precheck(View<Dependencies::InstallPlanAction> actions, View<CacheStatus*> cache_status) const = 0;
+    };
+
+    struct BinaryConfigParserState
+    {
+        bool m_cleared = false;
+        bool interactive = false;
+        std::string nugettimeout = "100";
+
+        std::vector<Path> archives_to_read;
+        std::vector<Path> archives_to_write;
+
+        std::vector<std::string> url_templates_to_get;
+        std::vector<std::string> azblob_templates_to_put;
+
+        std::vector<std::string> gcs_read_prefixes;
+        std::vector<std::string> gcs_write_prefixes;
+
+        std::vector<std::string> aws_read_prefixes;
+        std::vector<std::string> aws_write_prefixes;
+
+        std::vector<std::string> sources_to_read;
+        std::vector<std::string> sources_to_write;
+
+        std::vector<Path> configs_to_read;
+        std::vector<Path> configs_to_write;
+
+        std::vector<std::string> secrets;
+
+        void clear();
     };
 
+    ExpectedS<BinaryConfigParserState> create_binary_providers_from_configs_pure(const std::string& env_string,
+                                                                                 View<std::string> args);
     ExpectedS<std::vector<std::unique_ptr<IBinaryProvider>>> create_binary_providers_from_configs(
-        View<std::string> args);
-    ExpectedS<std::vector<std::unique_ptr<IBinaryProvider>>> create_binary_providers_from_configs_pure(
-        const std::string& env_string, View<std::string> args);
+        const VcpkgPaths& paths, View<std::string> args);
 
     struct BinaryCache
     {
         BinaryCache() = default;
-        explicit BinaryCache(const VcpkgCmdArguments& args);
+        explicit BinaryCache(const VcpkgCmdArguments& args, const VcpkgPaths& paths);
 
         void install_providers(std::vector<std::unique_ptr<IBinaryProvider>>&& providers);
-        void install_providers_for(const VcpkgCmdArguments& args);
+        void install_providers_for(const VcpkgCmdArguments& args, const VcpkgPaths& paths);
 
         /// Attempts to restore the package referenced by `action` into the packages directory.
-        RestoreResult try_restore(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action);
+        RestoreResult try_restore(const Dependencies::InstallPlanAction& action);
 
         /// Called upon a successful build of `action` to store those contents in the binary cache.
-        void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action);
+        void push_success(const Dependencies::InstallPlanAction& action);
 
         /// Gives the IBinaryProvider an opportunity to batch any downloading or server communication for
         /// executing `actions`.
-        void prefetch(const VcpkgPaths& paths, View<Dependencies::InstallPlanAction> actions);
+        void prefetch(View<Dependencies::InstallPlanAction> actions);
 
         /// Checks whether the `actions` are present in the cache, without restoring them. Used by CI to determine
         /// missing packages.
         /// Returns a vector where each index corresponds to the matching index in `actions`.
-        std::vector<CacheAvailability> precheck(const VcpkgPaths& paths, View<Dependencies::InstallPlanAction> actions);
+        std::vector<CacheAvailability> precheck(View<Dependencies::InstallPlanAction> actions);
 
     private:
         std::unordered_map<std::string, CacheStatus> m_status;

include/vcpkg/fwd/binarycaching.h

@@ -0,0 +1,28 @@
+#pragma once
+
+namespace vcpkg
+{
+    enum class RestoreResult
+    {
+        unavailable,
+        restored,
+    };
+
+    enum class CacheAvailability
+    {
+        unavailable,
+        available,
+    };
+
+    enum class CacheStatusState
+    {
+        unknown,   // the cache status of the indicated package ABI is unknown
+        available, // the cache is known to contain the package ABI, but it has not been restored
+        restored,  // the cache contains the ABI and it has been restored to the packages tree
+    };
+
+    struct CacheStatus;
+    struct IBinaryProvider;
+    struct BinaryCache;
+    struct BinaryConfigParserState;
+}

src/vcpkg-test/binarycaching.cpp

@@ -18,27 +18,34 @@ using namespace vcpkg;
 
 struct KnowNothingBinaryProvider : IBinaryProvider
 {
-    RestoreResult try_restore(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override
+    RestoreResult try_restore(const Dependencies::InstallPlanAction& action) const override
     {
+        CHECK(action.has_package_abi());
         return RestoreResult::unavailable;
     }
 
-    virtual void push_success(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override { }
-    virtual void prefetch(const VcpkgPaths&,
-                          View<Dependencies::InstallPlanAction>,
-                          View<CacheStatus* const>) const override
+    virtual void push_success(const Dependencies::InstallPlanAction& action) const override
     {
+        CHECK(action.has_package_abi());
     }
-    virtual void precheck(const VcpkgPaths&,
-                          View<Dependencies::InstallPlanAction>,
+
+    virtual void prefetch(View<Dependencies::InstallPlanAction> actions,
                           View<CacheStatus* const> cache_status) const override
     {
+        REQUIRE(actions.size() == cache_status.size());
+        for (size_t idx = 0; idx < cache_status.size(); ++idx)
+        {
+            CHECK(actions[idx].has_package_abi() == (cache_status[idx] != nullptr));
+        }
+    }
+    virtual void precheck(View<Dependencies::InstallPlanAction> actions,
+                          View<CacheStatus* const> cache_status) const override
+    {
+        REQUIRE(actions.size() == cache_status.size());
         for (const auto c : cache_status)
         {
-            if (c)
-            {
-                c->mark_unavailable(this);
-            }
+            CHECK(c);
+            c->mark_unavailable(this);
         }
     }
 };
@@ -359,6 +366,39 @@ Features: a, b
     }
 }
 
+TEST_CASE ("Provider nullptr checks", "[BinaryCache]")
+{
+    // create a binary cache to test
+    BinaryCache uut;
+    std::vector<std::unique_ptr<IBinaryProvider>> providers;
+    providers.emplace_back(std::make_unique<KnowNothingBinaryProvider>());
+    uut.install_providers(std::move(providers));
+
+    // create an action plan with an action without a package ABI set
+    auto pghs = Paragraphs::parse_paragraphs(R"(
+Source: someheadpackage
+Version: 1.5
+Description: 
+)",
+                                             "<testdata>");
+    REQUIRE(pghs.has_value());
+    auto maybe_scf = SourceControlFile::parse_control_file("", std::move(*pghs.get()));
+    REQUIRE(maybe_scf.has_value());
+    SourceControlFileAndLocation scfl{std::move(*maybe_scf.get()), Path()};
+    std::vector<Dependencies::InstallPlanAction> install_plan;
+    install_plan.emplace_back(PackageSpec{"someheadpackage", Test::X64_WINDOWS},
+                              scfl,
+                              Dependencies::RequestType::USER_REQUESTED,
+                              Test::ARM_UWP,
+                              std::map<std::string, std::vector<FeatureSpec>>{});
+    Dependencies::InstallPlanAction& ipa_without_abi = install_plan.back();
+
+    // test that the binary cache does the right thing. See also CHECKs etc. in KnowNothingBinaryProvider
+    uut.push_success(ipa_without_abi); // should have no effects
+    CHECK(uut.try_restore(ipa_without_abi) == RestoreResult::unavailable);
+    uut.prefetch(install_plan); // should have no effects
+}
+
 TEST_CASE ("XmlSerializer", "[XmlSerializer]")
 {
     XmlSerializer xml;