Tune search order of path probes

Within the groups of user-provided and system-provided paths,
environment variables need to take precedence over properties, because
there is currently no other way to override the search paths of probes
from the outside if the probe-using code did not explicitly bind them to
Product/Module properties.

We search directly user-provided paths before ones from system-provided
environment variables to minimize the risk of surprises due to outside
influence.

[ChangeLog][Behavior Changes] The lookup order in PathProbe changed to
[environmentPaths, searchPaths, platformEnvironmentPaths,
platformSearchPaths]

Change-Id: Ib0c3bc44e5a8efaaaa073f28f1f3a53feb0f78db
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

6 files changed, 35 insertions, 12 deletions

diff --git a/doc/reference/items/probe/path-probe.qdoc b/doc/reference/items/probe/path-probe.qdoc
index df5fbb22f..b7749f09d 100644
--- a/doc/reference/items/probe/path-probe.qdoc
+++ b/doc/reference/items/probe/path-probe.qdoc
@@ -37,10 +37,11 @@
     locations.
 
     PathProbe takes as input lists of paths to search files from and file name patterns. The paths
-    are specified by using the searchPaths, platformSearchPaths, environmentPaths, and
-    platformEnvironmentPaths properties. The file name patterns are specified by the \l names and
-    nameSuffixes properties. Returns the first file that matches the file name patterns. If no such
-    file is found, the \l {Probe::found}{probe.found} property is set to \c false.
+    are specified by using the environmentPaths, searchPaths, platformEnvironmentPaths and
+    platformSearchPaths properties; the path are searched in the same order as listed. The file
+    name patterns are specified by the \l names and nameSuffixes properties. Returns the first file
+    that matches the file name patterns. If no such file is found, the
+    \l {Probe::found}{probe.found} property is set to \c false.
 
     For example, a simple PathProbe that searches for the stdio.h header can be used as follows:
 
diff --git a/share/qbs/imports/qbs/Probes/path-probe.js b/share/qbs/imports/qbs/Probes/path-probe.js
index b4d745428..b1bdf9930 100644
--- a/share/qbs/imports/qbs/Probes/path-probe.js
+++ b/share/qbs/imports/qbs/Probes/path-probe.js
@@ -60,6 +60,17 @@ function canonicalSelectors(selectors, nameSuffixes) {
     return selectors.map(mapper);
 }
 
+function pathsFromEnvs(envs, pathListSeparator) {
+    envs = envs || [];
+    var result = [];
+    for (var i = 0; i < envs.length; ++i) {
+        var value = Environment.getEnv(envs[i]) || '';
+        if (value.length > 0)
+            result = result.concat(value.split(pathListSeparator));
+    }
+    return result;
+}
+
 function configure(selectors, names, nameSuffixes, nameFilter, candidateFilter,
                    searchPaths, pathSuffixes, platformSearchPaths, environmentPaths,
                    platformEnvironmentPaths, pathListSeparator) {
@@ -90,14 +101,11 @@ function configure(selectors, names, nameSuffixes, nameFilter, candidateFilter,
     });
 
     // FIXME: Suggest how to obtain paths from system
-    var _paths = ModUtils.concatAll(searchPaths, platformSearchPaths);
-    // FIXME: Add getenv support
-    var envs = ModUtils.concatAll(platformEnvironmentPaths, environmentPaths);
-    for (var i = 0; i < envs.length; ++i) {
-        var value = Environment.getEnv(envs[i]) || '';
-        if (value.length > 0)
-            _paths = _paths.concat(value.split(pathListSeparator));
-    }
+    var _paths = ModUtils.concatAll(
+                pathsFromEnvs(environmentPaths, pathListSeparator),
+                searchPaths,
+                pathsFromEnvs(platformEnvironmentPaths, pathListSeparator),
+                platformSearchPaths);
     var _suffixes = ModUtils.concatAll('', pathSuffixes);
     _paths = _paths.map(function(p) { return FileInfo.fromNativeSeparators(p); });
     _suffixes = _suffixes.map(function(p) { return FileInfo.fromNativeSeparators(p); });
diff --git a/tests/auto/blackbox/testdata/path-probe/BaseApp.qbs b/tests/auto/blackbox/testdata/path-probe/BaseApp.qbs
index 93172579f..2fe232baf 100644
--- a/tests/auto/blackbox/testdata/path-probe/BaseApp.qbs
+++ b/tests/auto/blackbox/testdata/path-probe/BaseApp.qbs
@@ -39,6 +39,7 @@ CppApplication {
     property pathList inputSearchPaths
     property var inputNameFilter
     property var inputCandidateFilter
+    property stringList inputEnvironmentPaths
 
     property stringList outputFilePaths
     property var outputCandidatePaths
@@ -52,6 +53,7 @@ CppApplication {
         candidateFilter: inputCandidateFilter
         searchPaths: inputSearchPaths
         platformSearchPaths: []
+        environmentPaths: inputEnvironmentPaths
     }
 
     property bool validate: {
diff --git a/tests/auto/blackbox/testdata/path-probe/environment-paths.qbs b/tests/auto/blackbox/testdata/path-probe/environment-paths.qbs
new file mode 100644
index 000000000..fca824bfb
--- /dev/null
+++ b/tests/auto/blackbox/testdata/path-probe/environment-paths.qbs
@@ -0,0 +1,10 @@
+import qbs.FileInfo
+
+BaseApp {
+    inputNames: "tool"
+    inputSearchPaths: ["bin", "usr/bin"]
+     // env takes precedence
+    inputEnvironmentPaths: "SEARCH_PATH"
+    outputFilePaths: ["usr/bin/tool"]
+    outputCandidatePaths: [["usr/bin/tool"]]
+}
diff --git a/tests/auto/blackbox/testdata/path-probe/usr/bin/tool b/tests/auto/blackbox/testdata/path-probe/usr/bin/tool
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/auto/blackbox/testdata/path-probe/usr/bin/tool
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 366a618c0..ba07b632f 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -2961,6 +2961,7 @@ void TestBlackbox::pathProbe_data()
     QTest::newRow("mult-files-mult-suffixes") << QString("mult-files-mult-suffixes.qbs") << true;
     QTest::newRow("name-filter") << QString("name-filter.qbs") << true;
     QTest::newRow("candidate-filter") << QString("candidate-filter.qbs") << true;
+    QTest::newRow("environment-paths") << QString("environment-paths.qbs") << true;
 }
 
 void TestBlackbox::pathProbe()
@@ -2972,6 +2973,7 @@ void TestBlackbox::pathProbe()
 
     QbsRunParameters buildParams("build", QStringList{"-f", projectFile});
     buildParams.expectFailure = !successExpected;
+    buildParams.environment.insert("SEARCH_PATH", "usr/bin");
     QCOMPARE(runQbs(buildParams) == 0, successExpected);
     if (!successExpected)
         QVERIFY2(m_qbsStderr.contains("Probe failed to find files"), m_qbsStderr);