mirror of
https://github.com/Kitware/CMake.git
synced 2026-01-06 13:51:33 -06:00
cmExportCommand: Fix PACKAGE_INFO argument parsing
While I'm unsure about why `cmPackageInfoArguments` was originally written the way it was, in its current form, the way command sub-parsers work, the parser never considers arguments associated with a sub-parser if the sub-parser keyword isn't present. This means that the arguments associated with `cmPackageInfoArguments` are treated as unknown, and the logic to reject them being set if `PACKAGE_INFO` is not present can never actually execute. Therefore, remove it, and remove the associated (and effectively useless) `enable` argument to its `Check` method. Instead, ensure that the package name is actually specified. The only case in which the parser will create the `optional` associated with the sub-parser arguments is if the relevant keyword (i.e. `PACKAGE_INFO`) is present. However, while the associated value is `NonEmpty`, the way we are using the parser does not actually enforce this, and it looks like a correct fix may be a breaking change. Therefore, enforce it manually for now.
This commit is contained in:
@@ -250,22 +250,21 @@ static bool HandleExportMode(std::vector<std::string> const& args,
|
||||
|
||||
if (arguments.PackageInfo) {
|
||||
if (arguments.PackageInfo->PackageName.empty()) {
|
||||
if (!arguments.PackageInfo->Check(status, false)) {
|
||||
return false;
|
||||
}
|
||||
} else {
|
||||
if (!arguments.Filename.empty()) {
|
||||
status.SetError("PACKAGE_INFO and FILE are mutually exclusive.");
|
||||
return false;
|
||||
}
|
||||
if (!arguments.Namespace.empty()) {
|
||||
status.SetError("PACKAGE_INFO and NAMESPACE are mutually exclusive.");
|
||||
return false;
|
||||
}
|
||||
if (!arguments.PackageInfo->Check(status) ||
|
||||
!arguments.PackageInfo->SetMetadataFromProject(status)) {
|
||||
return false;
|
||||
}
|
||||
// TODO: Fix our use of the parser to enforce this.
|
||||
status.SetError("PACKAGE_INFO missing required value.");
|
||||
return false;
|
||||
}
|
||||
if (!arguments.Filename.empty()) {
|
||||
status.SetError("PACKAGE_INFO and FILE are mutually exclusive.");
|
||||
return false;
|
||||
}
|
||||
if (!arguments.Namespace.empty()) {
|
||||
status.SetError("PACKAGE_INFO and NAMESPACE are mutually exclusive.");
|
||||
return false;
|
||||
}
|
||||
if (!arguments.PackageInfo->Check(status) ||
|
||||
!arguments.PackageInfo->SetMetadataFromProject(status)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2184,6 +2184,12 @@ bool HandlePackageInfoMode(std::vector<std::string> const& args,
|
||||
return false;
|
||||
}
|
||||
|
||||
if (arguments.PackageName.empty()) {
|
||||
// TODO: Fix our use of the parser to enforce this.
|
||||
status.SetError(cmStrCat(args[0], " missing package name."));
|
||||
return false;
|
||||
}
|
||||
|
||||
if (exportName.empty()) {
|
||||
status.SetError(cmStrCat(args[0], " missing EXPORT."));
|
||||
return false;
|
||||
|
||||
@@ -19,11 +19,6 @@ template void cmPackageInfoArguments::Bind<void>(cmArgumentParser<void>&,
|
||||
|
||||
namespace {
|
||||
|
||||
bool ArgWasSpecified(bool value)
|
||||
{
|
||||
return value;
|
||||
}
|
||||
|
||||
bool ArgWasSpecified(std::string const& value)
|
||||
{
|
||||
return !value.empty();
|
||||
@@ -52,26 +47,8 @@ bool ArgWasSpecified(std::vector<std::string> const& value)
|
||||
} \
|
||||
} while (false)
|
||||
|
||||
bool cmPackageInfoArguments::Check(cmExecutionStatus& status,
|
||||
bool enable) const
|
||||
bool cmPackageInfoArguments::Check(cmExecutionStatus& status) const
|
||||
{
|
||||
if (!enable) {
|
||||
// Check if any options were given.
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->LowerCase, "LOWER_CASE_FILE");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->Appendix, "APPENDIX");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->Version, "VERSION");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->License, "LICENSE");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->DefaultLicense, "DEFAULT_LICENSE");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->Description, "DESCRIPTION");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->Website, "HOMEPAGE_URL");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->DefaultTargets, "DEFAULT_TARGETS");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->DefaultConfigs,
|
||||
"DEFAULT_CONFIGURATIONS");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->ProjectName, "PROJECT");
|
||||
ENFORCE_REQUIRES("PACKAGE_INFO", this->NoProjectDefaults,
|
||||
"NO_PROJECT_METADATA");
|
||||
}
|
||||
|
||||
// Check for incompatible options.
|
||||
if (!this->Appendix.empty()) {
|
||||
ENFORCE_EXCLUSIVE("APPENDIX", this->Version, "VERSION");
|
||||
|
||||
@@ -43,9 +43,8 @@ public:
|
||||
std::string GetPackageDirName() const;
|
||||
std::string GetPackageFileName() const;
|
||||
|
||||
/// Ensure that no conflicting options were specified. If \p enable is
|
||||
/// \c false, forbid specifying any options whatsoever.
|
||||
bool Check(cmExecutionStatus& status, bool enable = true) const;
|
||||
/// Ensure that no conflicting options were specified.
|
||||
bool Check(cmExecutionStatus& status) const;
|
||||
|
||||
/// Set metadata (not already specified) from either the specified project,
|
||||
/// or from the project which matches the package name.
|
||||
|
||||
Reference in New Issue
Block a user