make custom command handling in vc(x)proj files sane

instead of glueing the commands with "&&" and having a monster hack
(\r\h) for the case where they cannot be and-ed, glue them with newlines
and error checks. this ensures that we a) never have excessively long
lines, b) don't need the monster hack and c) commands always have proper
error checks, just like in makefiles.

Reviewed-by: mariusSO

7 files changed, 50 insertions, 89 deletions

diff --git a/qmake/generators/win32/msbuild_objectmodel.cpp b/qmake/generators/win32/msbuild_objectmodel.cpp
index 2505056..c963f46 100644
--- a/qmake/generators/win32/msbuild_objectmodel.cpp
+++ b/qmake/generators/win32/msbuild_objectmodel.cpp
@@ -377,6 +377,16 @@ inline XmlOutput::xml_output valueTagT( const triState v)
 }
 
 
+static QString vcxCommandSeparator()
+{
+    // MSBuild puts the contents of the custom commands into a batch file and calls it.
+    // As we want every sub-command to be error-checked (as is done by makefile-based
+    // backends), we insert the checks ourselves, using the undocumented jump target.
+    static QString cmdSep =
+            QLatin1String("
if errorlevel 1 goto VCEnd
");
+    return cmdSep;
+}
+
 // VCXCLCompilerTool -------------------------------------------------
 VCXCLCompilerTool::VCXCLCompilerTool()
     :   BrowseInformation(_False),
@@ -2142,32 +2152,6 @@ VCXCustomBuildTool::VCXCustomBuildTool()
 
 XmlOutput &operator<<(XmlOutput &xml, const VCXCustomBuildTool &tool)
 {
-    // The code below offers two ways to split custom build step commands.
-    // Normally the $$escape_expand(\n\t) is used in a project file, which is correctly translated
-    // in all generators. However, if you use $$escape_expand(\n\r) (or \n\h) instead, the VCPROJ
-    // generator will instead of binding the commands with " && " will insert a proper newline into
-    // the VCPROJ file. We sometimes use this method of splitting commands if the custom buildstep
-    // contains a command-line which is too big to run on certain OS.
-    QString cmds;
-    int end = tool.CommandLine.count();
-    for(int i = 0; i < end; ++i) {
-        QString cmdl = tool.CommandLine.at(i);
-        if (cmdl.contains("\r\t")) {
-            if (i == end - 1)
-                cmdl = cmdl.trimmed();
-            cmdl.replace("\r\t", " && ");
-        } else if (cmdl.contains("\r\n")) {
-            ;
-        } else if (cmdl.contains("\r\\h")) {
-            // The above \r\n should work, but doesn't, so we have this hack
-            cmdl.replace("\r\\h", "\r\n");
-        } else {
-            if (i < end - 1)
-                cmdl += " && ";
-        }
-        cmds += cmdl;
-    }
-
     if ( !tool.AdditionalDependencies.isEmpty() )
     {
         xml << tag("AdditionalInputs")
@@ -2175,11 +2159,11 @@ XmlOutput &operator<<(XmlOutput &xml, const VCXCustomBuildTool &tool)
             << valueTagDefX(tool.AdditionalDependencies, "AdditionalInputs", ";");
     }
 
-    if( !cmds.isEmpty() )
+    if( !tool.CommandLine.isEmpty() )
     {
         xml << tag("Command")
             << attrTag("Condition", QString("'$(Configuration)|$(Platform)'=='%1'").arg(tool.ConfigName))
-            << valueTag(cmds);
+            << valueTag(tool.CommandLine.join(vcxCommandSeparator()));
     }
 
     if ( !tool.Description.isEmpty() )
@@ -2249,7 +2233,7 @@ XmlOutput &operator<<(XmlOutput &xml, const VCXEventTool &tool)
 {
     return xml
         << tag(tool.EventName)
-        << attrTagS(_Command, tool.CommandLine)
+        << attrTagS(_Command, tool.CommandLine.join(vcxCommandSeparator()))
         << attrTagS(_Message, tool.Description)
         << closetag(tool.EventName);
 }
@@ -2525,7 +2509,7 @@ bool VCXFilter::addExtraCompiler(const VCXFilterFile &info)
         if (!CustomBuildTool.Description.isEmpty())
             CustomBuildTool.Description += " & ";
         CustomBuildTool.Description += cmd_name;
-        CustomBuildTool.CommandLine += cmd.trimmed().split("\n", QString::SkipEmptyParts);
+        CustomBuildTool.CommandLine += VCToolBase::fixCommandLine(cmd.trimmed());
         int space = cmd.indexOf(' ');
         QFileInfo finf(cmd.left(space));
         if (CustomBuildTool.ToolPath.isEmpty())
diff --git a/qmake/generators/win32/msbuild_objectmodel.h b/qmake/generators/win32/msbuild_objectmodel.h
index 567985d..acca6c1 100644
--- a/qmake/generators/win32/msbuild_objectmodel.h
+++ b/qmake/generators/win32/msbuild_objectmodel.h
@@ -382,7 +382,7 @@ protected:
 
 public:
     // Variables
-    QString                 CommandLine;
+    QStringList             CommandLine;
     QString                 Description;
     triState                UseInBuild;
     QString                 EventName;
diff --git a/qmake/generators/win32/msvc_objectmodel.cpp b/qmake/generators/win32/msvc_objectmodel.cpp
index 980e686..df414ea 100644
--- a/qmake/generators/win32/msvc_objectmodel.cpp
+++ b/qmake/generators/win32/msvc_objectmodel.cpp
@@ -280,6 +280,24 @@ inline XmlOutput::xml_output attrX(const char *name, const QStringList &v, const
     return attr(name, v.join(s));
 }
 
+// VCToolBase -------------------------------------------------
+QStringList VCToolBase::fixCommandLine(const QString &input)
+{
+    // The splitting regexp is a bit bizarre for backwards compat reasons (why else ...).
+    return input.split(QRegExp(QLatin1String("\n\t|\r\\h|\r\n")));
+}
+
+static QString vcCommandSeparator()
+{
+    // MSVC transforms the build tree into a single batch file, simply pasting the contents
+    // of the custom commands into it, and putting an "if errorlevel goto" statement behind it.
+    // As we want every sub-command to be error-checked (as is done by makefile-based
+    // backends), we insert the checks ourselves, using the undocumented jump target.
+    static QString cmdSep =
+            QLatin1String("&#x000D;&#x000A;if errorlevel 1 goto VCReportError&#x000D;&#x000A;");
+    return cmdSep;
+}
+
 // VCCLCompilerTool -------------------------------------------------
 VCCLCompilerTool::VCCLCompilerTool()
     :        AssemblerOutput(asmListingNone),
@@ -1913,36 +1931,11 @@ VCCustomBuildTool::VCCustomBuildTool()
 
 XmlOutput &operator<<(XmlOutput &xml, const VCCustomBuildTool &tool)
 {
-    // The code below offers two ways to split custom build step commands.
-    // Normally the $$escape_expand(\n\t) is used in a project file, which is correctly translated
-    // in all generators. However, if you use $$escape_expand(\n\r) (or \n\h) instead, the VCPROJ
-    // generator will instead of binding the commands with " && " will insert a proper newline into
-    // the VCPROJ file. We sometimes use this method of splitting commands if the custom buildstep
-    // contains a command-line which is too big to run on certain OS.
-    QString cmds;
-    int end = tool.CommandLine.count();
-    for(int i = 0; i < end; ++i) {
-        QString cmdl = tool.CommandLine.at(i);
-        if (cmdl.contains("\r\t")) {
-            if (i == end - 1)
-                cmdl = cmdl.trimmed();
-            cmdl.replace("\r\t", " && ");
-        } else if (cmdl.contains("\r\n")) {
-            ;
-        } else if (cmdl.contains("\r\\h")) {
-            // The above \r\n should work, but doesn't, so we have this hack
-            cmdl.replace("\r\\h", "\r\n");
-        } else {
-            if (i < end - 1)
-                cmdl += " && ";
-        }
-        cmds += cmdl;
-    }
     return xml
         << tag(_Tool)
             << attrS(_Name, tool.ToolName)
             << attrX(_AdditionalDependencies, tool.AdditionalDependencies, ";")
-            << attrS(_CommandLine, cmds)
+            << attrS(_CommandLine, tool.CommandLine.join(vcCommandSeparator()))
             << attrS(_Description, tool.Description)
             << attrX(_Outputs, tool.Outputs, ";")
             << attrS(_Path, tool.ToolPath)
@@ -2002,7 +1995,7 @@ XmlOutput &operator<<(XmlOutput &xml, const VCEventTool &tool)
         << tag(_Tool)
             << attrS(_Name, tool.ToolName)
             << attrS(_Path, tool.ToolPath)
-            << attrS(_CommandLine, tool.CommandLine)
+            << attrS(_CommandLine, tool.CommandLine.join(vcCommandSeparator()))
             << attrS(_Description, tool.Description)
             << attrT(_ExcludedFromBuild, tool.ExcludedFromBuild)
         << closetag(_Tool);
@@ -2296,7 +2289,7 @@ bool VCFilter::addExtraCompiler(const VCFilterFile &info)
         if (!CustomBuildTool.Description.isEmpty())
             CustomBuildTool.Description += " & ";
         CustomBuildTool.Description += cmd_name;
-        CustomBuildTool.CommandLine += cmd.trimmed().split("\n", QString::SkipEmptyParts);
+        CustomBuildTool.CommandLine += VCToolBase::fixCommandLine(cmd.trimmed());
         int space = cmd.indexOf(' ');
         QFileInfo finf(cmd.left(space));
         if (CustomBuildTool.ToolPath.isEmpty())
diff --git a/qmake/generators/win32/msvc_objectmodel.h b/qmake/generators/win32/msvc_objectmodel.h
index 97f8570..22013e0 100644
--- a/qmake/generators/win32/msvc_objectmodel.h
+++ b/qmake/generators/win32/msvc_objectmodel.h
@@ -475,6 +475,7 @@ public:
         for (QStringList::ConstIterator it=options.begin(); (it!=options.end()); it++)
             parseOption((*it).toLatin1());
     }
+    static QStringList fixCommandLine(const QString &input);
 };
 
 class VCConfiguration;
@@ -746,7 +747,7 @@ protected:
 
 public:
     // Variables
-    QString                 CommandLine;
+    QStringList             CommandLine;
     QString                 Description;
     triState                ExcludedFromBuild;
     QString                 ToolName;
diff --git a/qmake/generators/win32/msvc_vcproj.cpp b/qmake/generators/win32/msvc_vcproj.cpp
index 8686ae8..cff7a14 100644
--- a/qmake/generators/win32/msvc_vcproj.cpp
+++ b/qmake/generators/win32/msvc_vcproj.cpp
@@ -1027,37 +1027,23 @@ void VcprojGenerator::initPreBuildEventTools()
 {
 }
 
-QString VcprojGenerator::fixCommandLine(DotNET version, const QString &input) const
-{
-    QString result = input;
-
-    if (version >= NET2005)
-        result = result.replace(QLatin1Char('\n'), QLatin1String("&#x000D;&#x000A;"));
-
-    return result;
-}
-
 void VcprojGenerator::initPostBuildEventTools()
 {
     VCConfiguration &conf = vcProject.Configuration;
     if(!project->values("QMAKE_POST_LINK").isEmpty()) {
-        QString cmdline = fixCommandLine(conf.CompilerVersion, var("QMAKE_POST_LINK"));
+        QStringList cmdline = VCToolBase::fixCommandLine(var("QMAKE_POST_LINK"));
         conf.postBuild.CommandLine = cmdline;
-        if (conf.CompilerVersion < NET2005)
-            cmdline = cmdline.replace("\n", "&&");
-        conf.postBuild.Description = cmdline;
+        conf.postBuild.Description = cmdline.join(QLatin1String("\r\n"));
     }
 
     QString signature = !project->isEmpty("SIGNATURE_FILE") ? var("SIGNATURE_FILE") : var("DEFAULT_SIGNATURE");
     bool useSignature = !signature.isEmpty() && !project->isActiveConfig("staticlib") &&
                         !project->isEmpty("CE_SDK") && !project->isEmpty("CE_ARCH");
     if(useSignature)
-        conf.postBuild.CommandLine.prepend(QLatin1String("signtool sign /F ") + signature + " \"$(TargetPath)\"\n" +
-            (!conf.postBuild.CommandLine.isEmpty() ? " && " : ""));
+        conf.postBuild.CommandLine.prepend(
+                QLatin1String("signtool sign /F ") + signature + QLatin1String(" \"$(TargetPath)\""));
 
     if(!project->values("MSVCPROJ_COPY_DLL").isEmpty()) {
-        if(!conf.postBuild.CommandLine.isEmpty())
-            conf.postBuild.CommandLine += " && ";
         conf.postBuild.Description += var("MSVCPROJ_COPY_DLL_DESC");
         conf.postBuild.CommandLine += var("MSVCPROJ_COPY_DLL");
     }
@@ -1186,9 +1172,9 @@ void VcprojGenerator::initPreLinkEventTools()
 {
     VCConfiguration &conf = vcProject.Configuration;
     if(!project->values("QMAKE_PRE_LINK").isEmpty()) {
-        QString cmdline = fixCommandLine(conf.CompilerVersion, var("QMAKE_PRE_LINK"));
-        conf.preLink.Description = cmdline;
+        QStringList cmdline = VCToolBase::fixCommandLine(var("QMAKE_PRE_LINK"));
         conf.preLink.CommandLine = cmdline;
+        conf.preLink.Description = cmdline.join(QLatin1String("\r\n"));
     }
 }
 
diff --git a/qmake/generators/win32/msvc_vcproj.h b/qmake/generators/win32/msvc_vcproj.h
index 8f028a1..05e765e 100644
--- a/qmake/generators/win32/msvc_vcproj.h
+++ b/qmake/generators/win32/msvc_vcproj.h
@@ -130,7 +130,6 @@ protected:
     QList<VcprojGenerator*> mergedProjects;
 
 private:
-    QString fixCommandLine(DotNET version, const QString &input) const;
     QUuid increaseUUID(const QUuid &id);
     friend class VCFilter;
 };
diff --git a/qmake/generators/win32/msvc_vcxproj.cpp b/qmake/generators/win32/msvc_vcxproj.cpp
index 271d9ae..396d2bf 100644
--- a/qmake/generators/win32/msvc_vcxproj.cpp
+++ b/qmake/generators/win32/msvc_vcxproj.cpp
@@ -380,9 +380,9 @@ void VcxprojGenerator::initPostBuildEventTools()
 {
     VCXConfiguration &conf = vcxProject.Configuration;
     if(!project->values("QMAKE_POST_LINK").isEmpty()) {
-        QString cmdline = var("QMAKE_POST_LINK");
+        QStringList cmdline = VCToolBase::fixCommandLine(var("QMAKE_POST_LINK"));
         conf.postBuild.CommandLine = cmdline;
-        conf.postBuild.Description = cmdline;
+        conf.postBuild.Description = cmdline.join(QLatin1String("\r\n"));
         conf.postBuild.UseInBuild = _True;
     }
 
@@ -390,14 +390,12 @@ void VcxprojGenerator::initPostBuildEventTools()
     bool useSignature = !signature.isEmpty() && !project->isActiveConfig("staticlib") &&
                         !project->isEmpty("CE_SDK") && !project->isEmpty("CE_ARCH");
     if(useSignature) {
-         conf.postBuild.CommandLine.prepend(QLatin1String("signtool sign /F ") + signature + " \"$(TargetPath)\"\n" +
-             (!conf.postBuild.CommandLine.isEmpty() ? " && " : ""));
+        conf.postBuild.CommandLine.prepend(
+                QLatin1String("signtool sign /F ") + signature + QLatin1String(" \"$(TargetPath)\""));
         conf.postBuild.UseInBuild = _True;
     }
 
     if(!project->values("MSVCPROJ_COPY_DLL").isEmpty()) {
-        if(!conf.postBuild.CommandLine.isEmpty())
-            conf.postBuild.CommandLine += " && ";
         conf.postBuild.Description += var("MSVCPROJ_COPY_DLL_DESC");
         conf.postBuild.CommandLine += var("MSVCPROJ_COPY_DLL");
         conf.postBuild.UseInBuild = _True;
@@ -530,9 +528,9 @@ void VcxprojGenerator::initPreLinkEventTools()
 {
     VCXConfiguration &conf = vcxProject.Configuration;
     if(!project->values("QMAKE_PRE_LINK").isEmpty()) {
-        QString cmdline = var("QMAKE_PRE_LINK");
-        conf.preLink.Description = cmdline;
+        QStringList cmdline = VCToolBase::fixCommandLine(var("QMAKE_PRE_LINK"));
         conf.preLink.CommandLine = cmdline;
+        conf.preLink.Description = cmdline.join(QLatin1String("\r\n"));
         conf.preLink.UseInBuild = _True;
     }
 }