rewrite qtAddToolEnv()

the primary purpose is making env var prepend mode work for unset
variables on windows. this is achieved by using a conditional and delayed
variable expansion. however, the latter is disabled by default and can
be locally enabled only in batch files. therefore, write wrapper scripts
and substitute them for the actual commands. we do this also on unix,
both for consistency and simply because the commands look much less
confusing.

this change is slightly backwards-incompatible, as invoking
qtAddToolEnv() multiple times on the same command will now make a total
mess. also, invoking it on a command that contains 'make' macro
expansions isn't a good idea, so testcase.prf needed an adjustment. the
function is an undocumented internal, so Nobody Should Care (TM).

this also reverts 80ebedecf9, as it's obsolete now.

Change-Id: I8394b77868b495abcf27b688996ca74c40b80994
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

.gitignore

@@ -47,6 +47,10 @@ pcviewer.cfg
 *_resource.rc
 .#*
 *.*#
+*_wrapper.sh
+*_wrapper.bat
+wrapper.sh
+wrapper.bat
 core
 .qmake.cache
 .qmake.vars

mkspecs/features/qt_functions.prf

@@ -70,6 +70,7 @@ defineTest(qtPrepareTool) {
         }
     }
     QT_TOOL_ENV += $$eval(QT_TOOL.$${2}.envvars)
+    QT_TOOL_NAME = $$2
     !isEmpty(3)|!isEmpty(4) {
         $$1$$3 =
         for (arg, cmd): \
@@ -90,6 +91,7 @@ defineTest(qtAddToolEnv) {
         ds = $$QMAKE_DIR_SEP
     else: \
         ds = $$DIR_SEPARATOR
+    batch_sets =
     for(env, 2) {
         value = $$eval($${env}.value)
         !isEmpty(value) {
@@ -97,22 +99,61 @@ defineTest(qtAddToolEnv) {
             equals(ds, /) {
                 contains($${env}.CONFIG, prepend): infix = \${$$name:+:\$$$name}
                 else: infix =
-                val = "$$name=$$shell_quote($$join(value, :))$$infix"
+                # Under msys, this path is taken only in the non-system()
+                # case, so using shell_quote() always works.
+                batch_sets += \
+                    "$$name=$$shell_quote($$join(value, :))$$infix" \
+                    "export $$name"
             } else {
-                # Escape closing parens when expanding the variable, otherwise cmd confuses itself.
-                contains($${env}.CONFIG, prepend): infix = ;%$$name:)=^)%
-                else: infix =
+                value ~= s,\\^,^^^^,g
+                value ~= s,!,^^!,g
                 value ~= s,\\),^),g
-                val = "(set $$name=$$join(value, ;)$$infix) &"
+                contains($${env}.CONFIG, prepend) {
+                    batch_sets += \
+                        "if defined $$name (" \
+                        "    set $$name=$$join(value, ;);!$$name!" \
+                        ") else (" \
+                        "    set $$name=$$join(value, ;)" \
+                        ")"
+                } else {
+                    batch_sets += "(set $$name=$$join(value, ;))"
+                }
             }
-            isEmpty(3): !contains(TEMPLATE, vc.*) {
-                contains(MAKEFILE_GENERATOR, MS.*): val ~= s,%,%%,g
-                val ~= s,\\\$,\$\$,g
-            }
-            $$1 = "$$val $$eval($$1)"
         }
     }
+    !isEmpty(batch_sets) {
+        batch_name = wrapper
+        !isEmpty(QT_TOOL_NAME): batch_name = $${QT_TOOL_NAME}_wrapper
+        cmd = $$eval($$1)
+        !isEmpty(cmd): cmd = "$$cmd "
+        equals(ds, /) {
+            batch_name = $${batch_name}.sh
+            batch_cont = \
+                "$$LITERAL_HASH!/bin/sh" \
+                $$batch_sets \
+                "exec $$cmd\"$@\""
+            # It would be nicer to use the '.' command (without 'exec' above),
+            # but that doesn't set the positional arguments under (d)ash.
+            $$1 =
+        } else {
+            batch_name = $${batch_name}.bat
+            batch_cont = \
+                "@echo off" \
+                "SetLocal EnableDelayedExpansion" \
+                $$batch_sets \
+                "$$cmd%*" \
+                "EndLocal"
+            $$1 = call
+        }
+        !build_pass:!write_file($$OUT_PWD/$$batch_name, batch_cont, exe): error("Aborting.")
+        isEmpty(3): \
+            $$1 += $$shell_quote($$shell_path($$OUT_PWD/$$batch_name))
+        else: \
+            $$1 += $$system_quote($$system_path($$OUT_PWD/$$batch_name))
+        QMAKE_DISTCLEAN += $$OUT_PWD/$$batch_name
+    }
     export($$1)
+    export(QMAKE_DISTCLEAN)
 }
 
 # target variable, dependency var name, [non-empty: prepare for system(), not make]

mkspecs/features/testcase.prf

@@ -9,6 +9,11 @@ testcase_exceptions: CONFIG += exceptions
 check.files                =
 check.path                 = .
 
+# Add environment for non-installed builds. Do this first, so the
+# 'make' variable expansions don't end up in a batch file/script.
+QT_TOOL_NAME = target
+qtAddTargetEnv(check.commands, QT)
+
 # If the test ends up in a different directory, we should cd to that directory.
 TESTRUN_CWD = $$DESTDIR
 
@@ -40,10 +45,6 @@ unix {
 # Allow for custom arguments to tests
 check.commands += $(TESTARGS)
 
-# Add environment for non-installed builds
-qtAddTargetEnv(check.commands, QT)
-
-# This must happen after adding the environment.
 !isEmpty(TESTRUN_CWD):!contains(TESTRUN_CWD, ^\\./?): \
     check.commands = cd $$shell_path($$TESTRUN_CWD) && $$check.commands
 

qmake/generators/win32/msvc_objectmodel.cpp

@@ -2399,11 +2399,7 @@ bool VCFilter::addExtraCompiler(const VCFilterFile &info)
         if (!CustomBuildTool.Description.isEmpty())
             CustomBuildTool.Description += ", ";
         CustomBuildTool.Description += cmd_name;
-        // Execute custom build steps in an environment variable scope to prevent unwanted
-        // side effects for downstream build steps
-        CustomBuildTool.CommandLine += QLatin1String("setlocal");
         CustomBuildTool.CommandLine += VCToolBase::fixCommandLine(cmd.trimmed());
-        CustomBuildTool.CommandLine += QLatin1String("endlocal");
         int space = cmd.indexOf(' ');
         QFileInfo finf(cmd.left(space));
         if (CustomBuildTool.ToolPath.isEmpty())