Fix wildcard change tracking

We recorded both too many and not enough time stamps for wildcards:
  - We should not try to track changes regarding recursive wildcards
    ("**/"), as that can easily pull in large parts of the source
    tree and/or the project root directory, likely resulting in
    unwanted re-resolvings.
  - We should track the parent dir of a wildcard pattern, even if
    the last evaluation yielded no matches.

Change-Id: I8e5521a2fcb7fd1f54103e0df566f33dd6f2a972
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>

8 files changed, 54 insertions, 8 deletions

diff --git a/src/lib/corelib/language/language.cpp b/src/lib/corelib/language/language.cpp
index 0e24b8b5e..b3f4b2a64 100644
--- a/src/lib/corelib/language/language.cpp
+++ b/src/lib/corelib/language/language.cpp
@@ -750,8 +750,6 @@ void SourceWildCards::expandPatterns(Set<QString> &result, const QStringList &pa
     if (baseDir.startsWith(buildDir))
         return;
 
-    dirTimeStamps.emplace_back(baseDir, FileInfo(baseDir).lastModified());
-
     QStringList changed_parts = parts;
     bool recursive = false;
     QString part = changed_parts.takeFirst();
@@ -778,8 +776,12 @@ void SourceWildCards::expandPatterns(Set<QString> &result, const QStringList &pa
             : QDir::Files | QDir::System
               | QDir::Dirs; // This one is needed to get symbolic links to directories
 
-    if (isDir && !FileInfo::isPattern(filePattern))
+    if (FileInfo::isPattern(filePattern)) {
+        if (!recursive)
+            dirTimeStamps.emplace_back(baseDir, FileInfo(baseDir).lastModified());
+    } else if (isDir) {
         itFilters |= QDir::Hidden;
+    }
     if (filePattern != StringConstants::dotDot() && filePattern != StringConstants::dot())
         itFilters |= QDir::NoDotAndDotDot;
 
@@ -791,13 +793,10 @@ void SourceWildCards::expandPatterns(Set<QString> &result, const QStringList &pa
             continue; // See above.
         if (!isDir && it.fileInfo().isDir() && !it.fileInfo().isSymLink())
             continue;
-        if (isDir) {
+        if (isDir)
             expandPatterns(result, changed_parts, filePath);
-        } else {
-            if (parentDir != baseDir)
-                dirTimeStamps.emplace_back(parentDir, FileInfo(parentDir).lastModified());
+        else
             result += QDir::cleanPath(filePath);
-        }
     }
 }
 
diff --git a/tests/auto/blackbox/testdata/wildcards-and-change-tracking/nonrecursive/subdir1/file.txt b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/nonrecursive/subdir1/file.txt
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/nonrecursive/subdir1/file.txt
diff --git a/tests/auto/blackbox/testdata/wildcards-and-change-tracking/nonrecursive/subdir2/file.txt b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/nonrecursive/subdir2/file.txt
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/nonrecursive/subdir2/file.txt
diff --git a/tests/auto/blackbox/testdata/wildcards-and-change-tracking/recursive1/recursive.txt b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/recursive1/recursive.txt
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/recursive1/recursive.txt
diff --git a/tests/auto/blackbox/testdata/wildcards-and-change-tracking/recursive2/recursive.txt b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/recursive2/recursive.txt
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/recursive2/recursive.txt
diff --git a/tests/auto/blackbox/testdata/wildcards-and-change-tracking/wildcards-and-change-tracking.qbs b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/wildcards-and-change-tracking.qbs
new file mode 100644
index 000000000..0a889bab3
--- /dev/null
+++ b/tests/auto/blackbox/testdata/wildcards-and-change-tracking/wildcards-and-change-tracking.qbs
@@ -0,0 +1,16 @@
+Product {
+    Group {
+        name: "recursive"
+        files: "**/file.txt"
+    }
+    Group {
+        name: "directories"
+        prefix: "nonrecursive/"
+        files: "subdi?/file.txt"
+    }
+    Group {
+        prefix: "nonrecursive/empty/"
+        name: "no files"
+        files: "*.txt"
+    }
+}
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 8ba6e2fa5..395e5aba6 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -8048,6 +8048,35 @@ void TestBlackbox::wildCardsAndRules()
     QVERIFY(!m_qbsStdout.contains("creating output artifact"));
 }
 
+void TestBlackbox::wildCardsAndChangeTracking_data()
+{
+    QTest::addColumn<QString>("dirToModify");
+    QTest::addColumn<bool>("expectReResolve");
+
+    QTest::newRow("root path") << QString(".") << false;
+    QTest::newRow("dir with recursive match") << QString("recursive1") << false;
+    QTest::newRow("non-recursive base dir") << QString("nonrecursive") << true;
+    QTest::newRow("empty base dir with file patterns") << QString("nonrecursive/empty") << true;
+}
+
+void TestBlackbox::wildCardsAndChangeTracking()
+{
+    QFETCH(QString, dirToModify);
+    QFETCH(bool, expectReResolve);
+
+    const QString srcDir = testDataDir + "/wildcards-and-change-tracking";
+    QDir::setCurrent(srcDir);
+    rmDirR("default");
+    QDir::current().mkdir("nonrecursive/empty");
+
+    QCOMPARE(runQbs({"resolve"}), 0);
+    QVERIFY2(m_qbsStdout.contains("Resolving"), m_qbsStdout.constData());
+    WAIT_FOR_NEW_TIMESTAMP();
+    touch(dirToModify + "/blubb.txt");
+    QCOMPARE(runQbs({"resolve"}), 0);
+    QCOMPARE(m_qbsStdout.contains("Resolving"), expectReResolve);
+}
+
 void TestBlackbox::loadableModule()
 {
     QDir::setCurrent(testDataDir + QLatin1String("/loadablemodule"));
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index f878fd89e..62676625e 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -344,6 +344,8 @@ private slots:
     void wholeArchive();
     void wholeArchive_data();
     void wildCardsAndRules();
+    void wildCardsAndChangeTracking_data();
+    void wildCardsAndChangeTracking();
     void wildcardRenaming();
     void zip();
     void zip_data();