Merge topic 'cmake-server-handshake-improvements'

42ccbee1 server-mode: Handle generator toolset and platform in handshake
d792491c cmake-server: Better error reporting during handshake

Help/manual/cmake-server.7.rst

@@ -276,7 +276,9 @@ Protocol version 1.0 requires the following attributes to be set:
   * "sourceDirectory" with a path to the sources
   * "buildDirectory" with a path to the build directory
   * "generator" with the generator name
-  * "extraGenerator" (optional!) with the extra generator to be used.
+  * "extraGenerator" (optional!) with the extra generator to be used
+  * "platform" with the generator platform (if supported by the generator)
+  * "toolset" with the generator toolset (if supported by the generator)
 
 Example::
 

Source/cmServerDictionary.h

@@ -62,6 +62,7 @@ static const std::string kMESSAGE_KEY = "message";
 static const std::string kMINOR_KEY = "minor";
 static const std::string kNAME_KEY = "name";
 static const std::string kPATH_KEY = "path";
+static const std::string kPLATFORM_KEY = "platform";
 static const std::string kPROGRESS_CURRENT_KEY = "progressCurrent";
 static const std::string kPROGRESS_MAXIMUM_KEY = "progressMaximum";
 static const std::string kPROGRESS_MESSAGE_KEY = "progressMessage";
@@ -77,6 +78,7 @@ static const std::string kSUPPORTED_PROTOCOL_VERSIONS =
 static const std::string kSYSROOT_KEY = "sysroot";
 static const std::string kTARGETS_KEY = "targets";
 static const std::string kTITLE_KEY = "title";
+static const std::string kTOOLSET_KEY = "toolset";
 static const std::string kTRACE_EXPAND_KEY = "traceExpand";
 static const std::string kTRACE_KEY = "trace";
 static const std::string kTYPE_KEY = "type";

Source/cmServerProtocol.cxx

@@ -249,6 +249,30 @@ std::pair<int, int> cmServerProtocol1_0::ProtocolVersion() const
   return std::make_pair(1, 0);
 }
 
+static void setErrorMessage(std::string* errorMessage, const std::string& text)
+{
+  if (errorMessage) {
+    *errorMessage = text;
+  }
+}
+
+static bool testValue(cmState* state, const std::string& key,
+                      std::string& value, const std::string& keyDescription,
+                      std::string* errorMessage)
+{
+  const std::string cachedValue = std::string(state->GetCacheEntryValue(key));
+  if (!cachedValue.empty() && !value.empty() && cachedValue != value) {
+    setErrorMessage(errorMessage, std::string("\"") + key +
+                      "\" is set but incompatible with configured " +
+                      keyDescription + " value.");
+    return false;
+  }
+  if (value.empty()) {
+    value = cachedValue;
+  }
+  return true;
+}
+
 bool cmServerProtocol1_0::DoActivate(const cmServerRequest& request,
                                      std::string* errorMessage)
 {
@@ -257,21 +281,20 @@ bool cmServerProtocol1_0::DoActivate(const cmServerRequest& request,
     request.Data[kBUILD_DIRECTORY_KEY].asString();
   std::string generator = request.Data[kGENERATOR_KEY].asString();
   std::string extraGenerator = request.Data[kEXTRA_GENERATOR_KEY].asString();
+  std::string toolset = request.Data[kTOOLSET_KEY].asString();
+  std::string platform = request.Data[kPLATFORM_KEY].asString();
 
   if (buildDirectory.empty()) {
-    if (errorMessage) {
-      *errorMessage =
-        std::string("\"") + kBUILD_DIRECTORY_KEY + "\" is missing.";
-    }
+    setErrorMessage(errorMessage, std::string("\"") + kBUILD_DIRECTORY_KEY +
+                      "\" is missing.");
     return false;
   }
+
   cmake* cm = CMakeInstance();
   if (cmSystemTools::PathExists(buildDirectory)) {
     if (!cmSystemTools::FileIsDirectory(buildDirectory)) {
-      if (errorMessage) {
-        *errorMessage = std::string("\"") + kBUILD_DIRECTORY_KEY +
-          "\" exists but is not a directory.";
-      }
+      setErrorMessage(errorMessage, std::string("\"") + kBUILD_DIRECTORY_KEY +
+                        "\" exists but is not a directory.");
       return false;
     }
 
@@ -280,77 +303,86 @@ bool cmServerProtocol1_0::DoActivate(const cmServerRequest& request,
       cmState* state = cm->GetState();
 
       // Check generator:
-      const std::string cachedGenerator =
-        std::string(state->GetCacheEntryValue("CMAKE_GENERATOR"));
-      if (cachedGenerator.empty() && generator.empty()) {
-        if (errorMessage) {
-          *errorMessage =
-            std::string("\"") + kGENERATOR_KEY + "\" is required but unset.";
-        }
-        return false;
-      }
-      if (generator.empty()) {
-        generator = cachedGenerator;
-      }
-      if (generator != cachedGenerator) {
-        if (errorMessage) {
-          *errorMessage = std::string("\"") + kGENERATOR_KEY +
-            "\" set but incompatible with configured generator.";
-        }
+      if (!testValue(state, "CMAKE_GENERATOR", generator, "generator",
+                     errorMessage)) {
         return false;
       }
 
       // check extra generator:
-      const std::string cachedExtraGenerator =
-        std::string(state->GetCacheEntryValue("CMAKE_EXTRA_GENERATOR"));
-      if (!cachedExtraGenerator.empty() && !extraGenerator.empty() &&
-          cachedExtraGenerator != extraGenerator) {
-        if (errorMessage) {
-          *errorMessage = std::string("\"") + kEXTRA_GENERATOR_KEY +
-            "\" is set but incompatible with configured extra generator.";
-        }
+      if (!testValue(state, "CMAKE_EXTRA_GENERATOR", extraGenerator,
+                     "extra generator", errorMessage)) {
         return false;
       }
-      if (extraGenerator.empty()) {
-        extraGenerator = cachedExtraGenerator;
-      }
 
       // check sourcedir:
-      const std::string cachedSourceDirectory =
-        std::string(state->GetCacheEntryValue("CMAKE_HOME_DIRECTORY"));
-      if (!cachedSourceDirectory.empty() && !sourceDirectory.empty() &&
-          cachedSourceDirectory != sourceDirectory) {
-        if (errorMessage) {
-          *errorMessage = std::string("\"") + kSOURCE_DIRECTORY_KEY +
-            "\" is set but incompatible with configured source directory.";
-        }
+      if (!testValue(state, "CMAKE_HOME_DIRECTORY", sourceDirectory,
+                     "source directory", errorMessage)) {
         return false;
       }
-      if (sourceDirectory.empty()) {
-        sourceDirectory = cachedSourceDirectory;
+
+      // check toolset:
+      if (!testValue(state, "CMAKE_GENERATOR_TOOLSET", toolset, "toolset",
+                     errorMessage)) {
+        return false;
+      }
+
+      // check platform:
+      if (!testValue(state, "CMAKE_GENERATOR_PLATFORM", platform, "platform",
+                     errorMessage)) {
+        return false;
       }
     }
   }
 
   if (sourceDirectory.empty()) {
-    if (errorMessage) {
-      *errorMessage = std::string("\"") + kSOURCE_DIRECTORY_KEY +
-        "\" is unset but required.";
-    }
+    setErrorMessage(errorMessage, std::string("\"") + kSOURCE_DIRECTORY_KEY +
+                      "\" is unset but required.");
     return false;
   }
   if (!cmSystemTools::FileIsDirectory(sourceDirectory)) {
-    if (errorMessage) {
-      *errorMessage =
-        std::string("\"") + kSOURCE_DIRECTORY_KEY + "\" is not a directory.";
-    }
+    setErrorMessage(errorMessage, std::string("\"") + kSOURCE_DIRECTORY_KEY +
+                      "\" is not a directory.");
     return false;
   }
   if (generator.empty()) {
-    if (errorMessage) {
-      *errorMessage =
-        std::string("\"") + kGENERATOR_KEY + "\" is unset but required.";
-    }
+    setErrorMessage(errorMessage, std::string("\"") + kGENERATOR_KEY +
+                      "\" is unset but required.");
+    return false;
+  }
+
+  std::vector<cmake::GeneratorInfo> generators;
+  cm->GetRegisteredGenerators(generators);
+  auto baseIt = std::find_if(generators.begin(), generators.end(),
+                             [&generator](const cmake::GeneratorInfo& info) {
+                               return info.name == generator;
+                             });
+  if (baseIt == generators.end()) {
+    setErrorMessage(errorMessage, std::string("Generator \"") + generator +
+                      "\" not supported.");
+    return false;
+  }
+  auto extraIt = std::find_if(
+    generators.begin(), generators.end(),
+    [&generator, &extraGenerator](const cmake::GeneratorInfo& info) {
+      return info.baseName == generator && info.extraName == extraGenerator;
+    });
+  if (extraIt == generators.end()) {
+    setErrorMessage(errorMessage,
+                    std::string("The combination of generator \"" + generator +
+                                "\" and extra generator \"" + extraGenerator +
+                                "\" is not supported."));
+    return false;
+  }
+  if (!extraIt->supportsToolset && !toolset.empty()) {
+    setErrorMessage(errorMessage,
+                    std::string("Toolset was provided but is not supported by "
+                                "the requested generator."));
+    return false;
+  }
+  if (!extraIt->supportsPlatform && !platform.empty()) {
+    setErrorMessage(errorMessage,
+                    std::string("Platform was provided but is not supported "
+                                "by the requested generator."));
     return false;
   }
 
@@ -358,13 +390,15 @@ bool cmServerProtocol1_0::DoActivate(const cmServerRequest& request,
     cmExternalMakefileProjectGenerator::CreateFullGeneratorName(
       generator, extraGenerator);
 
+  cm->SetGeneratorToolset(toolset);
+  cm->SetGeneratorPlatform(platform);
+
   cmGlobalGenerator* gg = cm->CreateGlobalGenerator(fullGeneratorName);
   if (!gg) {
-    if (errorMessage) {
-      *errorMessage =
-        std::string("Could not set up the requested combination of \"") +
-        kGENERATOR_KEY + "\" and \"" + kEXTRA_GENERATOR_KEY + "\"";
-    }
+    setErrorMessage(
+      errorMessage,
+      std::string("Could not set up the requested combination of \"") +
+        kGENERATOR_KEY + "\" and \"" + kEXTRA_GENERATOR_KEY + "\"");
     return false;
   }
 

Tests/Server/tc_handshake.json

@@ -59,10 +59,10 @@
 { "recv": {"cookie":"zimtstern","inReplyTo":"handshake","type":"error","errorMessage":"Failed to activate protocol version: \"generator\" is unset but required."} },
 
 { "send": {"cookie":"zimtstern","type": "handshake","protocolVersion":{"major":1},"sourceDirectory":".","buildDirectory":"/tmp/build","generator":"XXXX","extraGenerator":"CodeBlocks"} },
-{ "recv": {"cookie":"zimtstern","inReplyTo":"handshake","type":"error","errorMessage":"Failed to activate protocol version: Could not set up the requested combination of \"generator\" and \"extraGenerator\""} },
+{ "recv": {"cookie":"zimtstern","inReplyTo":"handshake","type":"error","errorMessage":"Failed to activate protocol version: Generator \"XXXX\" not supported."} },
 
 { "send": {"cookie":"zimtstern","type": "handshake","protocolVersion":{"major":1},"sourceDirectory":".","buildDirectory":"/tmp/build","generator":"Ninja","extraGenerator":"XXXX"} },
-{ "recv": {"cookie":"zimtstern","inReplyTo":"handshake","type":"error","errorMessage":"Failed to activate protocol version: Could not set up the requested combination of \"generator\" and \"extraGenerator\""} },
+{ "recv": {"cookie":"zimtstern","inReplyTo":"handshake","type":"error","errorMessage":"Failed to activate protocol version: The combination of generator \"Ninja\" and extra generator \"XXXX\" is not supported."} },
 
 { "send": {"cookie":"zimtstern","type": "handshake","protocolVersion":{"major":1},"sourceDirectory":".","buildDirectory":"/tmp/build","generator":"Ninja","extraGenerator":"CodeBlocks"} },
 { "recv": {"cookie":"zimtstern","inReplyTo":"handshake","type":"reply"} },