Deduplicate overlay-ports handling and avoid repeated querying of the…

… filesystem for 'port' vs 'subdirectories' (microsoft#1534)

Other notable behavior changes:

* Attempting to set `overlay-ports` to the provably same directory as a top-level manifest now generates an error, with a special error for `"."` in the configuration file. Note that `overlay-triplets` are *not* affected.
* Previously, the `load_all_control_files` family on `OverlayPortProviderImpl` would load all ports in the lowest priority `--overlay-ports`, then overwrite any of those with 'higher' values. This meant that malformed ports which vcpkg would never consider would be forced to be loaded, thus potentially triggering irrelevant errors.
* Previously, some paths that considered overlay ports in the "subdirectories of named overlay ports" would enforce that their contained `CONTROL`/`vcpkg.json` declared a matching name, and some paths did not. Now, all paths consider ports with mismatched names an error rather than silently using the name in `vcpkg.json`.
* Previously, there were additional `fs.exists` checks in order to determine if the indicated directory should contain the port or subdirectories that are ports; now, we `try_read_contents` and consider file not found as nonexistent, saving a disk operation.
* `commands.install.cpp` no longer gets entirely broken in the 'add the builtin ports directory as an overlay' cases if there is a `vcpkg.json` in `%VCPKG_ROOT%/ports`.

Related:
* microsoft/vcpkg-docs#422 (Documentation update clarifying overlay-ports)

Resolves:
* https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597

azure-pipelines/e2e-projects/overlays-dot/vcpkg-configuration.json

@@ -0,0 +1,3 @@
+{
+  "overlay-ports": [ "." ]
+}

azure-pipelines/e2e-projects/overlays-dot/vcpkg.json

@@ -0,0 +1,5 @@
+{
+	"dependencies": [
+		"a"
+	]
+}

azure-pipelines/e2e-projects/overlays-malformed-shadowing/config-overlays/a/portfile.cmake

@@ -0,0 +1 @@
+set(VCPKG_POLICY_EMPTY_PACKAGE enabled)

azure-pipelines/e2e-projects/overlays-malformed-shadowing/config-overlays/a/vcpkg.json

@@ -0,0 +1,4 @@
+{
+  "name": "a",
+  "version": "0"
+}

azure-pipelines/e2e-projects/overlays-malformed-shadowing/config-overlays/malformed/a/portfile.cmake

@@ -0,0 +1,2 @@
+# This port is intentionally malformed and should not be loaded, because ../../a should be loaded instead
+set(VCPKG_POLICY_EMPTY_PACKAGE enabled)

azure-pipelines/e2e-projects/overlays-malformed-shadowing/config-overlays/malformed/a/vcpkg.json

@@ -0,0 +1,5 @@
+{
+  "name": "a",
+  "version": "0",
+  "intentionally-malformed"
+}

azure-pipelines/e2e-projects/overlays-malformed-shadowing/vcpkg-configuration.json

@@ -0,0 +1,4 @@
+{
+  "$comment": "'config-overlays/a' in . should get loaded first, so we should never consider 'config-overlays/malformed/a'",
+  "overlay-ports": [ "config-overlays", "config-overlays/malformed" ]
+}

azure-pipelines/e2e-projects/overlays-malformed-shadowing/vcpkg.json

@@ -0,0 +1,5 @@
+{
+	"dependencies": [
+		"a"
+	]
+}

azure-pipelines/e2e-projects/overlays-not-quite-dot/hello/ensure-directory.txt

@@ -0,0 +1 @@
+This file exists only to ensure that git creates the directory containing it, so that ./hello/.. is a valid path.

azure-pipelines/e2e-projects/overlays-not-quite-dot/vcpkg-configuration.json

@@ -0,0 +1,3 @@
+{
+  "overlay-ports": [ "./hello/.." ]
+}

azure-pipelines/e2e-projects/overlays-not-quite-dot/vcpkg.json

@@ -0,0 +1,5 @@
+{
+	"dependencies": [
+		"a"
+	]
+}

azure-pipelines/end-to-end-tests-dir/commands.extract.ps1

@@ -12,8 +12,8 @@ if (-Not (Test-Path $extractedFilePath)) {
 
 if (-Not $IsWindows) {
     $unixMode = (Get-Item $extractedFilePath).UnixMode
-    if ($unixMode -ne "-rwxr-xr-x") {
-        throw "File does not have +x permission. UnixMode: $unixMode"
+    if (-not ($unixMode -match 'x.*x.*x')) {
+        throw "File does not have +x permission. $extractedFilePath UnixMode: $unixMode"
     }
 }
 

azure-pipelines/end-to-end-tests-dir/overlays.ps1

@@ -2,9 +2,9 @@
 
 # Tests a simple project with overlay ports and triplets configured on a vcpkg-configuration.json file
 Copy-Item -Recurse -LiteralPath @(
-    "$PSScriptRoot/../e2e-projects/overlays-project-with-config",
+    "$PSScriptRoot/../e2e-projects/overlays-malformed-shadowing",
     "$PSScriptRoot/../e2e-projects/overlays-project-config-embedded",
-    "$PSScriptRoot/../e2e-projects/overlays-bad-paths"
+    "$PSScriptRoot/../e2e-projects/overlays-project-with-config"
     ) $TestingRoot
 
 $manifestRoot = "$TestingRoot/overlays-project-with-config"
@@ -27,13 +27,32 @@ Run-Vcpkg install --x-manifest-root=$manifestRoot `
     --triplet fancy-config-embedded-triplet
 Throw-IfFailed
 
+# ... and with command line overlay-ports being 'dot'
+pushd "$manifestRoot/cli-overlays"
+try {
+    Run-Vcpkg install --x-manifest-root=$manifestRoot `
+    --overlay-ports=. `
+    --overlay-triplets=$manifestRoot/my-triplets `
+    --x-install-root=$installRoot `
+    --triplet fancy-config-embedded-triplet
+    Throw-IfFailed
+} finally {
+    popd
+}
+
 # Config with bad paths
-$manifestRoot = "$TestingRoot/overlays-bad-paths"
+$manifestRoot = "$PSScriptRoot/../e2e-projects/overlays-bad-paths"
 $env:VCPKG_OVERLAY_PORTS = "$manifestRoot/env_overlays"
 Run-Vcpkg install --x-manifest-root=$manifestRoot `
     --overlay-triplets=$manifestRoot/my-triplets `
     --x-install-root=$installRoot
 Throw-IfNotFailed
+Remove-Item env:VCPKG_OVERLAY_PORTS
+
+# Test that once an overlay port is loaded for a name, subsequent ports are not considered
+$manifestRoot = "$TestingRoot/overlays-malformed-shadowing"
+Run-Vcpkg install --x-manifest-root=$manifestRoot
+Throw-IfFailed
 
 # Test overlay_triplet paths remain relative to the manifest root after x-update-baseline
 $manifestRoot = "$TestingRoot/overlays-project-with-config"
@@ -47,11 +66,27 @@ $overlaysAfter = $configurationAfter."overlay-triplets"
 $notEqual = @(Compare-Object $overlaysBefore $overlaysAfter -SyncWindow 0).Length -ne 0
 
 if ($notEqual) {
-	Throw "Overlay triplets paths changed after x-update-baseline"
+    Throw "Overlay triplets paths changed after x-update-baseline"
 }
 
+# Test that trying to declare overlay-ports as '.' fails
+$manifestRoot = "$PSScriptRoot/../e2e-projects/overlays-dot"
+$output = Run-VcpkgAndCaptureStdErr install --x-manifest-root=$manifestRoot --x-install-root=$installRoot
+Throw-IfNotFailed
+Throw-IfNonContains -Actual $output -Expected @"
+error: The manifest directory cannot be the same as a directory configured in overlay-ports, so "overlay-ports" values cannot be ".".
+"@
+
+# Test that trying to declare overlay-ports as the same directory in a roundabout way fails
+$manifestRoot = "$PSScriptRoot/../e2e-projects/overlays-not-quite-dot"
+$canonicalManifestRoot = (Get-Item $manifestRoot).FullName
+$output = Run-VcpkgAndCaptureStdErr install --x-manifest-root=$manifestRoot --x-install-root=$installRoot
+Throw-IfNotFailed
+Throw-IfNonContains -Actual $output -Expected @"
+The manifest directory ($canonicalManifestRoot) cannot be the same as a directory configured in overlay-ports.
+"@
+
 # Test that removals can happen without the overlay triplets
-Remove-Item env:VCPKG_OVERLAY_PORTS
 Refresh-TestRoot
 Run-Vcpkg install another-vcpkg-empty-port:fancy-triplet `
     --overlay-ports=$PSScriptRoot/../e2e-projects/overlays-project-with-config/cli-overlays `

include/vcpkg/base/message-data.inc.h

@@ -1123,6 +1123,15 @@ DECLARE_MESSAGE(ErrorInvalidManifestModeOption,
                 (msg::option),
                 "",
                 "The option --{option} is not supported in manifest mode.")
+DECLARE_MESSAGE(ErrorManifestMustDifferFromOverlay,
+                (msg::path),
+                "",
+                "The manifest directory ({path}) cannot be the same as a directory configured in overlay-ports.")
+DECLARE_MESSAGE(ErrorManifestMustDifferFromOverlayDot,
+                (),
+                "",
+                "The manifest directory cannot be the same as a directory configured in overlay-ports, so "
+                "\"overlay-ports\" values cannot be \".\".")
 DECLARE_MESSAGE(
     ErrorMissingVcpkgRoot,
     (),
@@ -2104,7 +2113,8 @@ DECLARE_MESSAGE(MismatchedManifestAfterReserialize,
 DECLARE_MESSAGE(MismatchedNames,
                 (msg::package_name, msg::actual),
                 "{actual} is the port name found",
-                "names did not match: '{package_name}' != '{actual}'")
+                "the port name declared in the metadata file did not match the directory. Expected the port to be "
+                "named {package_name}, but the file declares {actual}.")
 DECLARE_MESSAGE(MismatchedSpec,
                 (msg::path, msg::expected, msg::actual),
                 "{expected} and {actual} are package specs like 'zlib:x64-windows'",
@@ -2238,8 +2248,11 @@ DECLARE_MESSAGE(OptionRequiresOption,
 DECLARE_MESSAGE(Options, (), "Printed just before a list of options for a command", "Options")
 DECLARE_MESSAGE(OriginalBinParagraphHeader, (), "", "\nOriginal Binary Paragraph")
 DECLARE_MESSAGE(OtherCommandsHeader, (), "", "Other")
-DECLARE_MESSAGE(OverlayPatchDir, (msg::path), "", "Overlay path \"{path}\" must exist and must be a directory.")
-DECLARE_MESSAGE(OverlayPortsDirectoriesHelp, (msg::env_var), "", "Directories of overlay ports (also: {env_var})")
+DECLARE_MESSAGE(OverlayPatchDir, (msg::path), "", "Overlay path \"{path}\" must be an existing directory.")
+DECLARE_MESSAGE(OverlayPortsHelp,
+                (msg::env_var),
+                "",
+                "Overlay-port directories, or directories containing overlay-port directories (also: {env_var})")
 DECLARE_MESSAGE(OverlayTripletDirectoriesHelp, (msg::env_var), "", "Directories of overlay triplets (also: {env_var})")
 DECLARE_MESSAGE(OverlayTriplets, (msg::path), "", "Overlay Triplets from \"{path}\":")
 DECLARE_MESSAGE(OverwritingFile, (msg::path), "", "File {path} was already present and will be overwritten")

include/vcpkg/commands.find.h

@@ -4,6 +4,7 @@
 #include <vcpkg/base/fwd/optional.h>
 #include <vcpkg/base/fwd/stringview.h>
 
+#include <vcpkg/fwd/portfileprovider.h>
 #include <vcpkg/fwd/vcpkgcmdarguments.h>
 #include <vcpkg/fwd/vcpkgpaths.h>
 
@@ -15,7 +16,7 @@ namespace vcpkg
                                     bool full_description,
                                     bool enable_json,
                                     Optional<StringView> filter,
-                                    View<Path> overlay_ports);
+                                    const OverlayPortPaths& overlay_ports);
     extern const CommandMetadata CommandFindMetadata;
     void command_find_and_exit(const VcpkgCmdArguments& args, const VcpkgPaths& paths);
 }

include/vcpkg/fwd/portfileprovider.h

@@ -2,9 +2,18 @@
 
 namespace vcpkg
 {
+    struct OverlayPortPaths;
+    struct OverlayPortIndexEntry;
     struct PortFileProvider;
     struct PathsPortFileProvider;
     struct IVersionedPortfileProvider;
     struct IBaselineProvider;
     struct IOverlayProvider;
+
+    enum class OverlayPortKind
+    {
+        Unknown,   // We have not tested yet
+        Port,      // The overlay directory is itself a port
+        Directory, // The overlay directory itself contains port directories
+    };
 }

include/vcpkg/paragraphs.h

@@ -27,8 +27,6 @@ namespace vcpkg::Paragraphs
 
     void append_paragraph_field(StringView name, StringView field, std::string& out_str);
 
-    bool is_port_directory(const ReadOnlyFilesystem& fs, const Path& maybe_directory);
-
     struct PortLoadResult
     {
         ExpectedL<SourceControlFileAndLocation> maybe_scfl;
@@ -63,7 +61,4 @@ namespace vcpkg::Paragraphs
 
     LoadResults try_load_all_registry_ports(const RegistrySet& registries);
     std::vector<SourceControlFileAndLocation> load_all_registry_ports(const RegistrySet& registries);
-
-    LoadResults try_load_overlay_ports(const ReadOnlyFilesystem& fs, const Path& dir);
-    std::vector<SourceControlFileAndLocation> load_overlay_ports(const ReadOnlyFilesystem& fs, const Path& dir);
 }

include/vcpkg/portfileprovider.h

@@ -5,12 +5,66 @@
 
 #include <vcpkg/fwd/portfileprovider.h>
 #include <vcpkg/fwd/registries.h>
+#include <vcpkg/fwd/sourceparagraph.h>
 #include <vcpkg/fwd/versions.h>
 
-#include <vcpkg/sourceparagraph.h>
+#include <vcpkg/base/path.h>
+
+#include <memory>
+#include <string>
+#include <unordered_map>
+#include <vector>
 
 namespace vcpkg
 {
+    struct OverlayPortPaths
+    {
+        Optional<Path> builtin_overlay_port_dir;
+        std::vector<Path> overlay_ports;
+
+        bool empty() const noexcept;
+    };
+
+    struct OverlayPortIndexEntry
+    {
+        OverlayPortIndexEntry(OverlayPortKind kind, const Path& directory);
+        OverlayPortIndexEntry(const OverlayPortIndexEntry&) = delete;
+        OverlayPortIndexEntry(OverlayPortIndexEntry&&);
+
+        const ExpectedL<SourceControlFileAndLocation>* try_load_port(const ReadOnlyFilesystem& fs,
+                                                                     StringView port_name);
+
+        ExpectedL<Unit> try_load_all_ports(const ReadOnlyFilesystem& fs,
+                                           std::map<std::string, const SourceControlFileAndLocation*>& out);
+
+        void check_directory(const ReadOnlyFilesystem& fs) const;
+
+    private:
+        OverlayPortKind m_kind;
+        Path m_directory;
+
+        using MapT = std::map<std::string, ExpectedL<SourceControlFileAndLocation>, std::less<>>;
+        // If kind == OverlayPortKind::Unknown, empty
+        // Otherwise, if kind == OverlayPortKind::Port,
+        //    upon load success, contains exactly one entry with the loaded name of the port
+        //    upon load failure, contains exactly one entry with a key of empty string, value being the load error
+        // Otherwise, if kind == OverlayPortKind::Directory, contains an entry for each loaded overlay-port in the
+        // directory
+        MapT m_loaded_ports;
+
+        OverlayPortKind determine_kind(const ReadOnlyFilesystem& fs);
+        const ExpectedL<SourceControlFileAndLocation>* try_load_port_cached_port(StringView port_name);
+
+        MapT::iterator try_load_port_subdirectory_uncached(MapT::iterator hint,
+                                                           const ReadOnlyFilesystem& fs,
+                                                           StringView port_name);
+
+        ExpectedL<Unit> load_all_port_subdirectories(const ReadOnlyFilesystem& fs);
+
+        const ExpectedL<SourceControlFileAndLocation>* try_load_port_subdirectory_with_cache(
+            const ReadOnlyFilesystem& fs, StringView port_name);
+    };
+
     struct PortFileProvider
     {
         virtual ~PortFileProvider() = default;
@@ -76,9 +130,10 @@ namespace vcpkg
 
     std::unique_ptr<IBaselineProvider> make_baseline_provider(const RegistrySet& registry_set);
     std::unique_ptr<IFullVersionedPortfileProvider> make_versioned_portfile_provider(const RegistrySet& registry_set);
-    std::unique_ptr<IFullOverlayProvider> make_overlay_provider(const ReadOnlyFilesystem& fs, View<Path> overlay_ports);
+    std::unique_ptr<IFullOverlayProvider> make_overlay_provider(const ReadOnlyFilesystem& fs,
+                                                                const OverlayPortPaths& overlay_ports);
     std::unique_ptr<IOverlayProvider> make_manifest_provider(const ReadOnlyFilesystem& fs,
-                                                             View<Path> overlay_ports,
+                                                             const OverlayPortPaths& overlay_ports,
                                                              const Path& manifest_path,
                                                              std::unique_ptr<SourceControlFile>&& manifest_scf);
 }

include/vcpkg/vcpkgpaths.h

@@ -22,6 +22,8 @@
 #include <vcpkg/base/optional.h>
 #include <vcpkg/base/path.h>
 
+#include <vcpkg/portfileprovider.h>
+
 #include <map>
 #include <string>
 #include <vector>
@@ -101,7 +103,7 @@ namespace vcpkg
         std::vector<Path> overlay_triplets;
 
     public:
-        std::vector<Path> overlay_ports;
+        OverlayPortPaths overlay_ports;
 
         std::string get_toolver_diagnostics() const;
 

locales/messages.json

@@ -653,6 +653,9 @@
   "_ErrorInvalidExtractOption.comment": "The keyword 'AUTO' should not be localized An example of {option} is editable.",
   "ErrorInvalidManifestModeOption": "The option --{option} is not supported in manifest mode.",
   "_ErrorInvalidManifestModeOption.comment": "An example of {option} is editable.",
+  "ErrorManifestMustDifferFromOverlay": "The manifest directory ({path}) cannot be the same as a directory configured in overlay-ports.",
+  "_ErrorManifestMustDifferFromOverlay.comment": "An example of {path} is /foo/bar.",
+  "ErrorManifestMustDifferFromOverlayDot": "The manifest directory cannot be the same as a directory configured in overlay-ports, so \"overlay-ports\" values cannot be \".\".",
   "ErrorMissingVcpkgRoot": "Could not detect vcpkg-root. If you are trying to use a copy of vcpkg that you've built, you must define the VCPKG_ROOT environment variable to point to a cloned copy of https://github.com/Microsoft/vcpkg.",
   "ErrorNoVSInstance": "in triplet {triplet}: Unable to find a valid Visual Studio instance",
   "_ErrorNoVSInstance.comment": "An example of {triplet} is x64-windows.",
@@ -1149,7 +1152,7 @@
   "MismatchedFiles": "file to store does not match hash",
   "MismatchedManifestAfterReserialize": "The serialized manifest was different from the original manifest. Please open an issue at https://github.com/microsoft/vcpkg, with the following output:",
   "_MismatchedManifestAfterReserialize.comment": "The original file output and generated output are printed after this line, in English as it's intended to be used in the issue submission and read by devs. This message indicates an internal error in vcpkg.",
-  "MismatchedNames": "names did not match: '{package_name}' != '{actual}'",
+  "MismatchedNames": "the port name declared in the metadata file did not match the directory. Expected the port to be named {package_name}, but the file declares {actual}.",
   "_MismatchedNames.comment": "{actual} is the port name found An example of {package_name} is zlib.",
   "MismatchedSpec": "Mismatched spec in port {path}: expected {expected}, actual {actual}",
   "_MismatchedSpec.comment": "{expected} and {actual} are package specs like 'zlib:x64-windows' An example of {path} is /foo/bar.",
@@ -1234,10 +1237,10 @@
   "_Options.comment": "Printed just before a list of options for a command",
   "OriginalBinParagraphHeader": "\nOriginal Binary Paragraph",
   "OtherCommandsHeader": "Other",
-  "OverlayPatchDir": "Overlay path \"{path}\" must exist and must be a directory.",
+  "OverlayPatchDir": "Overlay path \"{path}\" must be an existing directory.",
   "_OverlayPatchDir.comment": "An example of {path} is /foo/bar.",
-  "OverlayPortsDirectoriesHelp": "Directories of overlay ports (also: {env_var})",
-  "_OverlayPortsDirectoriesHelp.comment": "An example of {env_var} is VCPKG_DEFAULT_TRIPLET.",
+  "OverlayPortsHelp": "Overlay-port directories, or directories containing overlay-port directories (also: {env_var})",
+  "_OverlayPortsHelp.comment": "An example of {env_var} is VCPKG_DEFAULT_TRIPLET.",
   "OverlayTripletDirectoriesHelp": "Directories of overlay triplets (also: {env_var})",
   "_OverlayTripletDirectoriesHelp.comment": "An example of {env_var} is VCPKG_DEFAULT_TRIPLET.",
   "OverlayTriplets": "Overlay Triplets from \"{path}\":",

src/vcpkg/commands.build-external.cpp

@@ -43,7 +43,7 @@ namespace vcpkg
                 .value_or_exit(VCPKG_LINE_INFO);
 
         auto overlays = paths.overlay_ports;
-        overlays.insert(overlays.begin(), options.command_arguments[1]);
+        overlays.overlay_ports.insert(overlays.overlay_ports.begin(), options.command_arguments[1]);
 
         auto& fs = paths.get_filesystem();
         auto registry_set = paths.make_registry_set();

src/vcpkg/commands.find.cpp

@@ -133,7 +133,7 @@ namespace vcpkg
                                     bool full_description,
                                     bool enable_json,
                                     Optional<StringView> filter,
-                                    View<Path> overlay_ports)
+                                    const OverlayPortPaths& overlay_ports)
     {
         Checks::check_exit(VCPKG_LINE_INFO, msg::default_output_stream == OutputStream::StdErr);
         auto& fs = paths.get_filesystem();

src/vcpkg/commands.install.cpp

@@ -1237,16 +1237,14 @@ namespace vcpkg
             auto verprovider = make_versioned_portfile_provider(*registry_set);
             auto baseprovider = make_baseline_provider(*registry_set);
 
-            std::vector<Path> extended_overlay_ports;
-            extended_overlay_ports.reserve(paths.overlay_ports.size() + add_builtin_ports_directory_as_overlay);
-            extended_overlay_ports = paths.overlay_ports;
+            auto extended_overlay_port_directories = paths.overlay_ports;
             if (add_builtin_ports_directory_as_overlay)
             {
-                extended_overlay_ports.emplace_back(paths.builtin_ports_directory());
+                extended_overlay_port_directories.builtin_overlay_port_dir.emplace(paths.builtin_ports_directory());
             }
 
             auto oprovider =
-                make_manifest_provider(fs, extended_overlay_ports, manifest->path, std::move(manifest_scf));
+                make_manifest_provider(fs, extended_overlay_port_directories, manifest->path, std::move(manifest_scf));
             auto install_plan = create_versioned_install_plan(*verprovider,
                                                               *baseprovider,
                                                               *oprovider,