Skip to content

Commit f19f3d9

Browse files
BillyONealBillyONeal
authored
Ensure package_dir is always filled in. (#1103)
Resolves microsoft/vcpkg#31908 In #1040 , create_feature_install_plan and create_versioned_install_plan were fixed, but not create_upgrade_plan. Make this bug structurally impossible by making the FooAction ctors that fill in m_scfl also fill in package_dir.
1 parent add000a commit f19f3d9

File tree

8 files changed

+107
-46
lines changed

8 files changed

+107
-46
lines changed

‎azure-pipelines/end-to-end-tests-dir/bundles.ps1‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ foreach ($k in $b.keys) {
150150

151151
Run-Vcpkg install vcpkg-hello-world-1 --dry-run @commonArgs `
152152
--x-buildtrees-root=$buildtreesRoot `
153-
--x-builtin-ports-root=$deploy/ports `
153+
--x-builtin-ports-root=$deployment/ports `
154154
--x-install-root=$installRoot `
155155
--x-packages-root=$packagesRoot
156156
Throw-IfNotFailed
@@ -164,7 +164,7 @@ $CurrentTest = "Testing bundle.usegitregistry"
164164
Run-Vcpkg install --dry-run @commonArgs `
165165
--x-manifest-root=$manifestdir `
166166
--x-buildtrees-root=$buildtreesRoot `
167-
--x-builtin-ports-root=$deploy/ports `
167+
--x-builtin-ports-root=$deployment/ports `
168168
--x-install-root=$installRoot `
169169
--x-packages-root=$packagesRoot
170170
Throw-IfNotFailed
@@ -184,15 +184,15 @@ New-Item -ItemType Directory -Force $manifestdir2 | Out-Null
184184
Run-Vcpkg install @commonArgs `
185185
--x-manifest-root=$manifestdir2 `
186186
--x-buildtrees-root=$buildtreesRoot `
187-
--x-builtin-ports-root=$deploy/ports `
187+
--x-builtin-ports-root=$deployment/ports `
188188
--x-install-root=$installRoot `
189189
--x-packages-root=$packagesRoot
190190
Throw-IfFailed
191191

192192
Run-Vcpkg search vcpkg-hello-world-1 @commonArgs `
193193
--x-manifest-root=$manifestdir2 `
194194
--x-buildtrees-root=$buildtreesRoot `
195-
--x-builtin-ports-root=$deploy/ports `
195+
--x-builtin-ports-root=$deployment/ports `
196196
--x-install-root=$installRoot `
197197
--x-packages-root=$packagesRoot
198198
Throw-IfFailed

‎azure-pipelines/end-to-end-tests-dir/upgrade.ps1‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
. "$PSScriptRoot/../end-to-end-tests-prelude.ps1"
2+
3+
git clone $VcpkgRoot "$TestingRoot/temp-repo" --local
4+
try
5+
{
6+
$env:VCPKG_ROOT = "$TestingRoot/temp-repo"
7+
git -C "$TestingRoot/temp-repo" switch -d e1934f4a2a0c58bb75099d89ed980832379907fa # vcpkg-cmake @ 2022-12-22
8+
$output = Run-VcpkgAndCaptureOutput install vcpkg-cmake
9+
Throw-IfFailed
10+
if (-Not ($output -match 'vcpkg-cmake:[^ ]+ -> 2022-12-22'))
11+
{
12+
throw 'Unexpected vcpkg-cmake install'
13+
}
14+
15+
git -C "$TestingRoot/temp-repo" switch -d f6a5d4e8eb7476b8d7fc12a56dff300c1c986131 # vcpkg-cmake @ 2023-05-04
16+
$output = Run-VcpkgAndCaptureOutput upgrade
17+
Throw-IfNotFailed
18+
if (-Not ($output -match 'If you are sure you want to rebuild the above packages, run this command with the --no-dry-run option.'))
19+
{
20+
throw "Upgrade didn't handle dry-run correctly"
21+
}
22+
23+
if (-Not ($output -match '\* vcpkg-cmake:[^ ]+ -> 2023-05-04'))
24+
{
25+
throw "Upgrade didn't choose expected version"
26+
}
27+
28+
$output = Run-VcpkgAndCaptureOutput upgrade --no-dry-run
29+
Throw-IfFailed
30+
if (-Not ($output -match '\* vcpkg-cmake:[^ ]+ -> 2023-05-04'))
31+
{
32+
throw "Upgrade didn't choose expected version"
33+
}
34+
35+
if (-Not ($output -match 'vcpkg-cmake:[^:]+: REMOVED:'))
36+
{
37+
throw "Upgrade didn't remove"
38+
}
39+
40+
if (-Not ($output -match 'vcpkg-cmake:[^:]+: SUCCEEDED:'))
41+
{
42+
throw "Upgrade didn't install"
43+
}
44+
}
45+
finally
46+
{
47+
$env:VCPKG_ROOT = $VcpkgRoot
48+
}

‎include/vcpkg/dependencies.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ namespace vcpkg
6666

6767
InstallPlanAction(const PackageSpec& spec,
6868
const SourceControlFileAndLocation& scfl,
69+
const Path& packages_dir,
6970
const RequestType& request_type,
7071
Triplet host_triplet,
7172
std::map<std::string, std::vector<FeatureSpec>>&& dependencies,

‎src/vcpkg-test/binarycaching.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ Build-Depends: bzip
223223

224224
InstallPlanAction ipa(PackageSpec{"zlib2", Test::X64_WINDOWS},
225225
scfl,
226+
"test_packages_root",
226227
RequestType::USER_REQUESTED,
227228
Test::ARM_UWP,
228229
{{"a", {}}, {"b", {}}},
@@ -347,6 +348,7 @@ Version: 1.5
347348
std::vector<InstallPlanAction> install_plan;
348349
install_plan.emplace_back(PackageSpec{"someheadpackage", Test::X64_WINDOWS},
349350
scfl,
351+
"test_packages_root",
350352
RequestType::USER_REQUESTED,
351353
Test::ARM_UWP,
352354
std::map<std::string, std::vector<FeatureSpec>>{},
@@ -422,6 +424,7 @@ Description: a spiffy compression library wrapper
422424
SourceControlFileAndLocation scfl{std::move(*maybe_scf.get()), Path()};
423425
plan.install_actions.emplace_back(PackageSpec("zlib", Test::X64_ANDROID),
424426
scfl,
427+
"test_packages_root",
425428
RequestType::USER_REQUESTED,
426429
Test::ARM64_WINDOWS,
427430
std::map<std::string, std::vector<FeatureSpec>>{},
@@ -448,6 +451,7 @@ Description: a spiffy compression library wrapper
448451
SourceControlFileAndLocation scfl2{std::move(*maybe_scf2.get()), Path()};
449452
plan.install_actions.emplace_back(PackageSpec("zlib2", Test::X64_ANDROID),
450453
scfl2,
454+
"test_packages_root",
451455
RequestType::USER_REQUESTED,
452456
Test::ARM64_WINDOWS,
453457
std::map<std::string, std::vector<FeatureSpec>>{},

‎src/vcpkg-test/dependencies.cpp‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2278,11 +2278,16 @@ TEST_CASE ("formatting plan 1", "[dependencies]")
22782278
const RemovePlanAction remove_b({"b", Test::X64_OSX}, RequestType::USER_REQUESTED);
22792279
const RemovePlanAction remove_a({"a", Test::X64_OSX}, RequestType::USER_REQUESTED);
22802280
const RemovePlanAction remove_c({"c", Test::X64_OSX}, RequestType::AUTO_SELECTED);
2281-
InstallPlanAction install_a({"a", Test::X64_OSX}, scfl_a, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});
2281+
2282+
const Path pr = "packages_root";
2283+
InstallPlanAction install_a(
2284+
{"a", Test::X64_OSX}, scfl_a, pr, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});
22822285
InstallPlanAction install_b(
2283-
{"b", Test::X64_OSX}, scfl_b, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {{"1", {}}}, {});
2284-
InstallPlanAction install_c({"c", Test::X64_OSX}, scfl_c, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
2285-
InstallPlanAction install_f({"f", Test::X64_OSX}, scfl_f, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
2286+
{"b", Test::X64_OSX}, scfl_b, pr, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {{"1", {}}}, {});
2287+
InstallPlanAction install_c(
2288+
{"c", Test::X64_OSX}, scfl_c, pr, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
2289+
InstallPlanAction install_f(
2290+
{"f", Test::X64_OSX}, scfl_f, pr, RequestType::USER_REQUESTED, Test::X64_ANDROID, {}, {});
22862291
install_f.plan_type = InstallPlanType::EXCLUDED;
22872292

22882293
InstallPlanAction already_installed_d(
@@ -2374,7 +2379,7 @@ TEST_CASE ("dependency graph API snapshot")
23742379
MockVersionedPortfileProvider vp;
23752380
auto& scfl_a = vp.emplace("a", {"1", 0});
23762381
InstallPlanAction install_a(
2377-
{"a", Test::X64_WINDOWS}, scfl_a, RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});
2382+
{"a", Test::X64_WINDOWS}, scfl_a, "packages_root", RequestType::AUTO_SELECTED, Test::X64_ANDROID, {}, {});
23782383

23792384
ActionPlan plan;
23802385
plan.install_actions.push_back(std::move(install_a));

‎src/vcpkg-test/spdx.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ TEST_CASE ("spdx maximum serialization", "[spdx]")
2121
cpgh.raw_version = "1.0";
2222
cpgh.version_scheme = VersionScheme::Relaxed;
2323

24-
InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
24+
InstallPlanAction ipa(spec, scfl, "test_packages_root", RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
2525
auto& abi = *(ipa.abi_info = AbiInfo{}).get();
2626
abi.package_abi = "ABIHASH";
2727

@@ -175,7 +175,7 @@ TEST_CASE ("spdx minimum serialization", "[spdx]")
175175
cpgh.raw_version = "1.0";
176176
cpgh.version_scheme = VersionScheme::String;
177177

178-
InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
178+
InstallPlanAction ipa(spec, scfl, "test_packages_root", RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
179179
auto& abi = *(ipa.abi_info = AbiInfo{}).get();
180180
abi.package_abi = "deadbeef";
181181

@@ -303,7 +303,7 @@ TEST_CASE ("spdx concat resources", "[spdx]")
303303
cpgh.raw_version = "1.0";
304304
cpgh.version_scheme = VersionScheme::String;
305305

306-
InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
306+
InstallPlanAction ipa(spec, scfl, "test_packages_root", RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
307307
auto& abi = *(ipa.abi_info = AbiInfo{}).get();
308308
abi.package_abi = "deadbeef";
309309

‎src/vcpkg/dependencies.cpp‎

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,8 @@ namespace vcpkg
292292
PackageGraph(const PortFileProvider& provider,
293293
const CMakeVars::CMakeVarProvider& var_provider,
294294
const StatusParagraphs& status_db,
295-
Triplet host_triplet);
295+
Triplet host_triplet,
296+
const Path& packages_dir);
296297
~PackageGraph() = default;
297298

298299
void install(Span<const FeatureSpec> specs, UnsupportedPortAction unsupported_port_action);
@@ -306,6 +307,7 @@ namespace vcpkg
306307
const CMakeVars::CMakeVarProvider& m_var_provider;
307308

308309
std::unique_ptr<ClusterGraph> m_graph;
310+
Path m_packages_dir;
309311
std::map<FeatureSpec, PlatformExpression::Expr> m_unsupported_features;
310312
};
311313

@@ -457,6 +459,7 @@ namespace vcpkg
457459

458460
InstallPlanAction::InstallPlanAction(const PackageSpec& spec,
459461
const SourceControlFileAndLocation& scfl,
462+
const Path& packages_dir,
460463
const RequestType& request_type,
461464
Triplet host_triplet,
462465
std::map<std::string, std::vector<FeatureSpec>>&& dependencies,
@@ -469,6 +472,7 @@ namespace vcpkg
469472
, feature_dependencies(std::move(dependencies))
470473
, build_failure_messages(std::move(build_failure_messages))
471474
, host_triplet(host_triplet)
475+
, package_dir(packages_dir / spec.dir())
472476
{
473477
}
474478

@@ -707,7 +711,7 @@ namespace vcpkg
707711
const StatusParagraphs& status_db,
708712
const CreateInstallPlanOptions& options)
709713
{
710-
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet);
714+
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet, options.packages_dir);
711715

712716
std::vector<FeatureSpec> feature_specs;
713717
for (const FullPackageSpec& spec : specs)
@@ -719,17 +723,7 @@ namespace vcpkg
719723

720724
pgraph.install(feature_specs, options.unsupported_port_action);
721725

722-
auto res = pgraph.serialize(options.randomizer);
723-
724-
for (auto&& action : res.install_actions)
725-
{
726-
if (action.source_control_file_and_location.has_value())
727-
{
728-
action.package_dir.emplace(options.packages_dir / action.spec.dir());
729-
}
730-
}
731-
732-
return res;
726+
return pgraph.serialize(options.randomizer);
733727
}
734728

735729
void PackageGraph::mark_for_reinstall(const PackageSpec& first_remove_spec,
@@ -927,7 +921,7 @@ namespace vcpkg
927921
const StatusParagraphs& status_db,
928922
const CreateInstallPlanOptions& options)
929923
{
930-
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet);
924+
PackageGraph pgraph(port_provider, var_provider, status_db, options.host_triplet, options.packages_dir);
931925

932926
pgraph.upgrade(specs, options.unsupported_port_action);
933927

@@ -1044,21 +1038,33 @@ namespace vcpkg
10441038
fspecs.insert(fspec);
10451039
continue;
10461040
}
1041+
10471042
auto&& dep_clust = m_graph->get(fspec.spec());
10481043
const auto& default_features = [&] {
10491044
if (dep_clust.m_install_info.has_value())
1045+
{
10501046
return dep_clust.get_scfl_or_exit()
10511047
.source_control_file->core_paragraph->default_features;
1052-
if (auto p = dep_clust.m_installed.get()) return p->ipv.core->package.default_features;
1048+
}
1049+
1050+
if (auto p = dep_clust.m_installed.get())
1051+
{
1052+
return p->ipv.core->package.default_features;
1053+
}
1054+
10531055
Checks::unreachable(VCPKG_LINE_INFO);
10541056
}();
1057+
10551058
for (auto&& default_feature : default_features)
1059+
{
10561060
fspecs.emplace(fspec.spec(), default_feature);
1061+
}
10571062
}
10581063
computed_edges[kv.first].assign(fspecs.begin(), fspecs.end());
10591064
}
10601065
plan.install_actions.emplace_back(p_cluster->m_spec,
10611066
p_cluster->get_scfl_or_exit(),
1067+
m_packages_dir,
10621068
p_cluster->request_type,
10631069
m_graph->m_host_triplet,
10641070
std::move(computed_edges),
@@ -1113,8 +1119,11 @@ namespace vcpkg
11131119
PackageGraph::PackageGraph(const PortFileProvider& port_provider,
11141120
const CMakeVars::CMakeVarProvider& var_provider,
11151121
const StatusParagraphs& status_db,
1116-
Triplet host_triplet)
1117-
: m_var_provider(var_provider), m_graph(create_feature_install_graph(port_provider, status_db, host_triplet))
1122+
Triplet host_triplet,
1123+
const Path& packages_dir)
1124+
: m_var_provider(var_provider)
1125+
, m_graph(create_feature_install_graph(port_provider, status_db, host_triplet))
1126+
, m_packages_dir(packages_dir)
11181127
{
11191128
}
11201129

@@ -1272,12 +1281,14 @@ namespace vcpkg
12721281
const IBaselineProvider& base_provider,
12731282
const IOverlayProvider& oprovider,
12741283
const CMakeVars::CMakeVarProvider& var_provider,
1275-
Triplet host_triplet)
1284+
Triplet host_triplet,
1285+
const Path& packages_dir)
12761286
: m_ver_provider(ver_provider)
12771287
, m_base_provider(base_provider)
12781288
, m_o_provider(oprovider)
12791289
, m_var_provider(var_provider)
12801290
, m_host_triplet(host_triplet)
1291+
, m_packages_dir(packages_dir)
12811292
{
12821293
}
12831294

@@ -1294,6 +1305,7 @@ namespace vcpkg
12941305
const IOverlayProvider& m_o_provider;
12951306
const CMakeVars::CMakeVarProvider& m_var_provider;
12961307
const Triplet m_host_triplet;
1308+
const Path m_packages_dir;
12971309

12981310
struct DepSpec
12991311
{
@@ -1784,6 +1796,7 @@ namespace vcpkg
17841796
: RequestType::AUTO_SELECTED;
17851797
InstallPlanAction ipa(dep.spec,
17861798
*node.second.scfl,
1799+
m_packages_dir,
17871800
request,
17881801
m_host_triplet,
17891802
compute_feature_dependencies(node, deps),
@@ -1901,24 +1914,14 @@ namespace vcpkg
19011914
const PackageSpec& toplevel,
19021915
const CreateInstallPlanOptions& options)
19031916
{
1904-
VersionedPackageGraph vpg(provider, bprovider, oprovider, var_provider, options.host_triplet);
1917+
VersionedPackageGraph vpg(
1918+
provider, bprovider, oprovider, var_provider, options.host_triplet, options.packages_dir);
19051919
for (auto&& o : overrides)
19061920
{
19071921
vpg.add_override(o.name, {o.version, o.port_version});
19081922
}
19091923

19101924
vpg.solve_with_roots(deps, toplevel);
1911-
auto ret = vpg.finalize_extract_plan(toplevel, options.unsupported_port_action);
1912-
if (auto plan = ret.get())
1913-
{
1914-
for (auto&& action : plan->install_actions)
1915-
{
1916-
if (action.source_control_file_and_location.has_value())
1917-
{
1918-
action.package_dir.emplace(options.packages_dir / action.spec.dir());
1919-
}
1920-
}
1921-
}
1922-
return ret;
1925+
return vpg.finalize_extract_plan(toplevel, options.unsupported_port_action);
19231926
}
19241927
}

‎src/vcpkg/postbuildlint.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,11 +353,11 @@ namespace vcpkg
353353

354354
static LintStatus check_for_copyright_file(const ReadOnlyFilesystem& fs,
355355
const PackageSpec& spec,
356+
const Path& package_dir,
356357
const VcpkgPaths& paths,
357358
MessageSink& msg_sink)
358359
{
359-
const auto packages_dir = paths.package_dir(spec);
360-
const auto copyright_file = packages_dir / "share" / spec.name() / "copyright";
360+
const auto copyright_file = package_dir / "share" / spec.name() / "copyright";
361361

362362
switch (fs.status(copyright_file, IgnoreErrors{}))
363363
{
@@ -1364,7 +1364,7 @@ namespace vcpkg
13641364
error_count += check_folder_debug_lib_cmake(fs, package_dir, spec, msg_sink);
13651365
error_count += check_for_dlls_in_lib_dir(fs, package_dir, msg_sink);
13661366
error_count += check_for_dlls_in_lib_dir(fs, package_dir / "debug", msg_sink);
1367-
error_count += check_for_copyright_file(fs, spec, paths, msg_sink);
1367+
error_count += check_for_copyright_file(fs, spec, package_dir, paths, msg_sink);
13681368
error_count += check_for_exes(fs, package_dir, msg_sink);
13691369
error_count += check_for_exes(fs, package_dir / "debug", msg_sink);
13701370
error_count += check_for_usage_forgot_install(fs, port_dir, package_dir, spec, msg_sink);

0 commit comments

Comments
 (0)