Add check to ensure loaded port matches the declared version (#347)

* Add check to ensure loaded port matches the declared version * Dedupe errors. * Add end to end regression test. * Localize error * Remove e2e test dependency on Microsoft/vcpkg * Fixup localized error check. * Fix format-cxxcode on VS2022 machines, add message map. Co-authored-by: Billy Robert O'Neal III <[email protected]>
microsoft · Feb 11, 2022 · dac008b · dac008b
1 parent 5ba4532
commit dac008b
Show file tree

Hide file tree

Showing 20 changed files with 131 additions and 9 deletions.
diff --git a/azure-pipelines/Format-CxxCode.ps1 b/azure-pipelines/Format-CxxCode.ps1
@@ -16,6 +16,10 @@ if ($null -ne $clangFormat)
 
 if ($IsWindows)
 {
+    if ([String]::IsNullOrEmpty($clangFormat) -or -not (Test-Path $clangFormat))
+    {
+        $clangFormat = 'C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\Llvm\x64\bin\clang-format.exe'
+    }
     if ([String]::IsNullOrEmpty($clangFormat) -or -not (Test-Path $clangFormat))
     {
         $clangFormat = 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\Llvm\x64\bin\clang-format.exe'

diff --git a/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg-configuration.json b/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg-configuration.json
@@ -0,0 +1,13 @@
+{
+  "default-registry": null,
+  "registries": [
+    {
+      "kind": "filesystem",
+      "path": "./vcpkg_registry",
+      "packages": [
+        "arrow",
+        "bloom-filter"
+      ]
+    }
+  ]
+}
diff --git a/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg.json b/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg.json
@@ -0,0 +1,15 @@
+{
+  "name": "cycle-detected",
+  "version": "0.1.0",
+  "dependencies": [
+    "arrow",
+    "bloom-filter"
+  ],
+  "overrides": [
+    {
+      "name": "arrow",
+      "version": "6.0.0.20210925",
+      "port-version": 4
+    }
+  ]
+}
diff --git a/...pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/ports/arrow/portfile.cmake b/...pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/ports/arrow/portfile.cmake
@@ -0,0 +1 @@
+// intentionally empty
diff --git a/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/ports/arrow/vcpkg.json b/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/ports/arrow/vcpkg.json
@@ -0,0 +1,5 @@
+{
+  "name": "arrow",
+  "version": "6.0.0.20210925",
+  "description": "Cross-language development platform for in-memory analytics"
+}
diff --git a/...es/e2e_ports/mismatched-version-database/vcpkg_registry/ports/bloom-filter/portfile.cmake b/...es/e2e_ports/mismatched-version-database/vcpkg_registry/ports/bloom-filter/portfile.cmake
@@ -0,0 +1 @@
+// intentionally empty
diff --git a/...elines/e2e_ports/mismatched-version-database/vcpkg_registry/ports/bloom-filter/vcpkg.json b/...elines/e2e_ports/mismatched-version-database/vcpkg_registry/ports/bloom-filter/vcpkg.json
@@ -0,0 +1,7 @@
+{
+  "name": "bloom-filter",
+  "version": "0.2.0",
+  "port-version": 1,
+  "description": "bloom filter",
+  "dependencies": ["arrow"]
+}
diff --git a/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/versions/a-/arrow.json b/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/versions/a-/arrow.json
@@ -0,0 +1,9 @@
+{
+  "versions": [
+    {
+      "version": "6.0.0.20210925",
+      "port-version": 4,
+      "path": "$/ports/arrow"
+    }
+  ]
+}
diff --git a/...elines/e2e_ports/mismatched-version-database/vcpkg_registry/versions/b-/bloom-filter.json b/...elines/e2e_ports/mismatched-version-database/vcpkg_registry/versions/b-/bloom-filter.json
@@ -0,0 +1,9 @@
+{
+  "versions": [
+    {
+      "version": "0.2.0",
+      "port-version": 1,
+      "path": "$/ports/bloom-filter"
+    }
+  ]
+}
diff --git a/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/versions/baseline.json b/azure-pipelines/e2e_ports/mismatched-version-database/vcpkg_registry/versions/baseline.json
@@ -0,0 +1,13 @@
+{
+  "$doc": "https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/registries.md#filesystem-registries",
+  "default": {
+    "arrow": {
+      "baseline": "6.0.0.20210925",
+      "port-version": 4
+    },
+    "bloom-filter": {
+      "baseline": "0.2.0",
+      "port-version": 1
+    }
+  }
+}
diff --git a/azure-pipelines/end-to-end-tests-dir/versions.ps1 b/azure-pipelines/end-to-end-tests-dir/versions.ps1
@@ -71,12 +71,23 @@ Throw-IfFailed
 $CurrentTest = "default baseline"
 $out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root=$versionFilesPath/default-baseline-1 2>&1 | Out-String
 Throw-IfNotFailed
-if ($out -notmatch ".*Error: while checking out baseline.*")
+if ($out -notmatch ".*Error: while checking out baseline\.*")
 {
     $out
     throw "Expected to fail due to missing baseline"
 }
 
+$CurrentTest = "mismatched version database"
+$out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root="$PSScriptRoot/../e2e_ports/mismatched-version-database" 2>&1 | Out-String
+Throw-IfNotFailed
+if (($out -notmatch ".*Error: Failed to load port because version specs did not match*") -or
+  ($out -notmatch ".*Expected: [email protected]#4.*") -or
+  ($out -notmatch ".*Actual: [email protected].*"))
+{
+    $out
+    throw "Expected to fail due to mismatched versions between portfile and the version database"
+}
+
 git -C "$env:VCPKG_ROOT" fetch https://github.com/vicroms/test-registries
 foreach ($opt_registries in @("",",registries"))
 {

diff --git a/include/vcpkg/base/fwd/format.h b/include/vcpkg/base/fwd/format.h
@@ -19,3 +19,14 @@ namespace fmt
             return fmt::formatter<Base, Char, void>::format(static_cast<Base>(val), ctx);                              \
         }                                                                                                              \
     }
+
+#define VCPKG_FORMAT_WITH_TO_STRING(Type)                                                                              \
+    template<typename Char>                                                                                            \
+    struct fmt::formatter<Type, Char, void> : fmt::formatter<std::string, Char, void>                                  \
+    {                                                                                                                  \
+        template<typename FormatContext>                                                                               \
+        auto format(Type const& val, FormatContext& ctx) const -> decltype(ctx.out())                                  \
+        {                                                                                                              \
+            return fmt::formatter<std::string, Char, void>::format(val.to_string(), ctx);                              \
+        }                                                                                                              \
+    }
diff --git a/include/vcpkg/base/messages.h b/include/vcpkg/base/messages.h
@@ -166,6 +166,8 @@ namespace vcpkg::msg
     DECLARE_MSG_ARG(triplet);
     DECLARE_MSG_ARG(url);
     DECLARE_MSG_ARG(value);
+    DECLARE_MSG_ARG(expected);
+    DECLARE_MSG_ARG(actual);
     DECLARE_MSG_ARG(elapsed);
     DECLARE_MSG_ARG(version);
     DECLARE_MSG_ARG(list);

diff --git a/include/vcpkg/sourceparagraph.h b/include/vcpkg/sourceparagraph.h
@@ -125,6 +125,7 @@ namespace vcpkg
         {
             return SchemedVersion{core_paragraph->version_scheme, core_paragraph->to_version()};
         }
+        VersionSpec to_version_spec() const { return {core_paragraph->name, core_paragraph->to_version()}; }
 
         friend bool operator==(const SourceControlFile& lhs, const SourceControlFile& rhs);
         friend bool operator!=(const SourceControlFile& lhs, const SourceControlFile& rhs) { return !(lhs == rhs); }

diff --git a/include/vcpkg/versions.h b/include/vcpkg/versions.h
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <vcpkg/base/fwd/format.h>
+
 #include <vcpkg/base/expected.h>
 
 namespace vcpkg
@@ -74,6 +76,8 @@ namespace vcpkg
 
         VersionSpec(const std::string& port_name, const std::string& version_string, int port_version);
 
+        std::string to_string() const;
+
         friend bool operator==(const VersionSpec& lhs, const VersionSpec& rhs);
         friend bool operator!=(const VersionSpec& lhs, const VersionSpec& rhs);
     };
@@ -130,3 +134,5 @@ namespace vcpkg
         Minimum
     };
 }
+
+VCPKG_FORMAT_WITH_TO_STRING(vcpkg::VersionSpec);
diff --git a/locales/messages.json b/locales/messages.json
@@ -62,5 +62,6 @@
   "VcpkgHasCrashedArgument": "{value}|",
   "_VcpkgHasCrashedArgument.comment": "{LOCKED}",
   "VcpkgInvalidCommand": "invalid command: {value}",
-  "VcpkgSendMetricsButDisabled": "Warning: passed --sendmetrics, but metrics are disabled."
+  "VcpkgSendMetricsButDisabled": "Warning: passed --sendmetrics, but metrics are disabled.",
+  "VersionSpecMismatch": "Error: Failed to load port because version specs did not match\n    Path: {path}\n    Expected: {expected}\n    Actual: {actual}"
 }
diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp
@@ -1836,8 +1836,9 @@ namespace vcpkg::Dependencies
         ExpectedS<ActionPlan> VersionedPackageGraph::finalize_extract_plan(
             const PackageSpec& toplevel, UnsupportedPortAction unsupported_port_action)
         {
-            if (m_errors.size() > 0)
+            if (!m_errors.empty())
             {
+                Util::sort_unique_erase(m_errors);
                 return Strings::join("\n", m_errors);
             }
 

diff --git a/src/vcpkg/portfileprovider.cpp b/src/vcpkg/portfileprovider.cpp
@@ -1,4 +1,5 @@
 #include <vcpkg/base/json.h>
+#include <vcpkg/base/messages.h>
 #include <vcpkg/base/system.debug.h>
 
 #include <vcpkg/configuration.h>
@@ -87,6 +88,12 @@ namespace vcpkg::PortFileProvider
         return Util::fmap(m, [](const auto& p) { return p.second; });
     }
 
+    DECLARE_AND_REGISTER_MESSAGE(VersionSpecMismatch,
+                                 (msg::path, msg::expected, msg::actual),
+                                 "",
+                                 "Error: Failed to load port because version specs did not match\n    Path: "
+                                 "{path}\n    Expected: {expected}\n    Actual: {actual}");
+
     namespace
     {
         struct BaselineProviderImpl : IBaselineProvider
@@ -172,18 +179,19 @@ namespace vcpkg::PortFileProvider
                         auto maybe_control_file = Paragraphs::try_load_port(m_fs, *path);
                         if (auto scf = maybe_control_file.get())
                         {
-                            if (scf->get()->core_paragraph->name == version_spec.port_name)
+                            auto scf_vspec = scf->get()->to_version_spec();
+                            if (scf_vspec == version_spec)
                             {
                                 return std::unique_ptr<SourceControlFileAndLocation>(
                                     new SourceControlFileAndLocation{std::move(*scf), std::move(*path)});
                             }
                             else
                             {
-                                return Strings::format("Error: Failed to load port from %s: names did "
-                                                       "not match: '%s' != '%s'",
-                                                       *path,
-                                                       version_spec.port_name,
-                                                       scf->get()->core_paragraph->name);
+                                return msg::format(msgVersionSpecMismatch,
+                                                   msg::path = *path,
+                                                   msg::expected = version_spec,
+                                                   msg::actual = scf_vspec)
+                                    .extract_data();
                             }
                         }
                         else

diff --git a/src/vcpkg/registries.cpp b/src/vcpkg/registries.cpp
@@ -1004,6 +1004,8 @@ namespace
 
 Optional<Path> RegistryImplementation::get_path_to_baseline_version(StringView port_name) const
 {
+    // This code does not defend against the files in the baseline not matching the declared baseline version.
+    // However, this is only used by `Paragraphs::try_load_all_registry_ports` so it is not high-impact
     const auto baseline_version = this->get_baseline_version(port_name);
     if (auto b = baseline_version.get())
     {

diff --git a/src/vcpkg/versions.cpp b/src/vcpkg/versions.cpp
@@ -45,6 +45,8 @@ namespace vcpkg
         return Strings::format("%s -> %s", left.to_string(), right.to_string());
     }
 
+    std::string VersionSpec::to_string() const { return Strings::concat(port_name, '@', version); }
+
     namespace
     {
         Optional<uint64_t> as_numeric(StringView str)