Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deduplicate overlay-ports handling and avoid repeated querying of the filesystem for 'port' vs 'subdirectories' #1534

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"overlay-ports": [ "." ]
}
5 changes: 5 additions & 0 deletions azure-pipelines/e2e-projects/overlays-dot/vcpkg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": [
"a"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "a",
"version": "0"
}
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "a",
"version": "0",
"intentionally-malformed"
}
Original file line number Diff line number Diff line change
@@ -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" ]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": [
"a"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file exists only to ensure that git creates the directory containing it, so that ./hello/.. is a valid path.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"overlay-ports": [ "./hello/.." ]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": [
"a"
]
}
4 changes: 2 additions & 2 deletions azure-pipelines/end-to-end-tests-dir/commands.extract.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}

Expand Down
45 changes: 40 additions & 5 deletions azure-pipelines/end-to-end-tests-dir/overlays.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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 `
Expand Down
19 changes: 16 additions & 3 deletions include/vcpkg/base/message-data.inc.h
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,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,
(),
Expand Down Expand Up @@ -2090,7 +2099,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'",
Expand Down Expand Up @@ -2224,8 +2234,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")
Expand Down
3 changes: 2 additions & 1 deletion include/vcpkg/commands.find.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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);
}
9 changes: 9 additions & 0 deletions include/vcpkg/fwd/portfileprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}
5 changes: 0 additions & 5 deletions include/vcpkg/paragraphs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
61 changes: 58 additions & 3 deletions include/vcpkg/portfileprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
4 changes: 3 additions & 1 deletion include/vcpkg/vcpkgpaths.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include <vcpkg/base/optional.h>
#include <vcpkg/base/path.h>

#include <vcpkg/portfileprovider.h>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other vcpkgpaths.h things try very hard to only forward declare things. I did this because:

  1. portfileprovider.h itself looks fairly lightweight.
  2. This avoids needing to edit most users of VcpkgPaths::overlay_ports.


#include <map>
#include <string>
#include <utility>
Expand Down Expand Up @@ -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;

Expand Down
11 changes: 7 additions & 4 deletions locales/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,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.",
Expand Down Expand Up @@ -1143,7 +1146,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.",
Expand Down Expand Up @@ -1228,10 +1231,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}\":",
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.build-external.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.find.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 3 additions & 5 deletions src/vcpkg/commands.install.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1236,16 +1236,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,
Expand Down
Loading
Loading