Ensure spdx_location is not dropped when loading ports. (#1237)

include/vcpkg/fwd/sourceparagraph.h

@@ -3,6 +3,13 @@
 namespace vcpkg
 {
     struct ManifestAndPath;
+    struct DependencyConstraint;
+    struct DependencyRequestedFeature;
+    struct Dependency;
+    struct DependencyOverride;
+    struct FeatureParagraph;
+    struct SourceParagraph;
+    struct PortLocation;
     struct SourceControlFile;
     struct SourceControlFileAndLocation;
 }

include/vcpkg/paragraphs.h

@@ -27,22 +27,19 @@ namespace vcpkg::Paragraphs
     bool is_port_directory(const ReadOnlyFilesystem& fs, const Path& maybe_directory);
 
     // If an error occurs, the Expected will be in the error state.
-    // Otherwise, if the port is known, the unique_ptr contains the loaded port information.
-    // Otherwise, the unique_ptr is nullptr.
-    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port(const ReadOnlyFilesystem& fs,
-                                                                StringView port_name,
-                                                                const Path& port_directory);
+    // Otherwise, if the port is known, result->source_control_file contains the loaded port information.
+    // Otherwise, result->source_control_file is nullptr.
+    ExpectedL<SourceControlFileAndLocation> try_load_port(const ReadOnlyFilesystem& fs,
+                                                          StringView port_name,
+                                                          const PortLocation& port_location);
     // Identical to try_load_port, but the port unknown condition is mapped to an error.
-    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_required(const ReadOnlyFilesystem& fs,
-                                                                         StringView port_name,
-                                                                         const Path& port_directory);
-    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_project_manifest_text(StringView text,
-                                                                                 StringView origin,
-                                                                                 MessageSink& warning_sink);
+    ExpectedL<SourceControlFileAndLocation> try_load_port_required(const ReadOnlyFilesystem& fs,
+                                                                   StringView port_name,
+                                                                   const PortLocation& port_location);
     ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_manifest_text(StringView text,
-                                                                              StringView origin,
+                                                                              StringView control_path,
                                                                               MessageSink& warning_sink);
-    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_control_file_text(StringView text, StringView origin);
+    ExpectedL<std::unique_ptr<SourceControlFile>> try_load_control_file_text(StringView text, StringView control_path);
 
     ExpectedL<BinaryControlFile> try_load_cached_package(const ReadOnlyFilesystem& fs,
                                                          const Path& package_dir,

include/vcpkg/registries.h

@@ -7,6 +7,7 @@
 
 #include <vcpkg/fwd/configuration.h>
 #include <vcpkg/fwd/registries.h>
+#include <vcpkg/fwd/sourceparagraph.h>
 #include <vcpkg/fwd/vcpkgpaths.h>
 
 #include <vcpkg/base/path.h>
@@ -53,20 +54,11 @@ namespace vcpkg
         bool modified = false;
     };
 
-    struct PathAndLocation
-    {
-        Path path;
-
-        /// Should model SPDX PackageDownloadLocation. Empty implies NOASSERTION.
-        /// See https://spdx.github.io/spdx-spec/package-information/#77-package-download-location-field
-        std::string location;
-    };
-
     struct RegistryEntry
     {
         virtual ExpectedL<View<Version>> get_port_versions() const = 0;
 
-        virtual ExpectedL<PathAndLocation> get_version(const Version& version) const = 0;
+        virtual ExpectedL<PortLocation> get_version(const Version& version) const = 0;
 
         virtual ~RegistryEntry() = default;
     };

include/vcpkg/sourceparagraph.h

@@ -138,6 +138,15 @@ namespace vcpkg
         friend bool operator!=(const SourceParagraph& lhs, const SourceParagraph& rhs) { return !(lhs == rhs); }
     };
 
+    struct PortLocation
+    {
+        Path port_directory;
+
+        /// Should model SPDX PackageDownloadLocation. Empty implies NOASSERTION.
+        /// See https://spdx.github.io/spdx-spec/package-information/#77-package-download-location-field
+        std::string spdx_location;
+    };
+
     /// <summary>
     /// Full metadata of a package: core and other features.
     /// </summary>
@@ -168,7 +177,9 @@ namespace vcpkg
                                                     const FeatureFlagSettings& flags,
                                                     bool is_default_builtin_registry = true) const;
 
-        const Version& to_version() const { return core_paragraph->version; }
+        const std::string& to_name() const noexcept { return core_paragraph->name; }
+        VersionScheme to_version_scheme() const noexcept { return core_paragraph->version_scheme; }
+        const Version& to_version() const noexcept { return core_paragraph->version; }
         SchemedVersion to_schemed_version() const
         {
             return SchemedVersion{core_paragraph->version_scheme, core_paragraph->version};
@@ -190,15 +201,19 @@ namespace vcpkg
     /// </summary>
     struct SourceControlFileAndLocation
     {
+        const std::string& to_name() const noexcept { return source_control_file->to_name(); }
         const Version& to_version() const { return source_control_file->to_version(); }
         VersionScheme scheme() const { return source_control_file->core_paragraph->version_scheme; }
         SchemedVersion schemed_version() const { return {scheme(), to_version()}; }
+        VersionSpec to_version_spec() const { return source_control_file->to_version_spec(); }
+        Path port_directory() const { return control_path.parent_path(); }
 
         std::unique_ptr<SourceControlFile> source_control_file;
-        Path source_location;
+        Path control_path;
+
         /// Should model SPDX PackageDownloadLocation. Empty implies NOASSERTION.
         /// See https://spdx.github.io/spdx-spec/package-information/#77-package-download-location-field
-        std::string registry_location;
+        std::string spdx_location;
     };
 
     void print_error_message(const LocalizedString& message);
@@ -208,6 +223,6 @@ namespace vcpkg
 
     // Exposed for testing
     ExpectedL<std::vector<Dependency>> parse_dependencies_list(const std::string& str,
-                                                               StringView origin = "<unknown>",
+                                                               StringView origin,
                                                                TextRowCol textrowcol = {});
 }

src/vcpkg-test/dependencies.cpp

@@ -67,7 +67,10 @@ struct MockVersionedPortfileProvider : IVersionedPortfileProvider
             core->version_scheme = scheme;
             core->version = version;
             scf->core_paragraph = std::move(core);
-            it2 = version_map.emplace(std::move(version), SourceControlFileAndLocation{std::move(scf), name}).first;
+            it2 = version_map
+                      .emplace(std::move(version),
+                               SourceControlFileAndLocation{std::move(scf), Path(std::move(name)) / "vcpkg.json"})
+                      .first;
         }
 
         return it2->second;

src/vcpkg-test/manifests.cpp

@@ -93,9 +93,9 @@ TEST_CASE ("manifest construct minimum", "[manifests]")
     REQUIRE(m_pgh.has_value());
     auto& pgh = **m_pgh.get();
 
-    REQUIRE(pgh.core_paragraph->name == "zlib");
-    REQUIRE(pgh.core_paragraph->version_scheme == VersionScheme::String);
-    REQUIRE(pgh.core_paragraph->version == Version{"1.2.8", 0});
+    REQUIRE(pgh.to_name() == "zlib");
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::String);
+    REQUIRE(pgh.to_version() == Version{"1.2.8", 0});
     REQUIRE(pgh.core_paragraph->maintainers.empty());
     REQUIRE(pgh.core_paragraph->contacts.is_empty());
     REQUIRE(pgh.core_paragraph->summary.empty());
@@ -126,9 +126,9 @@ TEST_CASE ("project manifest construct minimum", "[manifests]")
     REQUIRE(m_pgh.has_value());
     auto& pgh = **m_pgh.get();
 
-    REQUIRE(pgh.core_paragraph->name.empty());
-    REQUIRE(pgh.core_paragraph->version_scheme == VersionScheme::Missing);
-    REQUIRE(pgh.core_paragraph->version == Version{});
+    REQUIRE(pgh.to_name().empty());
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::Missing);
+    REQUIRE(pgh.to_version() == Version{});
     REQUIRE(pgh.core_paragraph->maintainers.empty());
     REQUIRE(pgh.core_paragraph->contacts.is_empty());
     REQUIRE(pgh.core_paragraph->summary.empty());
@@ -904,9 +904,9 @@ TEST_CASE ("manifest construct maximum", "[manifests]")
     REQUIRE(*res.get() != nullptr);
     auto& pgh = **res.get();
 
-    REQUIRE(pgh.core_paragraph->name == "s");
-    REQUIRE(pgh.core_paragraph->version_scheme == VersionScheme::String);
-    REQUIRE(pgh.core_paragraph->version == Version{"v", 0});
+    REQUIRE(pgh.to_name() == "s");
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::String);
+    REQUIRE(pgh.to_version() == Version{"v", 0});
     REQUIRE(pgh.core_paragraph->maintainers.size() == 1);
     REQUIRE(pgh.core_paragraph->maintainers[0] == "m");
     REQUIRE(pgh.core_paragraph->contacts.size() == 1);
@@ -1028,9 +1028,9 @@ TEST_CASE ("SourceParagraph manifest construct qualified dependencies", "[manife
     REQUIRE(m_pgh.has_value());
     auto& pgh = **m_pgh.get();
 
-    REQUIRE(pgh.core_paragraph->name == "zlib");
-    REQUIRE(pgh.core_paragraph->version_scheme == VersionScheme::String);
-    REQUIRE(pgh.core_paragraph->version == Version{"1.2.8", 0});
+    REQUIRE(pgh.to_name() == "zlib");
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::String);
+    REQUIRE(pgh.to_version() == Version{"1.2.8", 0});
     REQUIRE(pgh.core_paragraph->maintainers.empty());
     REQUIRE(pgh.core_paragraph->description.empty());
     REQUIRE(pgh.core_paragraph->dependencies.size() == 2);
@@ -1058,9 +1058,9 @@ TEST_CASE ("SourceParagraph manifest construct host dependencies", "[manifests]"
     REQUIRE(m_pgh.has_value());
     auto& pgh = **m_pgh.get();
 
-    REQUIRE(pgh.core_paragraph->name == "zlib");
-    REQUIRE(pgh.core_paragraph->version_scheme == VersionScheme::String);
-    REQUIRE(pgh.core_paragraph->version == Version{"1.2.8", 0});
+    REQUIRE(pgh.to_name() == "zlib");
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::String);
+    REQUIRE(pgh.to_version() == Version{"1.2.8", 0});
     REQUIRE(pgh.core_paragraph->maintainers.empty());
     REQUIRE(pgh.core_paragraph->description.empty());
     REQUIRE(pgh.core_paragraph->dependencies.size() == 2);

src/vcpkg-test/paragraph.cpp

@@ -41,8 +41,9 @@ TEST_CASE ("SourceParagraph construct minimum", "[paragraph]")
     REQUIRE(m_pgh.has_value());
     auto& pgh = **m_pgh.get();
 
-    REQUIRE(pgh.core_paragraph->name == "zlib");
-    REQUIRE(pgh.core_paragraph->version == Version{"1.2.8", 0});
+    REQUIRE(pgh.to_name() == "zlib");
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::String);
+    REQUIRE(pgh.to_version() == Version{"1.2.8", 0});
     REQUIRE(pgh.core_paragraph->maintainers.empty());
     REQUIRE(pgh.core_paragraph->description.empty());
     REQUIRE(pgh.core_paragraph->dependencies.size() == 0);
@@ -105,8 +106,9 @@ TEST_CASE ("SourceParagraph construct maximum", "[paragraph]")
     REQUIRE(m_pgh.has_value());
     auto& pgh = **m_pgh.get();
 
-    REQUIRE(pgh.core_paragraph->name == "s");
-    REQUIRE(pgh.core_paragraph->version == Version{"v", 0});
+    REQUIRE(pgh.to_name() == "s");
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::String);
+    REQUIRE(pgh.to_version() == Version{"v", 0});
     REQUIRE(pgh.core_paragraph->maintainers.size() == 1);
     REQUIRE(pgh.core_paragraph->maintainers[0] == "m");
     REQUIRE(pgh.core_paragraph->description.size() == 1);
@@ -179,8 +181,9 @@ TEST_CASE ("SourceParagraph construct qualified dependencies", "[paragraph]")
     REQUIRE(m_pgh.has_value());
     auto& pgh = **m_pgh.get();
 
-    REQUIRE(pgh.core_paragraph->name == "zlib");
-    REQUIRE(pgh.core_paragraph->version == Version{"1.2.8", 0});
+    REQUIRE(pgh.to_name() == "zlib");
+    REQUIRE(pgh.to_version_scheme() == VersionScheme::String);
+    REQUIRE(pgh.to_version() == Version{"1.2.8", 0});
     REQUIRE(pgh.core_paragraph->maintainers.empty());
     REQUIRE(pgh.core_paragraph->description.empty());
     REQUIRE(pgh.core_paragraph->dependencies.size() == 2);

src/vcpkg-test/spdx.cpp

@@ -9,7 +9,7 @@ TEST_CASE ("spdx maximum serialization", "[spdx]")
 {
     PackageSpec spec{"zlib", Test::ARM_UWP};
     SourceControlFileAndLocation scfl;
-    scfl.registry_location = "git://some-vcs-url";
+    scfl.spdx_location = "git://some-vcs-url";
     auto& scf = *(scfl.source_control_file = std::make_unique<SourceControlFile>());
     auto& cpgh = *(scf.core_paragraph = std::make_unique<SourceParagraph>());
     cpgh.name = "zlib";

src/vcpkg-test/util.cpp

@@ -107,7 +107,8 @@ namespace vcpkg::Test
 
     PackageSpec PackageSpecMap::emplace(vcpkg::SourceControlFileAndLocation&& scfl)
     {
-        const auto& name = scfl.source_control_file->core_paragraph->name;
+        // copy name before moving scfl
+        auto name = scfl.to_name();
         REQUIRE(!Util::Maps::contains(map, name));
         map.emplace(name, std::move(scfl));
         return {name, triplet};

src/vcpkg-test/versionplan.cpp

@@ -11,7 +11,7 @@ using namespace vcpkg;
 
 TEST_CASE ("parse depends", "[dependencies]")
 {
-    auto w = parse_dependencies_list("liba (windows)");
+    auto w = parse_dependencies_list("liba (windows)", "<test>");
     REQUIRE(w);
     auto& v = *w.get();
     REQUIRE(v.size() == 1);
@@ -30,7 +30,7 @@ TEST_CASE ("filter depends", "[dependencies]")
 
     const std::unordered_map<std::string, std::string> arm_uwp_cmake_vars{{"VCPKG_TARGET_ARCHITECTURE", "arm"},
                                                                           {"VCPKG_CMAKE_SYSTEM_NAME", "WindowsStore"}};
-    auto deps_ = parse_dependencies_list("liba (!uwp), libb, libc (uwp)");
+    auto deps_ = parse_dependencies_list("liba (!uwp), libb, libc (uwp)", "<test>");
     REQUIRE(deps_);
     auto& deps = *deps_.get();
     SECTION ("x64-windows")
@@ -57,7 +57,8 @@ TEST_CASE ("filter depends", "[dependencies]")
 TEST_CASE ("parse feature depends", "[dependencies]")
 {
     auto u_ = parse_dependencies_list("libwebp[anim, gif2webp, img2webp, info, mux, nearlossless, "
-                                      "simd, cwebp, dwebp], libwebp[vwebp-sdl, extras] (!osx)");
+                                      "simd, cwebp, dwebp], libwebp[vwebp-sdl, extras] (!osx)",
+                                      "<test>");
     REQUIRE(u_);
     auto& v = *u_.get();
     REQUIRE(v.size() == 2);

src/vcpkg/cmakevars.cpp

@@ -365,7 +365,7 @@ endfunction()
         for (const auto& install_action : action_plan.install_actions)
         {
             auto& scfl = install_action.source_control_file_and_location.value_or_exit(VCPKG_LINE_INFO);
-            const auto override_path = scfl.source_location / "vcpkg-abi-settings.cmake";
+            const auto override_path = scfl.port_directory() / "vcpkg-abi-settings.cmake";
             spec_abi_settings.emplace_back(FullPackageSpec{install_action.spec, install_action.feature_list},
                                            override_path.generic_u8string());
         }

src/vcpkg/commands.add-version.cpp

@@ -4,6 +4,7 @@
 #include <vcpkg/base/git.h>
 #include <vcpkg/base/json.h>
 #include <vcpkg/base/strings.h>
+#include <vcpkg/base/system.process.h>
 #include <vcpkg/base/util.h>
 
 #include <vcpkg/commands.add-version.h>
@@ -386,7 +387,7 @@ namespace vcpkg
         auto maybe_git_tree_map = paths.git_get_local_port_treeish_map();
         auto& git_tree_map = maybe_git_tree_map.value_or_exit(VCPKG_LINE_INFO);
 
-        // Find ports with uncommited changes
+        // Find ports with uncommitted changes
         std::set<std::string> changed_ports;
         auto git_config = paths.git_builtin_config();
         auto maybe_changes = git_ports_with_uncommitted_changes(git_config);
@@ -403,36 +404,30 @@ namespace vcpkg
         {
             auto port_dir = paths.builtin_ports_directory() / port_name;
 
-            if (!fs.exists(port_dir, IgnoreErrors{}))
-            {
-                msg::println_error(msgPortDoesNotExist, msg::package_name = port_name);
-                Checks::check_exit(VCPKG_LINE_INFO, !add_all);
-                continue;
-            }
-
-            auto maybe_scf =
-                Paragraphs::try_load_port_required(fs, port_name, paths.builtin_ports_directory() / port_name);
-            auto scf = maybe_scf.get();
-            if (!scf)
+            auto maybe_scfl = Paragraphs::try_load_port_required(
+                fs, port_name, PortLocation{paths.builtin_ports_directory() / port_name});
+            auto scfl = maybe_scfl.get();
+            if (!scfl)
             {
                 msg::println_error(msgAddVersionLoadPortFailed, msg::package_name = port_name);
-                msg::println(Color::error, maybe_scf.error());
+                msg::println(Color::error, maybe_scfl.error());
                 Checks::check_exit(VCPKG_LINE_INFO, !add_all);
                 continue;
             }
 
             if (!skip_formatting_check)
             {
                 // check if manifest file is property formatted
-                const auto path_to_manifest = paths.builtin_ports_directory() / port_name / "vcpkg.json";
-                if (fs.exists(path_to_manifest, IgnoreErrors{}))
+
+                if (scfl->control_path.filename() == "vcpkg.json")
                 {
-                    const auto current_file_content = fs.read_contents(path_to_manifest, VCPKG_LINE_INFO);
-                    const auto json = serialize_manifest(**scf);
+                    const auto current_file_content = fs.read_contents(scfl->control_path, VCPKG_LINE_INFO);
+                    const auto json = serialize_manifest(*scfl->source_control_file);
                     const auto formatted_content = Json::stringify(json);
                     if (current_file_content != formatted_content)
                     {
-                        auto command_line = fmt::format("vcpkg format-manifest ports/{}/vcpkg.json", port_name);
+                        std::string command_line = "vcpkg format-manifest ";
+                        append_shell_escaped(command_line, scfl->control_path);
                         msg::println_error(
                             msg::format(msgAddVersionPortHasImproperFormat, msg::package_name = port_name)
                                 .append_raw('\n')
@@ -452,8 +447,7 @@ namespace vcpkg
                 msg::println_warning(msgAddVersionUncommittedChanges, msg::package_name = port_name);
             }
 
-            const auto& schemed_version = (*scf)->to_schemed_version();
-
+            auto schemed_version = scfl->source_control_file->to_schemed_version();
             auto git_tree_it = git_tree_map.find(port_name);
             if (git_tree_it == git_tree_map.end())
             {

src/vcpkg/commands.autocomplete.cpp

@@ -106,7 +106,7 @@ namespace vcpkg
                 StringView triplet_prefix{colon + 1, last_arg.end()};
                 // TODO: Support autocomplete for ports in --overlay-ports
                 auto maybe_port = Paragraphs::try_load_port_required(
-                    paths.get_filesystem(), port_name, paths.builtin_ports_directory() / port_name);
+                    paths.get_filesystem(), port_name, PortLocation{paths.builtin_ports_directory() / port_name});
                 if (!maybe_port)
                 {
                     Checks::exit_success(VCPKG_LINE_INFO);