[RFC] make it possible to _not_ have a baseline for git registries

strega-nil · strega-nil · commit 05b70f548a3c · 2025-10-16T10:06:15.000+02:00
Internally to reMarkable, we want to not have a baseline for our
internal dependencies, and only use `"version&gt;="` constraints;
this current set of changes does work for our current workflow, but of
course we can make the code better; I just want to put it out there to
see if there's interest (and so we can maybe avoid a fork).
diff --git a/include/vcpkg/portfileprovider.h b/include/vcpkg/portfileprovider.h
@@ -97,7 +97,7 @@ namespace vcpkg
 
     struct IBaselineProvider
     {
-        virtual ExpectedL<Version> get_baseline_version(StringView port_name) const = 0;
+        virtual ExpectedL<Optional<Version>> get_baseline_version(StringView port_name) const = 0;
         virtual ~IBaselineProvider() = default;
     };
 
diff --git a/include/vcpkg/registries.h b/include/vcpkg/registries.h
@@ -153,7 +153,7 @@ namespace vcpkg
     std::unique_ptr<RegistryImplementation> make_git_registry(const VcpkgPaths& paths,
                                                               std::string repo,
                                                               std::string reference,
-                                                              std::string baseline);
+                                                              Optional<std::string> baseline);
     std::unique_ptr<RegistryImplementation> make_filesystem_registry(const ReadOnlyFilesystem& fs,
                                                                      Path path,
                                                                      std::string baseline);
diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp
@@ -26,7 +26,7 @@ struct MockBaselineProvider : IBaselineProvider
 {
     mutable std::map<std::string, Version, std::less<>> v;
 
-    ExpectedL<Version> get_baseline_version(StringView name) const override
+    ExpectedL<Optional<Version>> get_baseline_version(StringView name) const override
     {
         auto it = v.find(name);
         if (it == v.end())
diff --git a/src/vcpkg-test/registries.cpp b/src/vcpkg-test/registries.cpp
@@ -368,8 +368,10 @@ TEST_CASE ("registry_parsing", "[registries]")
     )json");
     {
         Json::Reader r{"test"};
-        visit_default_registry(r, std::move(test_json));
-        CHECK(r.messages().any_errors());
+        auto registry_impl = visit_default_registry(r, std::move(test_json));
+        REQUIRE(registry_impl);
+        INFO(r.messages().join());
+        CHECK_FALSE(r.messages().any_errors());
     }
 
     test_json = parse_json(R"json(
diff --git a/src/vcpkg/configuration.cpp b/src/vcpkg/configuration.cpp
@@ -234,11 +234,7 @@ namespace
             r.required_object_field(
                 msg::format(msgAGitRegistry), obj, JsonIdRepository, res.repo.emplace(), GitUrlDeserializer::instance);
             r.optional_object_field_emplace(obj, JsonIdReference, res.reference, GitReferenceDeserializer::instance);
-            r.required_object_field(msg::format(msgAGitRegistry),
-                                    obj,
-                                    JsonIdBaseline,
-                                    res.baseline.emplace(),
-                                    BaselineShaDeserializer::instance);
+            r.optional_object_field_emplace(obj, JsonIdBaseline, res.baseline, BaselineShaDeserializer::instance);
             r.check_for_unexpected_fields(obj, valid_git_fields, msg::format(msgAGitRegistry));
         }
         else if (kind == JsonIdArtifact)
@@ -897,7 +893,7 @@ namespace vcpkg
                 return make_git_registry(paths,
                                          config.repo.value_or_exit(VCPKG_LINE_INFO),
                                          config.reference.value_or("HEAD"),
-                                         config.baseline.value_or_exit(VCPKG_LINE_INFO));
+                                         config.baseline);
             }
             else if (*k == JsonIdFilesystem)
             {
diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp
@@ -2,6 +2,7 @@
 #include <vcpkg/base/graphs.h>
 #include <vcpkg/base/optional.h>
 #include <vcpkg/base/strings.h>
+#include <vcpkg/base/system.debug.h>
 #include <vcpkg/base/util.h>
 
 #include <vcpkg/cmakevars.h>
@@ -1456,7 +1457,9 @@ namespace vcpkg
 
             // Add an initial requirement for a package.
             // Returns a reference to the node to place additional constraints
-            Optional<PackageNode&> require_package(const PackageSpec& spec, const std::string& origin);
+            Optional<PackageNode&> require_package(const PackageSpec& spec,
+                                                   const std::string& origin,
+                                                   Optional<Version> maybe_declared_ver);
 
             void require_scfl(PackageNode& ref, const SourceControlFileAndLocation* scfl, const std::string& origin);
 
@@ -1512,14 +1515,15 @@ namespace vcpkg
                 if (!dep.platform.is_empty() && !dep.platform.evaluate(batch_load_vars(frame.spec))) continue;
 
                 PackageSpec dep_spec(dep.name, dep.host ? m_host_triplet : frame.spec.triplet());
-                auto maybe_node = require_package(dep_spec, frame.spec.name());
+
+                auto maybe_dep_ver = dep.constraint.try_get_minimum_version();
+                auto maybe_node = require_package(dep_spec, frame.spec.name(), maybe_dep_ver);
                 if (auto node = maybe_node.get())
                 {
                     // If the node is overlayed or overridden, don't apply version constraints
                     // If the baseline is a version_string, it occludes other constraints
                     if (!node->second.overlay_or_override)
                     {
-                        const auto maybe_dep_ver = dep.constraint.try_get_minimum_version();
                         if (auto dep_ver = maybe_dep_ver.get())
                         {
                             auto maybe_scfl = m_ver_provider.get_control_file({dep.name, *dep_ver});
@@ -1639,8 +1643,8 @@ namespace vcpkg
             return *it;
         }
 
-        Optional<VersionedPackageGraph::PackageNode&> VersionedPackageGraph::require_package(const PackageSpec& spec,
-                                                                                             const std::string& origin)
+        Optional<VersionedPackageGraph::PackageNode&> VersionedPackageGraph::require_package(
+            const PackageSpec& spec, const std::string& origin, Optional<Version> maybe_declared_ver)
         {
             // Implicit defaults are disabled if spec is requested from top-level spec.
             const bool default_features_mask = origin != m_toplevel.name();
@@ -1686,9 +1690,24 @@ namespace vcpkg
                 }
                 else
                 {
-                    auto maybe_scfl = m_base_provider.get_baseline_version(spec.name()).then([&](const Version& ver) {
-                        return m_ver_provider.get_control_file({spec.name(), ver});
-                    });
+                    auto maybe_scfl =
+                        m_base_provider.get_baseline_version(spec.name())
+                            .then([&](const Optional<Version>& ver) -> ExpectedL<const SourceControlFileAndLocation&> {
+                                if (auto baseline_ver = ver.get())
+                                {
+                                    return m_ver_provider.get_control_file({spec.name(), *baseline_ver});
+                                }
+                                else if (auto declared_ver = maybe_declared_ver.get())
+                                {
+                                    return m_ver_provider.get_control_file({spec.name(), *declared_ver});
+                                }
+                                else
+                                {
+                                    // TODO: this should be a PortNotInBaselineAndNoVersion
+                                    Debug::println("port {} not in baseline, no declared version", spec.name());
+                                    return msg::format_error(msgPortNotInBaseline, msg::package_name = spec.name());
+                                }
+                            });
                     if (auto p_scfl = maybe_scfl.get())
                     {
                         it = m_graph.emplace(spec, PackageNodeData{}).first;
diff --git a/src/vcpkg/portfileprovider.cpp b/src/vcpkg/portfileprovider.cpp
@@ -372,15 +372,17 @@ namespace vcpkg
         {
             return *scfl;
         }
-        auto maybe_baseline = m_baseline->get_baseline_version(spec);
-        if (auto baseline = maybe_baseline.get())
-        {
-            return m_versioned->get_control_file({spec, *baseline});
-        }
-        else
-        {
-            return std::move(maybe_baseline).error();
-        }
+        return m_baseline->get_baseline_version(spec).then(
+            [&](const Optional<Version>& ver) -> ExpectedL<const SourceControlFileAndLocation&> {
+                if (auto v = ver.get())
+                {
+                    return m_versioned->get_control_file({spec, *v});
+                }
+                else
+                {
+                    return msg::format_error(msgPortNotInBaseline, msg::package_name = spec);
+                }
+            });
     }
 
     std::vector<const SourceControlFileAndLocation*> PathsPortFileProvider::load_all_control_files() const
@@ -399,25 +401,16 @@ namespace vcpkg
             BaselineProviderImpl(const BaselineProviderImpl&) = delete;
             BaselineProviderImpl& operator=(const BaselineProviderImpl&) = delete;
 
-            virtual ExpectedL<Version> get_baseline_version(StringView port_name) const override
+            virtual ExpectedL<Optional<Version>> get_baseline_version(StringView port_name) const override
             {
-                return m_baseline_cache.get_lazy(port_name, [this, port_name]() -> ExpectedL<Version> {
-                    return registry_set.baseline_for_port(port_name).then(
-                        [&](Optional<Version>&& maybe_version) -> ExpectedL<Version> {
-                            auto version = maybe_version.get();
-                            if (!version)
-                            {
-                                return msg::format_error(msgPortNotInBaseline, msg::package_name = port_name);
-                            }
-
-                            return std::move(*version);
-                        });
+                return m_baseline_cache.get_lazy(port_name, [this, port_name]() -> ExpectedL<Optional<Version>> {
+                    return registry_set.baseline_for_port(port_name);
                 });
             }
 
         private:
             const RegistrySet& registry_set;
-            Cache<std::string, ExpectedL<Version>> m_baseline_cache;
+            Cache<std::string, ExpectedL<Optional<Version>>> m_baseline_cache;
         };
 
         struct VersionedPortfileProviderImpl : IFullVersionedPortfileProvider
diff --git a/src/vcpkg/registries.cpp b/src/vcpkg/registries.cpp
@@ -73,7 +73,10 @@ namespace
 
     struct GitRegistry final : RegistryImplementation
     {
-        GitRegistry(const VcpkgPaths& paths, std::string&& repo, std::string&& reference, std::string&& baseline)
+        GitRegistry(const VcpkgPaths& paths,
+                    std::string&& repo,
+                    std::string&& reference,
+                    Optional<std::string>&& baseline)
             : m_paths(paths)
             , m_repo(std::move(repo))
             , m_reference(std::move(reference))
@@ -202,7 +205,7 @@ namespace
 
         std::string m_repo;
         std::string m_reference;
-        std::string m_baseline_identifier;
+        Optional<std::string> m_baseline_identifier;
         DelayedInit<ExpectedL<LockFile::Entry>> m_lock_entry;
         mutable Optional<Path> m_stale_versions_tree;
         DelayedInit<ExpectedL<Path>> m_versions_tree;
@@ -730,9 +733,15 @@ namespace
 
     ExpectedL<Optional<Version>> GitRegistry::get_baseline_version(StringView port_name) const
     {
+        if (!m_baseline_identifier)
+        {
+            return nullopt;
+        }
+
         return lookup_in_maybe_baseline(m_baseline.get([this, port_name]() -> ExpectedL<Baseline> {
             // We delay baseline validation until here to give better error messages and suggestions
-            if (!is_git_sha(m_baseline_identifier))
+            auto baseline_id = m_baseline_identifier.value_or_exit(VCPKG_LINE_INFO);
+            if (!is_git_sha(baseline_id))
             {
                 auto& maybe_lock_entry = get_lock_entry();
                 auto lock_entry = maybe_lock_entry.get();
@@ -752,7 +761,7 @@ namespace
             }
 
             auto path_to_baseline = Path(FileVersions) / FileBaselineDotJson;
-            auto maybe_contents = m_paths.git_show_from_remote_registry(m_baseline_identifier, path_to_baseline);
+            auto maybe_contents = m_paths.git_show_from_remote_registry(baseline_id, path_to_baseline);
             if (!maybe_contents)
             {
                 auto& maybe_lock_entry = get_lock_entry();
@@ -768,13 +777,13 @@ namespace
                     return std::move(maybe_up_to_date).error();
                 }
 
-                maybe_contents = m_paths.git_show_from_remote_registry(m_baseline_identifier, path_to_baseline);
+                maybe_contents = m_paths.git_show_from_remote_registry(baseline_id, path_to_baseline);
             }
 
             if (!maybe_contents)
             {
                 msg::println(msgFetchingBaselineInfo, msg::package_name = m_repo);
-                auto maybe_err = m_paths.git_fetch(m_repo, m_baseline_identifier);
+                auto maybe_err = m_paths.git_fetch(m_repo, baseline_id);
                 if (!maybe_err)
                 {
                     get_global_metrics_collector().track_define(DefineMetric::RegistriesErrorCouldNotFindBaseline);
@@ -783,15 +792,15 @@ namespace
                         .append(maybe_err.error());
                 }
 
-                maybe_contents = m_paths.git_show_from_remote_registry(m_baseline_identifier, path_to_baseline);
+                maybe_contents = m_paths.git_show_from_remote_registry(baseline_id, path_to_baseline);
             }
 
             if (!maybe_contents)
             {
                 get_global_metrics_collector().track_define(DefineMetric::RegistriesErrorCouldNotFindBaseline);
                 return msg::format_error(msgCouldNotFindBaselineInCommit,
                                          msg::url = m_repo,
-                                         msg::commit_sha = m_baseline_identifier,
+                                         msg::commit_sha = baseline_id,
                                          msg::package_name = port_name)
                     .append_raw('\n')
                     .append_raw(maybe_contents.error());
@@ -801,9 +810,8 @@ namespace
             return parse_baseline_versions(*contents, JsonIdDefault, path_to_baseline)
                 .map_error([&](LocalizedString&& error) {
                     get_global_metrics_collector().track_define(DefineMetric::RegistriesErrorCouldNotFindBaseline);
-                    return msg::format_error(msgErrorWhileFetchingBaseline,
-                                             msg::value = m_baseline_identifier,
-                                             msg::package_name = m_repo)
+                    return msg::format_error(
+                               msgErrorWhileFetchingBaseline, msg::value = baseline_id, msg::package_name = m_repo)
                         .append_raw('\n')
                         .append(error);
                 });
@@ -1509,7 +1517,7 @@ namespace vcpkg
     std::unique_ptr<RegistryImplementation> make_git_registry(const VcpkgPaths& paths,
                                                               std::string repo,
                                                               std::string reference,
-                                                              std::string baseline)
+                                                              Optional<std::string> baseline)
     {
         return std::make_unique<GitRegistry>(paths, std::move(repo), std::move(reference), std::move(baseline));
     }

Original file line number	Diff line number	Diff line change
`@@ -97,7 +97,7 @@ namespace vcpkg`
`97`	`97`
`98`	`98`	`struct IBaselineProvider`
`99`	`99`	`{`
`100`		`- virtual ExpectedL<Version> get_baseline_version(StringView port_name) const = 0;`
	`100`	`+ virtual ExpectedL<Optional<Version>> get_baseline_version(StringView port_name) const = 0;`
`101`	`101`	`virtual ~IBaselineProvider() = default;`
`102`	`102`	`};`
`103`	`103`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ struct MockBaselineProvider : IBaselineProvider`
`26`	`26`	`{`
`27`	`27`	`mutable std::map<std::string, Version, std::less<>> v;`
`28`	`28`
`29`		`- ExpectedL<Version> get_baseline_version(StringView name) const override`
	`29`	`+ ExpectedL<Optional<Version>> get_baseline_version(StringView name) const override`
`30`	`30`	`{`
`31`	`31`	`auto it = v.find(name);`
`32`	`32`	`if (it == v.end())`
Original file line number	Diff line number	Diff line change
`@@ -368,8 +368,10 @@ TEST_CASE ("registry_parsing", "[registries]")`
`368`	`368`	`)json");`
`369`	`369`	`{`
`370`	`370`	`Json::Reader r{"test"};`
`371`		`- visit_default_registry(r, std::move(test_json));`
`372`		`- CHECK(r.messages().any_errors());`
	`371`	`+ auto registry_impl = visit_default_registry(r, std::move(test_json));`
	`372`	`+ REQUIRE(registry_impl);`
	`373`	`+ INFO(r.messages().join());`
	`374`	`+ CHECK_FALSE(r.messages().any_errors());`
`373`	`375`	`}`
`374`	`376`
`375`	`377`	`test_json = parse_json(R"json(`