Consolidate expansion and modification test in SourceWildcard

In order to make use of SourceWildcard in other contexts, any special
relationship to ResolvedGroup is now removed. All information needed
to re-expand the wildcards is stored inside the object. The re-expansion
test has been moved into the class and the API has been simplified.

SourceArtifact objects, no matter if wildcard-based or not are now
stored inside ResolvedGroup and are marked by a boolean flag.

Task-number: QBS-1749
Change-Id: I53bf2776685b5abf4713341387d1efa23ad316b2
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

7 files changed, 77 insertions, 73 deletions

diff --git a/src/lib/corelib/api/project.cpp b/src/lib/corelib/api/project.cpp
index 8d5152a24..860959e97 100644
--- a/src/lib/corelib/api/project.cpp
+++ b/src/lib/corelib/api/project.cpp
@@ -269,14 +269,10 @@ GroupData ProjectPrivate::createGroupDataFromGroup(const GroupPtr &resolvedGroup
     for (const auto &sa : resolvedGroup->files) {
         ArtifactData artifact = createApiSourceArtifact(sa);
         setupInstallData(artifact, product);
-        group.d->sourceArtifacts.push_back(artifact);
-    }
-    if (resolvedGroup->wildcards) {
-        for (const auto &sa : resolvedGroup->wildcards->files) {
-            ArtifactData artifact = createApiSourceArtifact(sa);
-            setupInstallData(artifact, product);
+        if (sa->fromWildcard)
             group.d->sourceArtifactsFromWildcards.push_back(artifact);
-        }
+        else
+            group.d->sourceArtifacts.push_back(artifact);
     }
     std::sort(group.d->sourceArtifacts.begin(),
               group.d->sourceArtifacts.end());
diff --git a/src/lib/corelib/buildgraph/buildgraphloader.cpp b/src/lib/corelib/buildgraph/buildgraphloader.cpp
index f263a3dbd..023931e5c 100644
--- a/src/lib/corelib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/corelib/buildgraph/buildgraphloader.cpp
@@ -598,25 +598,11 @@ bool BuildGraphLoader::hasProductFileChanged(const std::vector<ResolvedProductPt
             AccumulatingTimer wildcardTimer(m_parameters.logElapsedTime()
                                             ? &m_wildcardExpansionEffort : nullptr);
             for (const GroupPtr &group : product->groups) {
-                if (!group->wildcards)
-                    continue;
-                const bool reExpansionRequired = Internal::any_of(group->wildcards->dirTimeStamps,
-                            [](const std::pair<QString, FileTime> &pair) {
-                                return FileInfo(pair.first).lastModified() > pair.second;
-                });
-                if (!reExpansionRequired)
-                    continue;
-                const Set<QString> files = group->wildcards->expandPatterns(group->prefix,
-                        FileInfo::path(group->location.filePath()),
-                        product->topLevelProject()->buildDirectory);
-                Set<QString> wcFiles;
-                for (const auto &sourceArtifact : group->wildcards->files)
-                    wcFiles += sourceArtifact->absoluteFilePath;
-                if (files == wcFiles)
-                    continue;
-                hasChanged = true;
-                changedProducts.push_back(product);
-                break;
+                if (group->wildcards && group->wildcards->hasChangedSinceExpansion()) {
+                    hasChanged = true;
+                    changedProducts.push_back(product);
+                    break;
+                }
             }
         }
     }
diff --git a/src/lib/corelib/language/language.cpp b/src/lib/corelib/language/language.cpp
index 8b87a7977..d2c213999 100644
--- a/src/lib/corelib/language/language.cpp
+++ b/src/lib/corelib/language/language.cpp
@@ -144,29 +144,16 @@ void Probe::restoreValues()
  /*!
   * \variable ResolvedGroup::files
   * \brief The files listed in the group item's "files" binding.
-  * Note that these do not include expanded wildcards.
   */
 
 /*!
  * \variable ResolvedGroup::wildcards
- * \brief Represents the wildcard elements in this group's "files" binding.
+ * \brief Represents the wildcard patterns in this group's "files" binding.
  *  If no wildcards are specified there, this variable is null.
  * \sa SourceWildCards
  */
 
 /*!
- * \brief Returns all files specified in the group item as source artifacts.
- * This includes the expanded list of wildcards.
- */
-std::vector<SourceArtifactPtr> ResolvedGroup::allFiles() const
-{
-    std::vector<SourceArtifactPtr> lst = files;
-    if (wildcards)
-        lst << wildcards->files;
-    return lst;
-}
-
-/*!
  * \class RuleArtifact
  * \brief The \c RuleArtifact class represents an Artifact item encountered in the context
  *        of a Rule item.
@@ -320,7 +307,7 @@ std::vector<SourceArtifactPtr> ResolvedProduct::allFiles() const
 {
     std::vector<SourceArtifactPtr> lst;
     for (const auto &group : groups)
-        lst << group->allFiles();
+        lst << group->files;
     return lst;
 }
 
@@ -333,7 +320,7 @@ std::vector<SourceArtifactPtr> ResolvedProduct::allEnabledFiles() const
     std::vector<SourceArtifactPtr> lst;
     for (const auto &group : groups) {
         if (group->enabled)
-            lst << group->allFiles();
+            lst << group->files;
     }
     return lst;
 }
@@ -369,6 +356,8 @@ void ResolvedProduct::load(PersistentPool &pool)
         rule->product = this;
     for (const ResolvedModulePtr &module : modules)
         module->product = this;
+    for (const auto &group: groups)
+        group->restoreWildcards(buildDirectory());
 }
 
 void ResolvedProduct::store(PersistentPool &pool)
@@ -498,6 +487,19 @@ QString ResolvedProduct::cachedExecutablePath(const QString &origFilePath) const
     return m_executablePathCache.value(origFilePath);
 }
 
+void ResolvedGroup::restoreWildcards(const QString &buildDir)
+{
+    if (wildcards) {
+        wildcards->buildDir = buildDir;
+        wildcards->prefix = prefix;
+        wildcards->baseDir = FileInfo::path(location.filePath());
+        for (const auto &sourceArtifact : files) {
+            if (sourceArtifact->fromWildcard)
+                wildcards->expandedFiles += sourceArtifact->absoluteFilePath;
+        }
+    }
+}
+
 
 ResolvedProject::ResolvedProject() : enabled(true), m_topLevelProject(nullptr)
 {
@@ -705,16 +707,13 @@ void TopLevelProject::cleanupModuleProviderOutput()
  * \brief The \c SourceArtifacts resulting from the expanded list of matching files.
  */
 
-Set<QString> SourceWildCards::expandPatterns(const QString &prefix, const QString &baseDir,
-                                             const QString &buildDir)
+void SourceWildCards::expandPatterns()
 {
-    Set<QString> files = expandPatterns(prefix, patterns, baseDir, buildDir);
-    files -= expandPatterns(prefix, excludePatterns, baseDir, buildDir);
-    return files;
+    dirTimeStamps.clear();
+    expandedFiles = expandPatterns(patterns) - expandPatterns(excludePatterns);
 }
 
-Set<QString> SourceWildCards::expandPatterns(const QString &prefix, const QStringList &patterns,
-                                             const QString &baseDir, const QString &buildDir)
+Set<QString> SourceWildCards::expandPatterns(const QStringList &patterns)
 {
     Set<QString> files;
     QString expandedPrefix = prefix;
@@ -733,9 +732,9 @@ Set<QString> SourceWildCards::expandPatterns(const QString &prefix, const QStrin
             } else {
                 rootDir = QLatin1Char('/');
             }
-            expandPatterns(files, parts, rootDir, buildDir);
+            expandPatterns(files, parts, rootDir);
         } else {
-            expandPatterns(files, parts, baseDir, buildDir);
+            expandPatterns(files, parts, baseDir);
         }
     }
 
@@ -743,7 +742,7 @@ Set<QString> SourceWildCards::expandPatterns(const QString &prefix, const QStrin
 }
 
 void SourceWildCards::expandPatterns(Set<QString> &result, const QStringList &parts,
-                                     const QString &baseDir, const QString &buildDir)
+                                     const QString &baseDir)
 {
     // People might build directly in the project source directory. This is okay, since
     // we keep the build data in a "container" directory. However, we must make sure we don't
@@ -793,7 +792,7 @@ void SourceWildCards::expandPatterns(Set<QString> &result, const QStringList &pa
         if (!isDir && it.fileInfo().isDir() && !it.fileInfo().isSymLink())
             continue;
         if (isDir) {
-            expandPatterns(result, changed_parts, filePath, buildDir);
+            expandPatterns(result, changed_parts, filePath);
         } else {
             if (parentDir != baseDir)
                 dirTimeStamps.emplace_back(parentDir, FileInfo(baseDir).lastModified());
@@ -802,6 +801,21 @@ void SourceWildCards::expandPatterns(Set<QString> &result, const QStringList &pa
     }
 }
 
+bool SourceWildCards::hasChangedSinceExpansion() const
+{
+    const bool reExpansionRequired =
+        Internal::any_of(dirTimeStamps,
+                         [](const std::pair<QString, FileTime> &pair) {
+                             return FileInfo(pair.first).lastModified() > pair.second;
+                         });
+    if (reExpansionRequired)
+        return true;
+
+    auto wc = *this;
+    wc.expandPatterns();
+    return this->expandedFiles != wc.expandedFiles;
+}
+
 template<typename L>
 QMap<QString, typename L::value_type> listToMap(const L &list)
 {
diff --git a/src/lib/corelib/language/language.h b/src/lib/corelib/language/language.h
index c2f892727..16a49b017 100644
--- a/src/lib/corelib/language/language.h
+++ b/src/lib/corelib/language/language.h
@@ -49,6 +49,7 @@
 
 #include <buildgraph/forward_decls.h>
 #include <tools/codelocation.h>
+#include <tools/fileinfo.h>
 #include <tools/filetime.h>
 #include <tools/joblimits.h>
 #include <tools/persistence.h>
@@ -241,11 +242,12 @@ public:
     bool overrideFileTags;
     QString targetOfModule;
     PropertyMapPtr properties;
+    bool fromWildcard;
 
     template<PersistentPool::OpType opType> void completeSerializationOp(PersistentPool &pool)
     {
         pool.serializationOp<opType>(absoluteFilePath, fileTags, overrideFileTags, properties,
-                                     targetOfModule);
+                                     targetOfModule, fromWildcard);
     }
 
 private:
@@ -259,24 +261,28 @@ inline bool operator!=(const SourceArtifactInternal &sa1, const SourceArtifactIn
 class SourceWildCards
 {
 public:
-    Set<QString> expandPatterns(const QString &prefix, const QString &baseDir,
-                                const QString &buildDir);
+    void expandPatterns();
+    bool hasChangedSinceExpansion() const;
 
+    // to be restored by the owning class
+    QString prefix;
+    QString baseDir;
+    QString buildDir;
+    Set<QString> expandedFiles;
+
+    // stored
     QStringList patterns;
     QStringList excludePatterns;
     std::vector<std::pair<QString, FileTime>> dirTimeStamps;
-    std::vector<SourceArtifactPtr> files;
 
     template<PersistentPool::OpType opType> void completeSerializationOp(PersistentPool &pool)
     {
-        pool.serializationOp<opType>(patterns, excludePatterns, dirTimeStamps, files);
+        pool.serializationOp<opType>(patterns, excludePatterns, dirTimeStamps);
     }
 
 private:
-    Set<QString> expandPatterns(const QString &prefix, const QStringList &patterns,
-                                const QString &baseDir, const QString &buildDir);
-    void expandPatterns(Set<QString> &result, const QStringList &parts,
-                        const QString &baseDir, const QString &buildDir);
+    Set<QString> expandPatterns(const QStringList &patterns);
+    void expandPatterns(Set<QString> &result, const QStringList &parts, const QString &baseDir);
 };
 
 class QBS_AUTOTEST_EXPORT ResolvedGroup
@@ -296,7 +302,7 @@ public:
     QString targetOfModule;
     bool overrideTags = false;
 
-    std::vector<SourceArtifactPtr> allFiles() const;
+    void restoreWildcards(const QString &buildDir);
 
     template<PersistentPool::OpType opType> void completeSerializationOp(PersistentPool &pool)
     {
diff --git a/src/lib/corelib/loader/productresolver.cpp b/src/lib/corelib/loader/productresolver.cpp
index 3b97b4adf..43d0a06f9 100644
--- a/src/lib/corelib/loader/productresolver.cpp
+++ b/src/lib/corelib/loader/productresolver.cpp
@@ -794,10 +794,11 @@ void ProductResolverStage2::resolveGroupFully(Item *item, bool isEnabled)
         wildcards->excludePatterns = evaluator.stringListValue(
             item, StringConstants::excludeFilesProperty());
         wildcards->patterns = patterns;
-        const Set<QString> files = wildcards->expandPatterns(group->prefix,
-                FileInfo::path(item->file()->filePath()),
-                m_product.project->project->topLevelProject()->buildDirectory);
-        for (const QString &fileName : files)
+        wildcards->prefix = group->prefix;
+        wildcards->baseDir = FileInfo::path(item->file()->filePath());
+        wildcards->buildDir = m_product.project->project->topLevelProject()->buildDirectory;
+        wildcards->expandPatterns();
+        for (const QString &fileName : wildcards->expandedFiles)
             createSourceArtifact(fileName, group, true, filesLocation, &fileError);
     }
 
@@ -864,7 +865,8 @@ SourceArtifactPtr ProductResolverStage2::createSourceArtifact(
     artifact->overrideFileTags = group->overrideTags;
     artifact->properties = group->properties;
     artifact->targetOfModule = group->targetOfModule;
-    (wildcard ? group->wildcards->files : group->files).push_back(artifact);
+    artifact->fromWildcard = wildcard;
+    group->files.push_back(artifact);
     return artifact;
 }
 
diff --git a/src/lib/corelib/tools/persistence.cpp b/src/lib/corelib/tools/persistence.cpp
index cf5903349..44090dee2 100644
--- a/src/lib/corelib/tools/persistence.cpp
+++ b/src/lib/corelib/tools/persistence.cpp
@@ -48,7 +48,7 @@
 namespace qbs {
 namespace Internal {
 
-static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-132";
+static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-133";
 
 NoBuildGraphError::NoBuildGraphError(const QString &filePath)
     : ErrorInfo(Tr::tr("Build graph not found for configuration '%1'. Expected location was '%2'.")
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index 6a29876dc..343b059c9 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -739,7 +739,7 @@ void TestLanguage::enumerateProjectProperties()
         auto products = productsFromProject(project);
         QCOMPARE(products.size(), 1);
         auto product = products.values().front();
-        auto files = product->groups.front()->allFiles();
+        auto files = product->groups.front()->files;
         QCOMPARE(product->groups.size(), size_t(1));
         QCOMPARE(files.size(), size_t(1));
         auto fileName = FileInfo::fileName(files.front()->absoluteFilePath);
@@ -3049,7 +3049,7 @@ void TestLanguage::relaxedErrorMode()
         QVERIFY(missingFile->enabled);
         QCOMPARE(missingFile->groups.size(), size_t(1));
         QVERIFY(missingFile->groups.front()->enabled);
-        QCOMPARE(missingFile->groups.front()->allFiles().size(), size_t(2));
+        QCOMPARE(missingFile->groups.front()->files.size(), size_t(2));
         const ResolvedProductConstPtr fine = productMap.value("fine");
         QVERIFY(fine->enabled);
         QCOMPARE(fine->allFiles().size(), size_t(1));
@@ -3445,10 +3445,10 @@ void TestLanguage::wildcards()
             group = product->groups.front();
         }
         QVERIFY(!!group);
-        QCOMPARE(group->files.size(), size_t(0));
+        QCOMPARE(group->files.size(), expected.size()); // we assume all files are wildcards
         QVERIFY(!!group->wildcards);
         QStringList actualFilePaths;
-        for (const SourceArtifactPtr &artifact : group->wildcards->files) {
+        for (const SourceArtifactPtr &artifact : group->files) {
             QString str = artifact->absoluteFilePath;
             int idx = str.indexOf(m_wildcardsTestDirPath);
             if (idx != -1)