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

Warn on classic mode build failure if version constraints weren't met #494

Merged
Merged
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 @@
set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "classic-versions-a",
"version": "1"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
message(FATAL_ERROR "FATAL_ERROR")
10 changes: 10 additions & 0 deletions azure-pipelines/e2e_ports/overlays/classic-versions-b/vcpkg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "classic-versions-b",
"version": "1",
"dependencies": [
{
"name": "classic-versions-a",
"version>=": "2"
}
]
}
10 changes: 10 additions & 0 deletions azure-pipelines/end-to-end-tests-dir/classic-versions.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
. $PSScriptRoot/../end-to-end-tests-prelude.ps1

# Not a number
$out = Run-Vcpkg @commonArgs install classic-versions-b | Out-String
Throw-IfNotFailed
if ($out -notmatch ".*warning:.*dependency classic-versions-a.*at least version 2.*is currently 1.*")
{
$out
throw "Expected to fail and print warning about mismatched versions"
}
2 changes: 2 additions & 0 deletions include/vcpkg/binaryparagraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ namespace vcpkg

bool is_feature() const { return !feature.empty(); }

Version get_version() const { return {version, port_version}; }

PackageSpec spec;
std::string version;
int port_version = 0;
Expand Down
4 changes: 3 additions & 1 deletion include/vcpkg/dependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ namespace vcpkg::Dependencies
const SourceControlFileAndLocation& scfl,
const RequestType& request_type,
Triplet host_triplet,
std::map<std::string, std::vector<FeatureSpec>>&& dependencies);
std::map<std::string, std::vector<FeatureSpec>>&& dependencies,
std::vector<LocalizedString>&& build_failure_messages);

std::string displayname() const;
const std::string& public_abi() const;
Expand All @@ -86,6 +87,7 @@ namespace vcpkg::Dependencies

std::map<std::string, std::vector<FeatureSpec>> feature_dependencies;
std::vector<PackageSpec> package_dependencies;
std::vector<LocalizedString> build_failure_messages;
InternalFeatureSet feature_list;
Triplet host_triplet;

Expand Down
6 changes: 6 additions & 0 deletions include/vcpkg/versions.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ namespace vcpkg

VerComp compare(const DateVersion& a, const DateVersion& b);

// Try parsing with all version schemas and return 'unk' if none match
VerComp compare_any(const Version& a, const Version& b);

VerComp compare_versions(VersionScheme sa, const Version& a, VersionScheme sb, const Version& b);

enum class VersionConstraintKind
{
None,
Expand All @@ -158,3 +163,4 @@ namespace vcpkg
}

VCPKG_FORMAT_WITH_TO_STRING(vcpkg::VersionSpec);
VCPKG_FORMAT_WITH_TO_STRING(vcpkg::Version);
1 change: 1 addition & 0 deletions locales/messages.en.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
"VcpkgInvalidCommand": "invalid command: {command_name}",
"VcpkgSendMetricsButDisabled": "Warning: passed --sendmetrics, but metrics are disabled.",
"VersionCommandHeader": "vcpkg package management program version {version}\n\nSee LICENSE.txt for license information.",
"VersionConstraintViolated": "dependency {spec} was expected to be at least version {expected_version}, but is currently {actual_version}.",
"VersionInvalidDate": "`{version}` is not a valid date version. Dates must follow the format YYYY-MM-DD and disambiguators must be dot-separated positive integer values without leading zeroes.",
"VersionInvalidRelaxed": "`{version}` is not a valid relaxed version (semver with arbitrary numeric element count).",
"VersionInvalidSemver": "`{version}` is not a valid semantic version, consult <https://semver.org>.",
Expand Down
2 changes: 2 additions & 0 deletions locales/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@
"VcpkgSendMetricsButDisabled": "Warning: passed --sendmetrics, but metrics are disabled.",
"VersionCommandHeader": "vcpkg package management program version {version}\n\nSee LICENSE.txt for license information.",
"_VersionCommandHeader.comment": "example of {version} is '1.3.8'.\n",
"VersionConstraintViolated": "dependency {spec} was expected to be at least version {expected_version}, but is currently {actual_version}.",
"_VersionConstraintViolated.comment": "example of {spec} is 'zlib:x64-windows'.\nexample of {expected_version} is '1.3.8'.\nexample of {actual_version} is '1.3.8'.\n",
"VersionInvalidDate": "`{version}` is not a valid date version. Dates must follow the format YYYY-MM-DD and disambiguators must be dot-separated positive integer values without leading zeroes.",
"_VersionInvalidDate.comment": "example of {version} is '1.3.8'.\n",
"VersionInvalidRelaxed": "`{version}` is not a valid relaxed version (semver with arbitrary numeric element count).",
Expand Down
6 changes: 4 additions & 2 deletions src/vcpkg-test/binarycaching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ Build-Depends: bzip
scfl,
Dependencies::RequestType::USER_REQUESTED,
Test::ARM_UWP,
{{"a", {}}, {"b", {}}});
{{"a", {}}, {"b", {}}},
{});

ipa.abi_info = Build::AbiInfo{};
ipa.abi_info.get()->package_abi = "packageabi";
Expand Down Expand Up @@ -390,7 +391,8 @@ Version: 1.5
scfl,
Dependencies::RequestType::USER_REQUESTED,
Test::ARM_UWP,
std::map<std::string, std::vector<FeatureSpec>>{});
std::map<std::string, std::vector<FeatureSpec>>{},
std::vector<LocalizedString>{});
Dependencies::InstallPlanAction& ipa_without_abi = install_plan.back();

// test that the binary cache does the right thing. See also CHECKs etc. in KnowNothingBinaryProvider
Expand Down
39 changes: 39 additions & 0 deletions src/vcpkg-test/dependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,45 @@ TEST_CASE ("version sort date", "[versionplan]")
CHECK(versions[8].original_string == "2021-01-01.10");
}

TEST_CASE ("version compare string", "[versionplan]")
{
const Version a_0("a", 0);
const Version a_1("a", 1);
const Version b_1("b", 1);
CHECK(VerComp::lt == compare_versions(VersionScheme::String, a_0, VersionScheme::String, a_1));
CHECK(VerComp::eq == compare_versions(VersionScheme::String, a_0, VersionScheme::String, a_0));
CHECK(VerComp::gt == compare_versions(VersionScheme::String, a_1, VersionScheme::String, a_0));
CHECK(VerComp::unk == compare_versions(VersionScheme::String, a_1, VersionScheme::String, b_1));
}

TEST_CASE ("version compare_any", "[versionplan]")
{
const Version a_0("a", 0);
const Version a_1("a", 1);
const Version b_1("b", 1);
CHECK(VerComp::lt == compare_any(a_0, a_1));
CHECK(VerComp::gt == compare_any(a_1, a_0));
CHECK(VerComp::eq == compare_any(a_0, a_0));
CHECK(VerComp::unk == compare_any(a_1, b_1));

const Version v_0_0("0", 0);
const Version v_1_0("1", 0);
const Version v_1_1_1("1.1", 1);
CHECK(VerComp::lt == compare_any(v_0_0, v_1_0));
CHECK(VerComp::gt == compare_any(v_1_1_1, v_1_0));
CHECK(VerComp::eq == compare_any(v_0_0, v_0_0));

const Version date_0("2021-04-05", 0);
const Version date_1("2022-02-01", 0);
CHECK(VerComp::eq == compare_any(date_0, date_0));
CHECK(VerComp::lt == compare_any(date_0, date_1));

CHECK(VerComp::unk == compare_any(date_0, a_0));
// Note: dates are valid relaxed dotversions, so these are valid comparisons
CHECK(VerComp::gt == compare_any(date_0, v_0_0));
CHECK(VerComp::gt == compare_any(date_0, v_1_1_1));
}

TEST_CASE ("version install simple semver", "[versionplan]")
{
MockBaselineProvider bp;
Expand Down
6 changes: 3 additions & 3 deletions src/vcpkg-test/spdx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ TEST_CASE ("spdx maximum serialization", "[spdx]")
cpgh.raw_version = "1.0";
cpgh.version_scheme = VersionScheme::Relaxed;

InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {});
InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
auto& abi = *(ipa.abi_info = Build::AbiInfo{}).get();
abi.package_abi = "ABIHASH";

Expand Down Expand Up @@ -178,7 +178,7 @@ TEST_CASE ("spdx minimum serialization", "[spdx]")
cpgh.raw_version = "1.0";
cpgh.version_scheme = VersionScheme::String;

InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {});
InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
auto& abi = *(ipa.abi_info = Build::AbiInfo{}).get();
abi.package_abi = "deadbeef";

Expand Down Expand Up @@ -307,7 +307,7 @@ TEST_CASE ("spdx concat resources", "[spdx]")
cpgh.raw_version = "1.0";
cpgh.version_scheme = VersionScheme::String;

InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {});
InstallPlanAction ipa(spec, scfl, RequestType::USER_REQUESTED, Test::X86_WINDOWS, {}, {});
auto& abi = *(ipa.abi_info = Build::AbiInfo{}).get();
abi.package_abi = "deadbeef";

Expand Down
11 changes: 10 additions & 1 deletion src/vcpkg/build.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,16 @@ namespace vcpkg::Build

if (result.code != BuildResult::SUCCEEDED)
{
print2(Color::error, Build::create_error_message(result, spec), '\n');
LocalizedString warnings;
for (auto&& msg : action->build_failure_messages)
{
warnings.append(msg).appendnl();
}
if (!warnings.data().empty())
{
msg::print(Color::warning, warnings);
}
msg::println(Color::error, Build::create_error_message(result, spec));
print2(Build::create_user_troubleshooting_message(*action, paths), '\n');
return 1;
}
Expand Down
112 changes: 67 additions & 45 deletions src/vcpkg/dependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ namespace vcpkg::Dependencies
{
namespace
{
DECLARE_AND_REGISTER_MESSAGE(VersionConstraintViolated,
(msg::spec, msg::expected_version, msg::actual_version),
"",
"dependency {spec} was expected to be at least version "
"{expected_version}, but is currently {actual_version}.");

struct ClusterGraph;

struct ClusterInstalled
Expand All @@ -44,6 +50,7 @@ namespace vcpkg::Dependencies
struct ClusterInstallInfo
{
std::map<std::string, std::vector<FeatureSpec>> build_edges;
std::map<PackageSpec, std::set<Version, VersionMapLess>> version_constraints;
bool defaults_requested = false;
};

Expand Down Expand Up @@ -127,12 +134,18 @@ namespace vcpkg::Dependencies
if (auto vars = maybe_vars.get())
{
// Qualified dependency resolution is available
auto fullspec_list = filter_dependencies(
*qualified_deps, m_spec.triplet(), host_triplet, *vars, ImplicitDefault::YES);

for (auto&& fspec : fullspec_list)
for (auto&& dep : *qualified_deps)
{
fspec.expand_fspecs_to(dep_list);
if (dep.platform.evaluate(*vars))
{
auto fullspec = dep.to_full_spec(m_spec.triplet(), host_triplet, ImplicitDefault::YES);
fullspec.expand_fspecs_to(dep_list);
if (auto opt = dep.constraint.try_get_minimum_version())
{
info.version_constraints[fullspec.package_spec].insert(
std::move(opt).value_or_exit(VCPKG_LINE_INFO));
}
}
}

Util::sort_unique_erase(dep_list);
Expand All @@ -145,8 +158,13 @@ namespace vcpkg::Dependencies
{
if (dep.platform.is_empty())
{
dep.to_full_spec(m_spec.triplet(), host_triplet, ImplicitDefault::YES)
.expand_fspecs_to(dep_list);
auto fullspec = dep.to_full_spec(m_spec.triplet(), host_triplet, ImplicitDefault::YES);
fullspec.expand_fspecs_to(dep_list);
if (auto opt = dep.constraint.try_get_minimum_version())
{
info.version_constraints[fullspec.package_spec].insert(
std::move(opt).value_or_exit(VCPKG_LINE_INFO));
}
}
else
{
Expand Down Expand Up @@ -253,6 +271,20 @@ namespace vcpkg::Dependencies
return nullopt;
}

Optional<Version> get_version() const
{
if (auto p_installed = m_installed.get())
{
return p_installed->ipv.core->package.get_version();
}
else if (auto p_scfl = m_scfl.get())
{
return p_scfl->to_version();
}
else
return nullopt;
}

PackageSpec m_spec;
ExpectedS<const SourceControlFileAndLocation&> m_scfl;

Expand Down Expand Up @@ -425,13 +457,15 @@ namespace vcpkg::Dependencies
const SourceControlFileAndLocation& scfl,
const RequestType& request_type,
Triplet host_triplet,
std::map<std::string, std::vector<FeatureSpec>>&& dependencies)
std::map<std::string, std::vector<FeatureSpec>>&& dependencies,
std::vector<LocalizedString>&& build_failure_messages)
: spec(spec)
, source_control_file_and_location(scfl)
, plan_type(InstallPlanType::BUILD_AND_INSTALL)
, request_type(request_type)
, build_options{}
, feature_dependencies(std::move(dependencies))
, build_failure_messages(std::move(build_failure_messages))
, host_triplet(host_triplet)
{
for (const auto& kv : feature_dependencies)
Expand Down Expand Up @@ -979,6 +1013,27 @@ namespace vcpkg::Dependencies
// If a cluster only has an installed object and is marked as user requested we should still report it.
if (auto info_ptr = p_cluster->m_install_info.get())
{
std::vector<LocalizedString> constraint_violations;
for (auto&& constraints : info_ptr->version_constraints)
{
for (auto&& constraint : constraints.second)
{
auto&& dep_clust = m_graph->get(constraints.first);
auto maybe_v = dep_clust.get_version();
if (auto v = maybe_v.get())
{
if (compare_any(*v, constraint) == VerComp::lt)
{
constraint_violations.push_back(msg::format(msg::msgWarningMessage)
.append(msgVersionConstraintViolated,
msg::spec = constraints.first,
msg::expected_version = constraint,
msg::actual_version = *v));
print2("found constraint violation: ", constraint_violations.back().data(), "\n");
}
}
}
}
std::map<std::string, std::vector<FeatureSpec>> computed_edges;
for (auto&& kv : info_ptr->build_edges)
{
Expand Down Expand Up @@ -1007,7 +1062,8 @@ namespace vcpkg::Dependencies
p_cluster->get_scfl_or_exit(),
p_cluster->request_type,
m_graph->m_host_triplet,
std::move(computed_edges));
std::move(computed_edges),
std::move(constraint_violations));
}
else if (p_cluster->request_type == RequestType::USER_REQUESTED && p_cluster->m_installed.has_value())
{
Expand Down Expand Up @@ -1382,41 +1438,6 @@ namespace vcpkg::Dependencies
return it == vermap.end() ? nullptr : it->second;
}

static VerComp compare_version_texts(VersionScheme sa, const Version& a, VersionScheme sb, const Version& b)
{
if (sa == VersionScheme::String && sb == VersionScheme::String)
{
return int_to_vercomp(a.text().compare(b.text()));
}

if (sa == VersionScheme::Date && sb == VersionScheme::Date)
{
return compare(DateVersion::try_parse(a.text()).value_or_exit(VCPKG_LINE_INFO),
DateVersion::try_parse(b.text()).value_or_exit(VCPKG_LINE_INFO));
}

if ((sa == VersionScheme::Semver || sa == VersionScheme::Relaxed) &&
(sb == VersionScheme::Semver || sb == VersionScheme::Relaxed))
{
return compare(DotVersion::try_parse(a.text(), sa).value_or_exit(VCPKG_LINE_INFO),
DotVersion::try_parse(b.text(), sb).value_or_exit(VCPKG_LINE_INFO));
}

return VerComp::unk;
}

static VerComp compare_versions(VersionScheme sa, const Version& a, VersionScheme sb, const Version& b)
{
const auto inner_compare = compare_version_texts(sa, a, sb, b);
if (inner_compare == VerComp::eq)
{
if (a.port_version() < b.port_version()) return VerComp::lt;
if (a.port_version() > b.port_version()) return VerComp::gt;
}

return inner_compare;
}

bool VersionedPackageGraph::VersionSchemeInfo::is_less_than(const Version& new_ver) const
{
Checks::check_exit(VCPKG_LINE_INFO, scfl);
Expand Down Expand Up @@ -1959,7 +1980,8 @@ namespace vcpkg::Dependencies
node.user_requested ? RequestType::USER_REQUESTED
: RequestType::AUTO_SELECTED,
m_host_triplet,
std::move(p_vnode->deps));
std::move(p_vnode->deps),
{});
std::vector<DepSpec> deps;
for (auto&& f : ipa.feature_list)
{
Expand Down
Loading