Skip to content

Commit

Permalink
Move merging original_cwd into overlay directories into VcpkgPaths.
Browse files Browse the repository at this point in the history
This work is part of resolving microsoft/vcpkg#30942 / https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597

When overlay directories from the config were added in microsoft#743 this added a condition where VcpkgPaths took responsibility to tack on the relative path, transforming those paths that come from the config into absolute paths. I did not realize at the time that this block was repeating machinery already present in IOverlayProvider family. The difference is that, there, it always assumed the prefix would be original_cwd.

As part of adding overlay-port-dirs, I needed to add the same kind of prefix handling as overlay-ports get, thus pointing this out to me. It doesn't make sense to keep two independent ways to do this, leaving two options:

* Move the prefix stapling into VcpkgPaths (as done in this PR)
* Move the prefix stapling related to configs down into OverlayPortProviderImpl

I chose to move into VcpkgPaths for several reasons:

* Plumbing the information about how to handle config paths correctly into OverlayPortProviderImpl would have added many many function parameters to large parts of the product that are only passing paths information around.
* The decision to chdir in VcpkgPaths is a consideration *it* makes, and OverlayPortProviderImpl would be just as happy if the relative paths were left alone. (And in fact I think the right behavior in the future would be to remove this harmful chdir but that is not proposed right now)
* Despite making VcpkgPaths do "more work" here, I actually argue that it makes the "VcpkgPaths big ball of mud" problem better, by removing parts of the code that need to consider original_cwd.
  • Loading branch information
BillyONeal committed Oct 30, 2024
1 parent 3c2f4bf commit 1cb8eee
Show file tree
Hide file tree
Showing 19 changed files with 80 additions and 96 deletions.
3 changes: 2 additions & 1 deletion include/vcpkg/commands.find.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <vcpkg/base/fwd/files.h>
#include <vcpkg/base/fwd/optional.h>
#include <vcpkg/base/fwd/stringview.h>

Expand All @@ -14,7 +15,7 @@ namespace vcpkg
bool full_description,
bool enable_json,
Optional<StringView> filter,
View<std::string> overlay_ports);
View<Path> overlay_ports);
extern const CommandMetadata CommandFindMetadata;
void command_find_and_exit(const VcpkgCmdArguments& args, const VcpkgPaths& paths);
}
7 changes: 2 additions & 5 deletions include/vcpkg/portfileprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,9 @@ 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,
const Path& original_cwd,
View<std::string> overlay_ports);
std::unique_ptr<IFullOverlayProvider> make_overlay_provider(const ReadOnlyFilesystem& fs, View<Path> overlay_ports);
std::unique_ptr<IOverlayProvider> make_manifest_provider(const ReadOnlyFilesystem& fs,
const Path& original_cwd,
View<std::string> overlay_ports,
View<Path> overlay_ports,
const Path& manifest_path,
std::unique_ptr<SourceControlFile>&& manifest_scf);
}
4 changes: 2 additions & 2 deletions include/vcpkg/vcpkgpaths.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ namespace vcpkg
private:
const Path triplets;
const Path community_triplets;
std::vector<std::string> overlay_triplets;
std::vector<Path> overlay_triplets;

public:
std::vector<std::string> overlay_ports;
std::vector<Path> overlay_ports;

std::string get_toolver_diagnostics() const;

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 @@ -48,7 +48,7 @@ namespace vcpkg

auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.original_cwd, overlays));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, overlays));
command_build_and_exit_ex(args, paths, host_triplet, build_options, spec, provider, null_build_logs_recorder());
}
}
3 changes: 1 addition & 2 deletions src/vcpkg/commands.build.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ namespace vcpkg

auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));
Checks::exit_with_code(VCPKG_LINE_INFO,
command_build_ex(args,
paths,
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.check-support.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,7 @@ namespace vcpkg

auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));
auto cmake_vars = CMakeVars::make_triplet_cmake_var_provider(paths);

// for each spec in the user-requested specs, check all dependencies
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.ci.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,7 @@ namespace vcpkg
build_logs_recorder_storage ? *(build_logs_recorder_storage.get()) : null_build_logs_recorder();

auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(filesystem, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(filesystem, paths.overlay_ports));
auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths);
auto& var_provider = *var_provider_storage;

Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.depend-info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,7 @@ namespace vcpkg

auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));
auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths);
auto& var_provider = *var_provider_storage;

Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.env.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ namespace vcpkg
const ParsedArguments options = args.parse_arguments(CommandEnvMetadata);

auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));
auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths);
auto& var_provider = *var_provider_storage;

Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,7 @@ namespace vcpkg
// Load ports from ports dirs
auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));

// create the plan
std::vector<ExportPlanAction> export_plan = create_export_plan(opts.specs, status_db);
Expand Down
4 changes: 2 additions & 2 deletions src/vcpkg/commands.find.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,12 @@ namespace vcpkg
bool full_description,
bool enable_json,
Optional<StringView> filter,
View<std::string> overlay_ports)
View<Path> overlay_ports)
{
Checks::check_exit(VCPKG_LINE_INFO, msg::default_output_stream == OutputStream::StdErr);
auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.original_cwd, overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, overlay_ports));
auto source_paragraphs =
Util::fmap(provider.load_all_control_files(),
[](auto&& port) -> const SourceControlFile* { return port->source_control_file.get(); });
Expand Down
11 changes: 5 additions & 6 deletions src/vcpkg/commands.install.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1240,16 +1240,16 @@ namespace vcpkg
auto verprovider = make_versioned_portfile_provider(*registry_set);
auto baseprovider = make_baseline_provider(*registry_set);

std::vector<std::string> extended_overlay_ports;
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;
if (add_builtin_ports_directory_as_overlay)
{
extended_overlay_ports.emplace_back(paths.builtin_ports_directory().native());
extended_overlay_ports.emplace_back(paths.builtin_ports_directory());
}

auto oprovider = make_manifest_provider(
fs, paths.original_cwd, extended_overlay_ports, manifest->path, std::move(manifest_scf));
auto oprovider =
make_manifest_provider(fs, extended_overlay_ports, manifest->path, std::move(manifest_scf));
auto install_plan = create_versioned_install_plan(*verprovider,
*baseprovider,
*oprovider,
Expand Down Expand Up @@ -1278,8 +1278,7 @@ namespace vcpkg
}

auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));

const std::vector<FullPackageSpec> specs = Util::fmap(options.command_arguments, [&](const std::string& arg) {
return check_and_get_full_package_spec(arg, default_triplet, paths.get_triplet_db())
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.package-info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ namespace vcpkg
Json::Object response;
Json::Object results;
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));

for (auto&& arg : options.command_arguments)
{
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.remove.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,7 @@ namespace vcpkg
// Load ports from ports dirs
auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));

specs =
Util::fmap(find_outdated_packages(provider, status_db), [](auto&& outdated) { return outdated.spec; });
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.set-installed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,7 @@ namespace vcpkg

auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));
auto cmake_vars = CMakeVars::make_triplet_cmake_var_provider(paths);

Optional<Path> pkgsconfig;
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.update.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ namespace vcpkg
const StatusParagraphs status_db = database_load_check(fs, paths.installed());

auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));

const auto outdated_packages = SortedVector<OutdatedPackage, decltype(&OutdatedPackage::compare_by_name)>(
find_outdated_packages(provider, status_db), &OutdatedPackage::compare_by_name);
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg/commands.upgrade.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ namespace vcpkg
// Load ports from ports dirs
auto& fs = paths.get_filesystem();
auto registry_set = paths.make_registry_set();
PathsPortFileProvider provider(*registry_set,
make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports));
PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports));
auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths);
auto& var_provider = *var_provider_storage;

Expand Down
23 changes: 8 additions & 15 deletions src/vcpkg/portfileprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,8 @@ namespace vcpkg

struct OverlayProviderImpl : IFullOverlayProvider
{
OverlayProviderImpl(const ReadOnlyFilesystem& fs, const Path& original_cwd, View<std::string> overlay_ports)
: m_fs(fs), m_overlay_ports(Util::fmap(overlay_ports, [&original_cwd](const std::string& s) -> Path {
return original_cwd / s;
}))
OverlayProviderImpl(const ReadOnlyFilesystem& fs, View<Path> overlay_ports)
: m_fs(fs), m_overlay_ports(overlay_ports.begin(), overlay_ports.end())
{
for (auto&& overlay : m_overlay_ports)
{
Expand Down Expand Up @@ -363,11 +361,10 @@ namespace vcpkg
struct ManifestProviderImpl : IFullOverlayProvider
{
ManifestProviderImpl(const ReadOnlyFilesystem& fs,
const Path& original_cwd,
View<std::string> overlay_ports,
View<Path> overlay_ports,
const Path& manifest_path,
std::unique_ptr<SourceControlFile>&& manifest_scf)
: m_overlay_ports{fs, original_cwd, overlay_ports}
: m_overlay_ports{fs, overlay_ports}
, m_manifest_scf_and_location{std::move(manifest_scf), manifest_path}
{
}
Expand Down Expand Up @@ -406,21 +403,17 @@ namespace vcpkg
return std::make_unique<VersionedPortfileProviderImpl>(registry_set);
}

std::unique_ptr<IFullOverlayProvider> make_overlay_provider(const ReadOnlyFilesystem& fs,
const Path& original_cwd,
View<std::string> overlay_ports)
std::unique_ptr<IFullOverlayProvider> make_overlay_provider(const ReadOnlyFilesystem& fs, View<Path> overlay_ports)
{
return std::make_unique<OverlayProviderImpl>(fs, original_cwd, overlay_ports);
return std::make_unique<OverlayProviderImpl>(fs, overlay_ports);
}

std::unique_ptr<IOverlayProvider> make_manifest_provider(const ReadOnlyFilesystem& fs,
const Path& original_cwd,
View<std::string> overlay_ports,
View<Path> overlay_ports,
const Path& manifest_path,
std::unique_ptr<SourceControlFile>&& manifest_scf)
{
return std::make_unique<ManifestProviderImpl>(
fs, original_cwd, overlay_ports, manifest_path, std::move(manifest_scf));
return std::make_unique<ManifestProviderImpl>(fs, overlay_ports, manifest_path, std::move(manifest_scf));
}

} // namespace vcpkg
Loading

0 comments on commit 1cb8eee

Please sign in to comment.