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.

Source/cmExportCommand.cxx

@@ -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;
     }
   }
 

Source/cmInstallCommand.cxx

@@ -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;

Source/cmPackageInfoArguments.cxx

@@ -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");

Source/cmPackageInfoArguments.h

@@ -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.

Tests/RunCMake/ExportPackageInfo/BadArgs0-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/ExportPackageInfo/BadArgs0-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at BadArgs0\.cmake:3 \(export\):
+  export PACKAGE_INFO missing required value\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:3 \(include\)

Tests/RunCMake/ExportPackageInfo/BadArgs0.cmake

@@ -0,0 +1,3 @@
+add_library(foo INTERFACE)
+install(TARGETS foo EXPORT foo DESTINATION .)
+export(EXPORT foo PACKAGE_INFO)

Tests/RunCMake/ExportPackageInfo/RunCMakeTest.cmake

@@ -12,6 +12,7 @@ set(RunCMake_TEST_OPTIONS
   )
 
 # Test incorrect usage
+run_cmake(BadArgs0)
 run_cmake(BadArgs1)
 run_cmake(BadArgs2)
 run_cmake(BadArgs3)

Tests/RunCMake/InstallPackageInfo/BadArgs0-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/InstallPackageInfo/BadArgs0-stderr.txt

@@ -0,0 +1,22 @@
+CMake Error at BadArgs0\.cmake:1 \(install\):
+  install PACKAGE_INFO missing package name\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:3 \(include\)
+
+
+CMake Error at BadArgs0\.cmake:2 \(install\):
+  install PACKAGE_INFO missing EXPORT\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:3 \(include\)
+
+
+CMake Error at BadArgs0\.cmake:3 \(install\):
+  install PACKAGE_INFO missing EXPORT\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:3 \(include\)
+
+
+CMake Error at BadArgs0\.cmake:7 \(install\):
+  install PACKAGE_INFO missing package name\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:3 \(include\)

Tests/RunCMake/InstallPackageInfo/BadArgs0.cmake

@@ -0,0 +1,7 @@
+install(PACKAGE_INFO)
+install(PACKAGE_INFO test)
+install(PACKAGE_INFO test EXPORT)
+
+add_library(foo INTERFACE)
+install(TARGETS foo EXPORT foo DESTINATION .)
+install(PACKAGE_INFO EXPORT foo)

Tests/RunCMake/InstallPackageInfo/RunCMakeTest.cmake

@@ -25,6 +25,7 @@ function(run_cmake_install test)
 endfunction()
 
 # Test incorrect usage
+run_cmake(BadArgs0)
 run_cmake(BadArgs1)
 run_cmake(BadArgs2)
 run_cmake(BadName)