Fix search-by-name commands broken by wildcard packages feature. (#1147)

* get_all_port_names => append_all_port_names

* Extract get_all_reachable_port_names.

* Fix redundant equal compare in package_match_prefix when the prefix is a wildcard.

* Fix handling of wildcards when getting all reachable names.

* Teach autocomplete to do its work without hitting the network.

* Add unit test.

include/vcpkg/registries.h

@@ -79,7 +79,12 @@ namespace vcpkg
 
         // appends the names of the ports to the out parameter
         // may result in duplicated port names; make sure to Util::sort_unique_erase at the end
-        virtual void get_all_port_names(std::vector<std::string>& port_names) const = 0;
+        virtual void append_all_port_names(std::vector<std::string>& port_names) const = 0;
+
+        // appends the names of the ports to the out parameter if this can be known without
+        // network access.
+        // returns true if names were appended, otherwise returns false.
+        virtual bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const = 0;
 
         virtual ExpectedL<Version> get_baseline_version(StringView port_name) const = 0;
 
@@ -89,18 +94,16 @@ namespace vcpkg
     struct Registry
     {
         // requires: static_cast<bool>(implementation)
-        Registry(std::vector<std::string>&& packages, std::unique_ptr<RegistryImplementation>&& implementation);
+        Registry(std::vector<std::string>&& patterns, std::unique_ptr<RegistryImplementation>&& implementation);
 
         Registry(std::vector<std::string>&&, std::nullptr_t) = delete;
 
-        // always ordered lexicographically
-        View<std::string> packages() const { return packages_; }
+        // always ordered lexicographically; note the JSON name is "packages"
+        View<std::string> patterns() const { return patterns_; }
         const RegistryImplementation& implementation() const { return *implementation_; }
 
-        friend RegistrySet; // for experimental_set_builtin_registry_baseline
-
     private:
-        std::vector<std::string> packages_;
+        std::vector<std::string> patterns_;
         std::unique_ptr<RegistryImplementation> implementation_;
     };
 
@@ -112,8 +115,8 @@ namespace vcpkg
     // configuration fields.
     struct RegistrySet
     {
-        RegistrySet(std::unique_ptr<RegistryImplementation>&& x, std::vector<Registry>&& y)
-            : default_registry_(std::move(x)), registries_(std::move(y))
+        RegistrySet(std::unique_ptr<RegistryImplementation>&& default_registry, std::vector<Registry>&& registries)
+            : default_registry_(std::move(default_registry)), registries_(std::move(registries))
         {
         }
 
@@ -138,6 +141,12 @@ namespace vcpkg
         // for checking against the registry feature flag.
         bool has_modifications() const;
 
+        // Returns a sorted vector of all reachable port names in this set.
+        std::vector<std::string> get_all_reachable_port_names() const;
+
+        // Returns a sorted vector of all reachable port names we can provably determine without touching the network.
+        std::vector<std::string> get_all_known_reachable_port_names_no_network() const;
+
     private:
         std::unique_ptr<RegistryImplementation> default_registry_;
         std::vector<Registry> registries_;
@@ -159,5 +168,9 @@ namespace vcpkg
     ExpectedL<std::map<std::string, Version, std::less<>>> get_builtin_baseline(const VcpkgPaths& paths);
 
     bool is_git_commit_sha(StringView sv);
-    size_t package_match_prefix(StringView name, StringView pattern);
+
+    // Returns the effective match length of the package pattern `pattern` against `name`.
+    // No match is 0, exact match is SIZE_MAX, wildcard match is the length of the pattern.
+    // Note that the * is included in the match size to distinguish from 0 == no match.
+    size_t package_pattern_match(StringView name, StringView pattern);
 }

src/vcpkg-test/registries.cpp

@@ -17,7 +17,16 @@ namespace
 
         std::unique_ptr<RegistryEntry> get_port_entry(StringView) const override { return nullptr; }
 
-        void get_all_port_names(std::vector<std::string>&) const override { }
+        void append_all_port_names(std::vector<std::string>& port_names) const override
+        {
+            port_names.insert(port_names.end(), all_port_names.begin(), all_port_names.end());
+        }
+
+        bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const override
+        {
+            port_names.insert(port_names.end(), no_network_port_names.begin(), no_network_port_names.end());
+            return !no_network_port_names.empty();
+        }
 
         ExpectedL<Version> get_baseline_version(StringView) const override
         {
@@ -25,13 +34,30 @@ namespace
         }
 
         int number;
+        std::vector<std::string> all_port_names;
+        std::vector<std::string> no_network_port_names;
 
-        TestRegistryImplementation(int n) : number(n) { }
+        TestRegistryImplementation(int n) : number(n), all_port_names(), no_network_port_names() { }
+        TestRegistryImplementation(int n,
+                                   std::vector<std::string>&& all_port_names,
+                                   std::vector<std::string>&& no_network_port_names)
+            : number(n), all_port_names(all_port_names), no_network_port_names(no_network_port_names)
+        {
+        }
     };
 
-    Registry make_registry(int n, std::vector<std::string>&& port_names)
+    Registry make_registry(int n, std::vector<std::string>&& patterns)
     {
-        return {std::move(port_names), std::make_unique<TestRegistryImplementation>(n)};
+        return {std::move(patterns), std::make_unique<TestRegistryImplementation>(n)};
+    }
+
+    Registry make_registry(int n,
+                           std::vector<std::string>&& patterns,
+                           std::vector<std::string>&& known_no_network,
+                           std::vector<std::string>&& known_network)
+    {
+        return {std::move(patterns),
+                std::make_unique<TestRegistryImplementation>(n, std::move(known_no_network), std::move(known_network))};
     }
 
     int get_tri_num(const RegistryImplementation& r)
@@ -138,19 +164,19 @@ TEST_CASE ("check valid package patterns", "[registries]")
 
 TEST_CASE ("calculate prefix priority", "[registries]")
 {
-    CHECK(package_match_prefix("boost", "*") == 1);
-    CHECK(package_match_prefix("boost", "b*") == 2);
-    CHECK(package_match_prefix("boost", "boost*") == 6);
-    CHECK(package_match_prefix("boost", "boost") == SIZE_MAX);
+    CHECK(package_pattern_match("boost", "*") == 1);
+    CHECK(package_pattern_match("boost", "b*") == 2);
+    CHECK(package_pattern_match("boost", "boost*") == 6);
+    CHECK(package_pattern_match("boost", "boost") == SIZE_MAX);
 
-    CHECK(package_match_prefix("", "") == SIZE_MAX);
-    CHECK(package_match_prefix("", "*") == 1);
-    CHECK(package_match_prefix("", "a") == 0);
-    CHECK(package_match_prefix("boost", "") == 0);
-    CHECK(package_match_prefix("boost", "c*") == 0);
-    CHECK(package_match_prefix("boost", "*c") == 0);
-    CHECK(package_match_prefix("boost", "c**") == 0);
-    CHECK(package_match_prefix("boost", "c*a") == 0);
+    CHECK(package_pattern_match("", "") == SIZE_MAX);
+    CHECK(package_pattern_match("", "*") == 1);
+    CHECK(package_pattern_match("", "a") == 0);
+    CHECK(package_pattern_match("boost", "") == 0);
+    CHECK(package_pattern_match("boost", "c*") == 0);
+    CHECK(package_pattern_match("boost", "*c") == 0);
+    CHECK(package_pattern_match("boost", "c**") == 0);
+    CHECK(package_pattern_match("boost", "c*a") == 0);
 }
 
 TEST_CASE ("select highest priority registry", "[registries]")
@@ -713,3 +739,57 @@ TEST_CASE ("filesystem_version_db_parsing", "[registries]")
         CHECK(!r.errors().empty());
     }
 }
+
+TEST_CASE ("get_all_port_names", "[registries]")
+{
+    std::vector<Registry> registries;
+    // no network 0 known ports, unrelated and example are not selected
+    registries.emplace_back(make_registry(1,
+                                          {"hello", "world", "abc*", "notpresent"},
+                                          {"hello", "world", "unrelated", "example", "abcdefg", "abc", "abcde"},
+                                          {}));
+    // no network has some known ports
+    registries.emplace_back(
+        make_registry(2,
+                      {"two*"},
+                      {"hello", "world", "unrelated", "twoRegistry", "abcdefgXXX", "abcXXX", "abcdeXXX"},
+                      {"old", "ports", "abcdefgsuper", "twoOld"}));
+
+    SECTION ("with default registry")
+    {
+        RegistrySet with_default_registry{std::make_unique<TestRegistryImplementation>(
+                                              1,
+                                              std::vector<std::string>{"aDefault", "bDefault", "cDefault"},
+                                              std::vector<std::string>{"aDefaultOld", "bDefaultOld", "cDefaultOld"}),
+                                          std::move(registries)};
+
+        // All the known ports from the default registry
+        // hello, world, abcdefg, abc, abcde from the first registry
+        // twoRegistry from the second registry
+        CHECK(with_default_registry.get_all_reachable_port_names() ==
+              std::vector<std::string>{
+                  "aDefault", "abc", "abcde", "abcdefg", "bDefault", "cDefault", "hello", "twoRegistry", "world"});
+
+        // All the old ports from the default registry
+        // hello, world, notpresent from the first registry (since network was unknown)
+        // twoOld from the second registry
+        CHECK(with_default_registry.get_all_known_reachable_port_names_no_network() ==
+              std::vector<std::string>{
+                  "aDefaultOld", "bDefaultOld", "cDefaultOld", "hello", "notpresent", "twoOld", "world"});
+    }
+
+    SECTION ("without default registry")
+    {
+        RegistrySet without_default_registry{nullptr, std::move(registries)};
+
+        // hello, world, abcdefg, abc, abcde from the first registry
+        // twoRegistry from the second registry
+        CHECK(without_default_registry.get_all_reachable_port_names() ==
+              std::vector<std::string>{"abc", "abcde", "abcdefg", "hello", "twoRegistry", "world"});
+
+        // hello, world, notpresent from the first registry
+        // twoOld from the second registry
+        CHECK(without_default_registry.get_all_known_reachable_port_names_no_network() ==
+              std::vector<std::string>{"hello", "notpresent", "twoOld", "world"});
+    }
+}

src/vcpkg/commands.install.cpp

@@ -648,23 +648,9 @@ namespace vcpkg
         {OPTION_MANIFEST_FEATURE, []() { return msg::format(msgHelpTxtOptManifestFeature); }},
     }};
 
-    static std::vector<std::string> get_all_port_names(const VcpkgPaths& paths)
+    static std::vector<std::string> get_all_known_reachable_port_names_no_network(const VcpkgPaths& paths)
     {
-        const auto registries = paths.make_registry_set();
-
-        std::vector<std::string> ret;
-        for (const auto& registry : registries->registries())
-        {
-            const auto packages = registry.packages();
-            ret.insert(ret.end(), packages.begin(), packages.end());
-        }
-        if (auto registry = registries->default_registry())
-        {
-            registry->get_all_port_names(ret);
-        }
-
-        Util::sort_unique_erase(ret);
-        return ret;
+        return paths.make_registry_set()->get_all_known_reachable_port_names_no_network();
     }
 
     const CommandStructure Install::COMMAND_STRUCTURE = {
@@ -672,7 +658,7 @@ namespace vcpkg
         0,
         SIZE_MAX,
         {INSTALL_SWITCHES, INSTALL_SETTINGS, INSTALL_MULTISETTINGS},
-        &get_all_port_names,
+        &get_all_known_reachable_port_names_no_network,
     };
 
     // This command structure must share "critical" values (switches, number of arguments). It exists only to provide a

src/vcpkg/paragraphs.cpp

@@ -505,21 +505,7 @@ namespace vcpkg::Paragraphs
     LoadResults try_load_all_registry_ports(const ReadOnlyFilesystem& fs, const RegistrySet& registries)
     {
         LoadResults ret;
-
-        std::vector<std::string> ports;
-
-        for (const auto& registry : registries.registries())
-        {
-            const auto packages = registry.packages();
-            ports.insert(end(ports), begin(packages), end(packages));
-        }
-        if (auto registry = registries.default_registry())
-        {
-            registry->get_all_port_names(ports);
-        }
-
-        Util::sort_unique_erase(ports);
-
+        std::vector<std::string> ports = registries.get_all_reachable_port_names();
         for (const auto& port_name : ports)
         {
             auto impl = registries.registry_for_port(port_name);

src/vcpkg/registries.cpp

@@ -18,7 +18,11 @@
 #include <vcpkg/versiondeserializers.h>
 #include <vcpkg/versions.h>
 
+#include <algorithm>
+#include <iterator>
 #include <map>
+#include <string>
+#include <vector>
 
 namespace
 {
@@ -212,7 +216,9 @@ namespace
 
         std::unique_ptr<RegistryEntry> get_port_entry(StringView) const override;
 
-        void get_all_port_names(std::vector<std::string>&) const override;
+        void append_all_port_names(std::vector<std::string>&) const override;
+
+        bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const override;
 
         ExpectedL<Version> get_baseline_version(StringView) const override;
 
@@ -370,7 +376,9 @@ namespace
 
         std::unique_ptr<RegistryEntry> get_port_entry(StringView port_name) const override;
 
-        void get_all_port_names(std::vector<std::string>&) const override;
+        void append_all_port_names(std::vector<std::string>&) const override;
+
+        bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const override;
 
         ExpectedL<Version> get_baseline_version(StringView port_name) const override;
 
@@ -407,7 +415,9 @@ namespace
 
         std::unique_ptr<RegistryEntry> get_port_entry(StringView port_name) const override;
 
-        void get_all_port_names(std::vector<std::string>&) const override;
+        void append_all_port_names(std::vector<std::string>&) const override;
+
+        bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const override;
 
         ExpectedL<Version> get_baseline_version(StringView port_name) const override;
 
@@ -436,7 +446,12 @@ namespace
             Checks::msg_exit_with_error(VCPKG_LINE_INFO, msgErrorRequireBaseline);
         }
 
-        void get_all_port_names(std::vector<std::string>&) const override
+        void append_all_port_names(std::vector<std::string>&) const override
+        {
+            Checks::msg_exit_with_error(VCPKG_LINE_INFO, msgErrorRequireBaseline);
+        }
+
+        bool try_append_all_port_names_no_network(std::vector<std::string>&) const override
         {
             Checks::msg_exit_with_error(VCPKG_LINE_INFO, msgErrorRequireBaseline);
         }
@@ -461,7 +476,9 @@ namespace
 
         std::unique_ptr<RegistryEntry> get_port_entry(StringView) const override;
 
-        void get_all_port_names(std::vector<std::string>&) const override;
+        void append_all_port_names(std::vector<std::string>&) const override;
+
+        bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const override;
 
         ExpectedL<Version> get_baseline_version(StringView) const override;
 
@@ -610,7 +627,7 @@ namespace
         return LocalizedString::from_raw(ParseControlErrorInfo::format_errors({&maybe_scf.error(), 1}));
     }
 
-    void BuiltinFilesRegistry::get_all_port_names(std::vector<std::string>& out) const
+    void BuiltinFilesRegistry::append_all_port_names(std::vector<std::string>& out) const
     {
         std::error_code ec;
         auto port_directories = m_fs.get_directories_non_recursive(m_builtin_ports_directory, VCPKG_LINE_INFO);
@@ -622,6 +639,12 @@ namespace
             out.push_back(filename.to_string());
         }
     }
+
+    bool BuiltinFilesRegistry::try_append_all_port_names_no_network(std::vector<std::string>& port_names) const
+    {
+        append_all_port_names(port_names);
+        return true;
+    }
     // } BuiltinFilesRegistry::RegistryImplementation
 
     // { BuiltinGitRegistry::RegistryImplementation
@@ -680,7 +703,7 @@ namespace
         return msg::format(msg::msgErrorMessage).append(msgPortNotInBaseline, msg::package_name = port_name);
     }
 
-    void BuiltinGitRegistry::get_all_port_names(std::vector<std::string>& out) const
+    void BuiltinGitRegistry::append_all_port_names(std::vector<std::string>& out) const
     {
         const auto& fs = m_paths.get_filesystem();
 
@@ -689,7 +712,13 @@ namespace
             load_all_port_names_from_registry_versions(out, fs, m_paths.builtin_registry_versions);
         }
 
-        m_files_impl->get_all_port_names(out);
+        m_files_impl->append_all_port_names(out);
+    }
+
+    bool BuiltinGitRegistry::try_append_all_port_names_no_network(std::vector<std::string>& port_names) const
+    {
+        append_all_port_names(port_names);
+        return true;
     }
     // } BuiltinGitRegistry::RegistryImplementation
 
@@ -747,10 +776,16 @@ namespace
         return res;
     }
 
-    void FilesystemRegistry::get_all_port_names(std::vector<std::string>& out) const
+    void FilesystemRegistry::append_all_port_names(std::vector<std::string>& out) const
     {
         load_all_port_names_from_registry_versions(out, m_fs, m_path / registry_versions_dir_name);
     }
+
+    bool FilesystemRegistry::try_append_all_port_names_no_network(std::vector<std::string>& port_names) const
+    {
+        append_all_port_names(port_names);
+        return true;
+    }
     // } FilesystemRegistry::RegistryImplementation
 
     // { GitRegistry::RegistryImplementation
@@ -860,11 +895,20 @@ namespace
         return msg::format(msg::msgErrorMessage).append(msgPortNotInBaseline, msg::package_name = port_name);
     }
 
-    void GitRegistry::get_all_port_names(std::vector<std::string>& out) const
+    void GitRegistry::append_all_port_names(std::vector<std::string>& out) const
     {
         auto versions_path = get_stale_versions_tree_path();
         load_all_port_names_from_registry_versions(out, m_paths.get_filesystem(), versions_path.p);
     }
+
+    bool GitRegistry::try_append_all_port_names_no_network(std::vector<std::string>&) const
+    {
+        // At this time we don't record enough information to know what the last fetch for a registry is,
+        // so we can't even return what the most recent answer was.
+        //
+        // This would be fixable if we recorded LockFile in the registries cache.
+        return false;
+    }
     // } GitRegistry::RegistryImplementation
 
     // } RegistryImplementation
@@ -1168,10 +1212,10 @@ namespace vcpkg
         }
     }
 
-    Registry::Registry(std::vector<std::string>&& packages, std::unique_ptr<RegistryImplementation>&& impl)
-        : packages_(std::move(packages)), implementation_(std::move(impl))
+    Registry::Registry(std::vector<std::string>&& patterns, std::unique_ptr<RegistryImplementation>&& impl)
+        : patterns_(std::move(patterns)), implementation_(std::move(impl))
     {
-        Util::sort_unique_erase(packages_);
+        Util::sort_unique_erase(patterns_);
         Checks::check_exit(VCPKG_LINE_INFO, implementation_ != nullptr);
     }
 
@@ -1186,26 +1230,24 @@ namespace vcpkg
         return candidates[0];
     }
 
-    size_t package_match_prefix(StringView name, StringView prefix)
+    size_t package_pattern_match(StringView name, StringView pattern)
     {
-        if (name == prefix)
+        const auto pattern_size = pattern.size();
+        const auto maybe_star_index = pattern_size - 1;
+        if (pattern_size != 0 && pattern[maybe_star_index] == '*')
+        {
+            // pattern ends in wildcard
+            if (name.size() >= maybe_star_index && std::equal(pattern.begin(), pattern.end() - 1, name.begin()))
+            {
+                return pattern_size;
+            }
+        }
+        else if (name == pattern)
         {
             // exact match is like matching "infinity" prefix
             return SIZE_MAX;
         }
 
-        // Note that the * is included in the match so that 0 means no match
-        const auto prefix_size = prefix.size();
-        if (prefix_size != 0)
-        {
-            const auto star_index = prefix_size - 1;
-            if (prefix[star_index] == '*' && name.size() >= star_index &&
-                name.substr(0, star_index) == prefix.substr(0, star_index))
-            {
-                return prefix_size;
-            }
-        }
-
         return 0;
     }
 
@@ -1221,9 +1263,9 @@ namespace vcpkg
         for (auto&& registry : registries())
         {
             std::size_t longest_prefix = 0;
-            for (auto&& package : registry.packages())
+            for (auto&& pattern : registry.patterns())
             {
-                longest_prefix = std::max(longest_prefix, package_match_prefix(name, package));
+                longest_prefix = std::max(longest_prefix, package_pattern_match(name, pattern));
             }
 
             if (longest_prefix != 0)
@@ -1257,6 +1299,80 @@ namespace vcpkg
         return default_registry_ && default_registry_->kind() == BuiltinFilesRegistry::s_kind;
     }
     bool RegistrySet::has_modifications() const { return !registries_.empty() || !is_default_builtin_registry(); }
+} // namespace vcpkg
+
+namespace
+{
+    void remove_unreachable_port_names_by_patterns(std::vector<std::string>& result,
+                                                   std::size_t start_at,
+                                                   View<std::string> patterns)
+    {
+        // Remove names in result[start_at .. end] which no package pattern matches
+        result.erase(std::remove_if(result.begin() + start_at,
+                                    result.end(),
+                                    [&](const std::string& name) {
+                                        return std::none_of(
+                                            patterns.begin(), patterns.end(), [&](const std::string& pattern) {
+                                                return package_pattern_match(name, pattern) != 0;
+                                            });
+                                    }),
+                     result.end());
+    }
+} // unnamed namespace
+
+namespace vcpkg
+{
+    std::vector<std::string> RegistrySet::get_all_reachable_port_names() const
+    {
+        std::vector<std::string> result;
+        for (const auto& registry : registries())
+        {
+            const auto start_at = result.size();
+            registry.implementation().append_all_port_names(result);
+            remove_unreachable_port_names_by_patterns(result, start_at, registry.patterns());
+        }
+
+        if (auto registry = default_registry())
+        {
+            registry->append_all_port_names(result);
+        }
+
+        Util::sort_unique_erase(result);
+        return result;
+    }
+
+    std::vector<std::string> RegistrySet::get_all_known_reachable_port_names_no_network() const
+    {
+        std::vector<std::string> result;
+        for (const auto& registry : registries())
+        {
+            const auto start_at = result.size();
+            const auto patterns = registry.patterns();
+            if (registry.implementation().try_append_all_port_names_no_network(result))
+            {
+                remove_unreachable_port_names_by_patterns(result, start_at, patterns);
+            }
+            else
+            {
+                // we don't know all names, but we can at least assume the exact match patterns
+                // will be names
+                std::remove_copy_if(patterns.begin(),
+                                    patterns.end(),
+                                    std::back_inserter(result),
+                                    [&](const std::string& package_pattern) -> bool {
+                                        return package_pattern.empty() || package_pattern.back() == '*';
+                                    });
+            }
+        }
+
+        if (auto registry = default_registry())
+        {
+            (void)registry->try_append_all_port_names_no_network(result);
+        }
+
+        Util::sort_unique_erase(result);
+        return result;
+    }
 
     ExpectedL<std::vector<std::pair<SchemedVersion, std::string>>> get_builtin_versions(const VcpkgPaths& paths,
                                                                                         StringView port_name)