Enable manifests, versions, and registries by default (#70)

* Remove stray debug output * Allow versioning without builtin-baseline ...only if the default registry is replaced * Enable registries, manifests, versions by default * Block update and upgrade in manifest mode * Require baseline for builtin registries * Improve status reporting for user Co-authored-by: Robert Schumacher <[email protected]>
microsoft · May 4, 2021 · 1bc8779 · 1bc8779
1 parent 5f76b14
commit 1bc8779
Show file tree

Hide file tree

Showing 12 changed files with 104 additions and 74 deletions.
diff --git a/azure-pipelines/end-to-end-tests-dir/registries.ps1 b/azure-pipelines/end-to-end-tests-dir/registries.ps1
@@ -1,6 +1,5 @@
 . "$PSScriptRoot/../end-to-end-tests-prelude.ps1"
 
-
 $builtinRegistryArgs = $commonArgs + @("--x-builtin-registry-versions-dir=$PSScriptRoot/../e2e_ports/versions")
 
 Run-Vcpkg install @builtinRegistryArgs 'vcpkg-internal-e2e-test-port'
@@ -123,7 +122,12 @@ $vcpkgJson = @{
     "version-string" = "1.0.0";
     "dependencies" = @(
         "vcpkg-internal-e2e-test-port"
-    )
+    );
+    # Use versioning features without a builtin-baseline
+    "overrides" = @(@{
+        "name" = "unused";
+        "version" = "0";
+    })
 }
 
 # test the filesystem registry

diff --git a/include/vcpkg/registries.h b/include/vcpkg/registries.h
@@ -111,7 +111,7 @@ namespace vcpkg
         void add_registry(Registry&& r);
         void set_default_registry(std::unique_ptr<RegistryImplementation>&& r);
         void set_default_registry(std::nullptr_t r);
-
+        bool is_default_builtin_registry() const;
         void set_default_builtin_registry_baseline(StringView baseline) const;
 
         // returns whether the registry set has any modifications to the default

diff --git a/include/vcpkg/sourceparagraph.h b/include/vcpkg/sourceparagraph.h
@@ -107,7 +107,8 @@ namespace vcpkg
         Optional<const std::vector<Dependency>&> find_dependencies_for_feature(const std::string& featurename) const;
 
         Optional<std::string> check_against_feature_flags(const fs::path& origin,
-                                                          const FeatureFlagSettings& flags) const;
+                                                          const FeatureFlagSettings& flags,
+                                                          bool is_default_builtin_registry = true) const;
 
         VersionT to_versiont() const { return core_paragraph->to_versiont(); }
         SchemedVersion to_schemed_version() const

diff --git a/include/vcpkg/vcpkgcmdarguments.h b/include/vcpkg/vcpkgcmdarguments.h
@@ -104,6 +104,7 @@ namespace vcpkg
         bool compiler_tracking;
         bool binary_caching;
         bool versions;
+        bool manifests;
     };
 
     struct VcpkgCmdArguments
@@ -205,15 +206,17 @@ namespace vcpkg
 
         bool binary_caching_enabled() const { return binary_caching.value_or(true); }
         bool compiler_tracking_enabled() const { return compiler_tracking.value_or(true); }
-        bool registries_enabled() const { return registries_feature.value_or(false); }
-        bool versions_enabled() const { return versions_feature.value_or(false); }
+        bool registries_enabled() const { return registries_feature.value_or(true); }
+        bool versions_enabled() const { return versions_feature.value_or(true); }
+        bool manifests_enabled() const { return manifest_mode.value_or(true); }
         FeatureFlagSettings feature_flag_settings() const
         {
             FeatureFlagSettings f;
             f.binary_caching = binary_caching_enabled();
             f.compiler_tracking = compiler_tracking_enabled();
             f.registries = registries_enabled();
             f.versions = versions_enabled();
+            f.manifests = manifests_enabled();
             return f;
         }
 

diff --git a/src/vcpkg-test/registries.cpp b/src/vcpkg-test/registries.cpp
@@ -79,45 +79,59 @@ TEST_CASE ("registry_parsing", "[registries]")
     auto registry_impl_des = get_registry_implementation_deserializer({});
     {
         Json::Reader r;
-
         auto test_json = parse_json(R"json(
 {
     "kind": "builtin"
 }
     )json");
-        auto registry_impl = r.visit(test_json, *registry_impl_des);
-        REQUIRE(registry_impl);
-        CHECK(*registry_impl.get());
-        CHECK(r.errors().empty());
-
-        test_json = parse_json(R"json(
+        r.visit(test_json, *registry_impl_des);
+        CHECK(!r.errors().empty());
+    }
+    {
+        Json::Reader r;
+        auto test_json = parse_json(R"json(
 {
     "kind": "builtin",
     "baseline": "hi"
 }
     )json");
-        registry_impl = r.visit(test_json, *registry_impl_des);
+        r.visit(test_json, *registry_impl_des);
+        CHECK(!r.errors().empty());
+    }
+    {
+        Json::Reader r;
+        auto test_json = parse_json(R"json(
+{
+    "kind": "builtin",
+    "baseline": "1234567890123456789012345678901234567890"
+}
+    )json");
+        auto registry_impl = r.visit(test_json, *registry_impl_des);
         REQUIRE(registry_impl);
         CHECK(*registry_impl.get());
         CHECK(r.errors().empty());
-
-        test_json = parse_json(R"json(
+    }
+    {
+        Json::Reader r;
+        auto test_json = parse_json(R"json(
 {
     "kind": "builtin",
+    "baseline": "1234567890123456789012345678901234567890",
     "path": "a/b"
 }
     )json");
-        registry_impl = r.visit(test_json, *registry_impl_des);
-        CHECK_FALSE(r.errors().empty());
-        r.errors().clear();
-
-        test_json = parse_json(R"json(
+        r.visit(test_json, *registry_impl_des);
+        CHECK(!r.errors().empty());
+    }
+    {
+        Json::Reader r;
+        auto test_json = parse_json(R"json(
 {
     "kind": "filesystem",
     "path": "a/b/c"
 }
     )json");
-        registry_impl = r.visit(test_json, *registry_impl_des);
+        auto registry_impl = r.visit(test_json, *registry_impl_des);
         REQUIRE(registry_impl);
         CHECK(*registry_impl.get());
         CHECK(r.errors().empty());

diff --git a/src/vcpkg/commands.upgrade.cpp b/src/vcpkg/commands.upgrade.cpp
@@ -41,6 +41,13 @@ namespace vcpkg::Commands::Upgrade
                           Triplet default_triplet,
                           Triplet host_triplet)
     {
+        if (paths.manifest_mode_enabled())
+        {
+            Checks::exit_maybe_upgrade(VCPKG_LINE_INFO,
+                                       "Error: the upgrade command does not currently support manifest mode. Instead, "
+                                       "modify your vcpkg.json and run install.");
+        }
+
         const ParsedArguments options = args.parse_arguments(COMMAND_STRUCTURE);
 
         const bool no_dry_run = Util::Sets::contains(options.switches, OPTION_NO_DRY_RUN);

diff --git a/src/vcpkg/install.cpp b/src/vcpkg/install.cpp
@@ -833,7 +833,10 @@ namespace vcpkg::Install
 
             auto& manifest_scf = *maybe_manifest_scf.value_or_exit(VCPKG_LINE_INFO);
 
-            if (auto maybe_error = manifest_scf.check_against_feature_flags(manifest_path, paths.get_feature_flags()))
+            if (auto maybe_error = manifest_scf.check_against_feature_flags(
+                    manifest_path,
+                    paths.get_feature_flags(),
+                    paths.get_configuration().registry_set.is_default_builtin_registry()))
             {
                 Checks::exit_with_message(VCPKG_LINE_INFO, maybe_error.value_or_exit(VCPKG_LINE_INFO));
             }
@@ -918,7 +921,6 @@ namespace vcpkg::Install
             extended_overlay_ports.reserve(args.overlay_ports.size() + 1);
             extended_overlay_ports.push_back(fs::u8string(manifest_path.parent_path()));
             Util::Vectors::append(&extended_overlay_ports, args.overlay_ports);
-            System::print2(Strings::join(",", extended_overlay_ports), "\n");
             auto oprovider = PortFileProvider::make_overlay_provider(paths, extended_overlay_ports);
 
             PackageSpec toplevel{manifest_scf.core_paragraph->name, default_triplet};

diff --git a/src/vcpkg/registries.cpp b/src/vcpkg/registries.cpp
@@ -571,6 +571,7 @@ namespace
             auto maybe_contents = paths.git_show_from_remote_registry(m_baseline_identifier, path_to_baseline);
             if (!maybe_contents.has_value())
             {
+                System::print2("Fetching baseline information from ", m_repo, "...\n");
                 if (auto err = paths.git_fetch(m_repo, m_baseline_identifier))
                 {
                     Metrics::g_metrics.lock()->track_property("registries-error-could-not-find-baseline", "defined");
@@ -966,7 +967,13 @@ namespace
         if (kind == KIND_BUILTIN)
         {
             std::string baseline;
-            r.optional_object_field(obj, BASELINE, baseline, baseline_deserializer);
+            r.required_object_field("a builtin registry", obj, BASELINE, baseline, baseline_deserializer);
+            if (!is_git_commit_sha(baseline))
+            {
+                r.add_generic_error(
+                    "a builtin registry",
+                    "The baseline field of builtin registries must be a git commit SHA (40 lowercase hex characters)");
+            }
             r.check_for_unexpected_fields(obj, valid_builtin_fields(), "a builtin registry");
             res = std::make_unique<BuiltinRegistry>(std::move(baseline));
         }
@@ -1172,6 +1179,7 @@ namespace vcpkg
         auto it = lockdata.find(key);
         if (it == lockdata.end())
         {
+            System::print2("Fetching registry information from ", key, "...\n");
             auto x = paths.git_fetch_from_remote_registry(key, "HEAD");
             it = lockdata.emplace(key.to_string(), EntryData{x.value_or_exit(VCPKG_LINE_INFO), false}).first;
             modified = true;
@@ -1182,6 +1190,7 @@ namespace vcpkg
     {
         if (data->second.stale)
         {
+            System::print2("Fetching registry information from ", data->first, "...\n");
             data->second.value =
                 paths.git_fetch_from_remote_registry(data->first, "HEAD").value_or_exit(VCPKG_LINE_INFO);
             data->second.stale = false;
@@ -1237,6 +1246,14 @@ namespace vcpkg
     }
     void RegistrySet::set_default_registry(std::nullptr_t) { default_registry_.reset(); }
 
+    bool RegistrySet::is_default_builtin_registry() const
+    {
+        if (auto default_builtin_registry = dynamic_cast<BuiltinRegistry*>(default_registry_.get()))
+        {
+            return default_builtin_registry->m_baseline_identifier.empty();
+        }
+        return false;
+    }
     void RegistrySet::set_default_builtin_registry_baseline(StringView baseline) const
     {
         if (auto default_builtin_registry = dynamic_cast<BuiltinRegistry*>(default_registry_.get()))
@@ -1257,14 +1274,14 @@ namespace vcpkg
         {
             System::printf(System::Color::warning,
                            "warning: the default registry has been replaced with a %s registry, but `builtin-baseline` "
-                           "is specified in vcpkg.json. This field will have no effect.",
+                           "is specified in vcpkg.json. This field will have no effect.\n",
                            default_registry->kind());
         }
         else
         {
             System::print2(System::Color::warning,
                            "warning: the default registry has been disabled, but `builtin-baseline` is specified in "
-                           "vcpkg.json. This field will have no effect.");
+                           "vcpkg.json. This field will have no effect.\n");
         }
     }
 

diff --git a/src/vcpkg/remove.cpp b/src/vcpkg/remove.cpp
@@ -218,9 +218,9 @@ namespace vcpkg::Remove
     {
         if (paths.manifest_mode_enabled())
         {
-            Checks::exit_maybe_upgrade(VCPKG_LINE_INFO,
-                                       "vcpkg remove does not support manifest mode. In order to remove dependencies, "
-                                       "you will need to edit your manifest (vcpkg.json).");
+            Checks::exit_maybe_upgrade(
+                VCPKG_LINE_INFO,
+                "To remove dependencies in manifest mode, edit your manifest (vcpkg.json) and run 'install'.");
         }
         const ParsedArguments options = args.parse_arguments(COMMAND_STRUCTURE);
 

diff --git a/src/vcpkg/sourceparagraph.cpp b/src/vcpkg/sourceparagraph.cpp
@@ -1024,7 +1024,8 @@ namespace vcpkg
     }
 
     Optional<std::string> SourceControlFile::check_against_feature_flags(const fs::path& origin,
-                                                                         const FeatureFlagSettings& flags) const
+                                                                         const FeatureFlagSettings& flags,
+                                                                         bool is_default_builtin_registry) const
     {
         static constexpr StringLiteral s_extended_help = "See `vcpkg help versioning` for more information.";
         if (!flags.versions)
@@ -1076,7 +1077,7 @@ namespace vcpkg
         }
         else
         {
-            if (!core_paragraph->builtin_baseline.has_value())
+            if (!core_paragraph->builtin_baseline.has_value() && is_default_builtin_registry)
             {
                 if (std::any_of(core_paragraph->dependencies.begin(),
                                 core_paragraph->dependencies.end(),

diff --git a/src/vcpkg/update.cpp b/src/vcpkg/update.cpp
@@ -7,6 +7,7 @@
 #include <vcpkg/update.h>
 #include <vcpkg/vcpkgcmdarguments.h>
 #include <vcpkg/vcpkglib.h>
+#include <vcpkg/vcpkgpaths.h>
 
 namespace vcpkg::Update
 {
@@ -55,6 +56,13 @@ namespace vcpkg::Update
 
     void perform_and_exit(const VcpkgCmdArguments& args, const VcpkgPaths& paths)
     {
+        if (paths.manifest_mode_enabled())
+        {
+            Checks::exit_maybe_upgrade(VCPKG_LINE_INFO,
+                                       "Error: the update command does not currently support manifest mode. Instead, "
+                                       "modify your vcpkg.json and run install.");
+        }
+
         (void)args.parse_arguments(COMMAND_STRUCTURE);
         System::print2("Using local portfile versions. To update the local portfiles, use `git pull`.\n");
 

diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp
@@ -90,7 +90,7 @@ namespace vcpkg
             for (auto&& msg : reader.errors())
                 System::print2("    ", msg, '\n');
 
-            System::print2("See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/registries.md for "
+            System::print2("See https://github.com/Microsoft/vcpkg/tree/master/docs/users/registries.md for "
                            "more information.\n");
             Checks::exit_fail(VCPKG_LINE_INFO);
         }
@@ -309,17 +309,24 @@ namespace vcpkg
         Debug::print("Using vcpkg-root: ", fs::u8string(root), '\n');
 
         std::error_code ec;
-        bool manifest_mode_on = args.manifest_mode.value_or(args.manifest_root_dir != nullptr);
-        if (args.manifest_root_dir)
+        if (args.manifests_enabled())
         {
-            manifest_root_dir = filesystem.canonical(VCPKG_LINE_INFO, fs::u8path(*args.manifest_root_dir));
+            if (args.manifest_root_dir)
+            {
+                manifest_root_dir = filesystem.canonical(VCPKG_LINE_INFO, fs::u8path(*args.manifest_root_dir));
+            }
+            else
+            {
+                manifest_root_dir = filesystem.find_file_recursively_up(original_cwd, fs::u8path("vcpkg.json"));
+            }
         }
-        else
+
+        if (manifest_root_dir.empty())
         {
-            manifest_root_dir = filesystem.find_file_recursively_up(original_cwd, fs::u8path("vcpkg.json"));
+            installed =
+                process_output_directory(filesystem, root, args.install_root_dir.get(), "installed", VCPKG_LINE_INFO);
         }
-
-        if (!manifest_root_dir.empty() && manifest_mode_on)
+        else
         {
             Debug::print("Using manifest-root: ", fs::u8string(manifest_root_dir), '\n');
 
@@ -350,40 +357,6 @@ namespace vcpkg
             m_pimpl->m_manifest_doc = load_manifest(filesystem, manifest_root_dir);
             m_pimpl->m_manifest_path = manifest_root_dir / fs::u8path("vcpkg.json");
         }
-        else
-        {
-            // we ignore the manifest root dir if the user requests -manifest
-            if (!manifest_root_dir.empty() && !args.manifest_mode.has_value() && !args.output_json())
-            {
-                System::print2(System::Color::warning,
-                               "Warning: manifest-root detected at ",
-                               fs::generic_u8string(manifest_root_dir),
-                               ", but manifests are not enabled.\n");
-                System::printf(System::Color::warning,
-                               R"(If you wish to use manifest mode, you may do one of the following:
-    * Add the `%s` feature flag to the comma-separated environment
-      variable `%s`.
-    * Add the `%s` feature flag to the `--%s` option.
-    * Pass your manifest directory to the `--%s` option.
-If you wish to silence this error and use classic mode, you can:
-    * Add the `-%s` feature flag to `%s`.
-    * Add the `-%s` feature flag to `--%s`.
-)",
-                               VcpkgCmdArguments::MANIFEST_MODE_FEATURE,
-                               VcpkgCmdArguments::FEATURE_FLAGS_ENV,
-                               VcpkgCmdArguments::MANIFEST_MODE_FEATURE,
-                               VcpkgCmdArguments::FEATURE_FLAGS_ARG,
-                               VcpkgCmdArguments::MANIFEST_ROOT_DIR_ARG,
-                               VcpkgCmdArguments::MANIFEST_MODE_FEATURE,
-                               VcpkgCmdArguments::FEATURE_FLAGS_ENV,
-                               VcpkgCmdArguments::MANIFEST_MODE_FEATURE,
-                               VcpkgCmdArguments::FEATURE_FLAGS_ARG);
-            }
-
-            manifest_root_dir.clear();
-            installed =
-                process_output_directory(filesystem, root, args.install_root_dir.get(), "installed", VCPKG_LINE_INFO);
-        }
 
         auto config_file = load_configuration(filesystem, args, root, manifest_root_dir);