Fix "build single file" and "changed files" functionalities.

These had several problems.
Firstly, the "changed files" case was implemented by setting all
artifacts to "Built" and then setting the ones to "Buildable" that were
reachable bottom-up from artifacts corresponding to the respective files.
This approach broke with the introduction of rule nodes, because
parent nodes do not necessarily exist yet at initialization time. This
was not caught due to the lack of an autotest.
Secondly, the logic behind the "build single file" functionality was
faulty. The assumption was that this could be implemented on top of the
"changed file" functionality, which is wrong: Consider the case where
you have several cpp files that have not yet been built. Now
marking one of them as changed and filtering by the "obj" tag will still
cause all of them to be compiled, as we cannot simply exclude all other
source files from being built, which would break the build for the
normal "changed files" case without tag filtering. Therefore we need a
dedicated list of input files by which we can filter transformers.

Task-number: QBS-537
Change-Id: I47e2ba6d0cbd073561064640eaf8f63c4e0b39fa
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

14 files changed, 130 insertions, 59 deletions

diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp
index 583fd2396..f54034173 100644
--- a/src/lib/corelib/buildgraph/executor.cpp
+++ b/src/lib/corelib/buildgraph/executor.cpp
@@ -111,7 +111,12 @@ void Executor::retrieveSourceFileTimestamp(Artifact *artifact) const
 {
     QBS_CHECK(artifact->artifactType == Artifact::SourceFile);
 
-    artifact->setTimestamp(recursiveFileTime(artifact->filePath()));
+    if (m_buildOptions.changedFiles().contains(artifact->filePath()))
+        artifact->setTimestamp(FileTime::currentTime());
+    else if (m_buildOptions.changedFiles().isEmpty())
+        artifact->setTimestamp(recursiveFileTime(artifact->filePath()));
+    else
+        artifact->setTimestamp(FileTime::oldestTime());
     artifact->timestampRetrieved = true;
 }
 
@@ -220,21 +225,10 @@ void Executor::setBuildOptions(const BuildOptions &buildOptions)
     m_buildOptions = buildOptions;
 }
 
-static void initNodesBottomUp(BuildGraphNode *node)
-{
-    if (node->buildState == BuildGraphNode::Untouched)
-        return;
-    node->buildState = BuildGraphNode::Buildable;
-    foreach (BuildGraphNode *parent, node->parents)
-        initNodesBottomUp(parent);
-}
 
 void Executor::initLeaves()
 {
-    if (m_buildOptions.changedFiles().isEmpty())
-        updateLeaves(m_roots);
-    else
-        initLeavesForSelectedFiles();
+    updateLeaves(m_roots);
 }
 
 void Executor::updateLeaves(const NodeSet &nodes)
@@ -244,28 +238,6 @@ void Executor::updateLeaves(const NodeSet &nodes)
         updateLeaves(node, seenNodes);
 }
 
-void Executor::initLeavesForSelectedFiles()
-{
-    ArtifactSet changedArtifacts;
-    foreach (const QString &filePath, m_buildOptions.changedFiles()) {
-        QList<FileResourceBase *> lookupResults;
-        lookupResults.append(m_project->buildData->lookupFiles(filePath));
-        if (lookupResults.isEmpty()) {
-            m_logger.qbsWarning() << QString::fromLocal8Bit("Out of date file '%1' provided "
-                    "but not found.").arg(QDir::toNativeSeparators(filePath));
-            continue;
-        }
-        foreach (FileResourceBase *lookupResult, lookupResults)
-            if (Artifact *artifact = dynamic_cast<Artifact *>(lookupResult))
-                changedArtifacts += artifact;
-    }
-
-    foreach (Artifact *artifact, changedArtifacts) {
-        m_leaves.push(artifact);
-        initNodesBottomUp(artifact);
-    }
-}
-
 void Executor::updateLeaves(BuildGraphNode *node, NodeSet &seenNodes)
 {
     if (seenNodes.contains(node))
@@ -306,7 +278,8 @@ bool Executor::scheduleJobs()
 
         switch (nodeToBuild->buildState) {
         case BuildGraphNode::Untouched:
-            QBS_ASSERT(!"untouched node in leaves list", /* ignore */);
+            QBS_ASSERT(!"untouched node in leaves list",
+                       qDebug("%s", qPrintable(nodeToBuild->toString())));
             break;
         case BuildGraphNode::Buildable:
             // This is the only state in which we want to build a node.
@@ -582,6 +555,34 @@ QString Executor::configString() const
     return tr(" for configuration %1").arg(m_project->id());
 }
 
+bool Executor::transformerHasMatchingOutputTags(const TransformerConstPtr &transformer) const
+{
+    if (m_activeFileTags.isEmpty())
+        return true; // No filtering requested.
+
+    foreach (Artifact * const output, transformer->outputs) {
+        if (m_activeFileTags.matches(output->fileTags))
+            return true;
+    }
+
+    return false;
+}
+
+bool Executor::transformerHasMatchingInputFiles(const TransformerConstPtr &transformer) const
+{
+    if (m_buildOptions.filesToConsider().isEmpty())
+        return true; // No filtering requested.
+
+    foreach (const Artifact * const input, transformer->inputs) {
+        foreach (const QString &filePath, m_buildOptions.filesToConsider()) {
+            if (input->filePath() == filePath)
+                return true;
+        }
+    }
+
+    return false;
+}
+
 void Executor::cancelJobs()
 {
     m_logger.qbsTrace() << "Canceling all jobs.";
@@ -733,8 +734,6 @@ bool Executor::checkForUnbuiltDependencies(Artifact *artifact)
 
 void Executor::potentiallyRunTransformer(const TransformerPtr &transformer)
 {
-    bool matchingFileTagsExist = m_activeFileTags.isEmpty();
-
     foreach (Artifact * const output, transformer->outputs) {
         // Rescuing build data can introduce new dependencies, potentially delaying execution of
         // this transformer.
@@ -742,20 +741,22 @@ void Executor::potentiallyRunTransformer(const TransformerPtr &transformer)
         rescueOldBuildData(output, &childrenAddedDueToRescue);
         if (childrenAddedDueToRescue && checkForUnbuiltDependencies(output))
             return;
-
-        if (!matchingFileTagsExist && m_activeFileTags.matches(output->fileTags))
-            matchingFileTagsExist = true;
     }
 
-    // Skip if we're building just one file and the file tags do not match.
-    if (!matchingFileTagsExist) {
+    if (!transformerHasMatchingOutputTags(transformer)) {
         if (m_doDebug)
             m_logger.qbsDebug() << "[EXEC] file tags do not match. Skipping.";
         finishTransformer(transformer);
         return;
     }
 
-    // Skip transformers that do not need to be built.
+    if (!transformerHasMatchingInputFiles(transformer)) {
+        if (m_doDebug)
+            m_logger.qbsDebug() << "[EXEC] input files do not match. Skipping.";
+        finishTransformer(transformer);
+        return;
+    }
+
     if (!mustExecuteTransformer(transformer)) {
         if (m_doDebug)
             m_logger.qbsDebug() << "[EXEC] Up to date. Skipping.";
@@ -927,21 +928,18 @@ void Executor::prepareArtifact(Artifact *artifact)
  */
 void Executor::prepareReachableNodes()
 {
-    const BuildGraphNode::BuildState initialBuildState = m_buildOptions.changedFiles().isEmpty()
-            ? BuildGraphNode::Buildable : BuildGraphNode::Built;
     foreach (BuildGraphNode *root, m_roots)
-        prepareReachableNodes_impl(root, initialBuildState);
+        prepareReachableNodes_impl(root);
 }
 
-void Executor::prepareReachableNodes_impl(BuildGraphNode *node,
-        const BuildGraphNode::BuildState buildState)
+void Executor::prepareReachableNodes_impl(BuildGraphNode *node)
 {
     if (node->buildState != BuildGraphNode::Untouched)
         return;
 
-    node->buildState = buildState;
+    node->buildState = BuildGraphNode::Buildable;
     foreach (BuildGraphNode *child, node->children)
-        prepareReachableNodes_impl(child, buildState);
+        prepareReachableNodes_impl(child);
 }
 
 void Executor::prepareProducts()
diff --git a/src/lib/corelib/buildgraph/executor.h b/src/lib/corelib/buildgraph/executor.h
index 68afe6083..63dcd7dd2 100644
--- a/src/lib/corelib/buildgraph/executor.h
+++ b/src/lib/corelib/buildgraph/executor.h
@@ -100,13 +100,12 @@ private:
     void prepareAllNodes();
     void prepareArtifact(Artifact *artifact);
     void prepareReachableNodes();
-    void prepareReachableNodes_impl(BuildGraphNode *node, const Artifact::BuildState buildState);
+    void prepareReachableNodes_impl(BuildGraphNode *node);
     void prepareProducts();
     void setupRootNodes();
     void initLeaves();
     void updateLeaves(const NodeSet &nodes);
     void updateLeaves(BuildGraphNode *node, NodeSet &seenNodes);
-    void initLeavesForSelectedFiles();
     bool scheduleJobs();
     void buildArtifact(Artifact *artifact);
     void executeRuleNode(RuleNode *ruleNode);
@@ -130,6 +129,8 @@ private:
     void retrieveSourceFileTimestamp(Artifact *artifact) const;
     FileTime recursiveFileTime(const QString &filePath) const;
     QString configString() const;
+    bool transformerHasMatchingOutputTags(const TransformerConstPtr &transformer) const;
+    bool transformerHasMatchingInputFiles(const TransformerConstPtr &transformer) const;
 
     typedef QHash<ExecutorJob *, TransformerPtr> JobMap;
     JobMap m_processingJobs;
diff --git a/src/lib/corelib/tools/buildoptions.cpp b/src/lib/corelib/tools/buildoptions.cpp
index 2261124d8..a2d703363 100644
--- a/src/lib/corelib/tools/buildoptions.cpp
+++ b/src/lib/corelib/tools/buildoptions.cpp
@@ -42,6 +42,7 @@ public:
     }
 
     QStringList changedFiles;
+    QStringList filesToConsider;
     QStringList activeFileTags;
     int maxJobCount;
     bool dryRun;
@@ -98,6 +99,26 @@ void BuildOptions::setChangedFiles(const QStringList &changedFiles)
 }
 
 /*!
+ * \brief The list of files to consider.
+ * \sa setFilesToConsider.
+ * By default, this list is empty.
+ */
+QStringList BuildOptions::filesToConsider() const
+{
+    return d->filesToConsider;
+}
+
+/*!
+ * \brief If the given list is non-empty, qbs will run only commands whose rule has at least one
+ *        of these files as an input.
+ * \note The list elements must be absolute file paths.
+ */
+void BuildOptions::setFilesToConsider(const QStringList &files)
+{
+    d->filesToConsider = files;
+}
+
+/*!
  * \brief The list of active file tags.
  * \sa setActiveFileTags
  */
@@ -109,8 +130,8 @@ QStringList BuildOptions::activeFileTags() const
 /*!
  * \brief Set the list of active file tags.
  * If this list is non-empty, then every transformer with non-matching output file tags is skipped.
- * E.g. set changed files to "foo.cpp" and activeFileTags to ["obj"] to run the compiler
- * on foo.cpp without further processing like linking.
+ * E.g. call \c setFilesToConsider() with "foo.cpp" and \c setActiveFileTags() with "obj" to
+ * run the compiler on foo.cpp without further processing like linking.
  * \sa activeFileTags
  */
 void BuildOptions::setActiveFileTags(const QStringList &fileTags)
diff --git a/src/lib/corelib/tools/buildoptions.h b/src/lib/corelib/tools/buildoptions.h
index 2c277fc3b..c79e6477b 100644
--- a/src/lib/corelib/tools/buildoptions.h
+++ b/src/lib/corelib/tools/buildoptions.h
@@ -45,6 +45,9 @@ public:
     BuildOptions &operator=(const BuildOptions &other);
     ~BuildOptions();
 
+    QStringList filesToConsider() const;
+    void setFilesToConsider(const QStringList &files);
+
     QStringList changedFiles() const;
     void setChangedFiles(const QStringList &changedFiles);
 
diff --git a/src/lib/corelib/tools/filetime.h b/src/lib/corelib/tools/filetime.h
index 0dc0524df..0f16f3ad3 100644
--- a/src/lib/corelib/tools/filetime.h
+++ b/src/lib/corelib/tools/filetime.h
@@ -68,6 +68,7 @@ public:
     QString toString() const;
 
     static FileTime currentTime();
+    static FileTime oldestTime();
 
     friend class FileInfo;
     InternalType m_fileTime;
diff --git a/src/lib/corelib/tools/filetime_unix.cpp b/src/lib/corelib/tools/filetime_unix.cpp
index 945be8f44..5dd176c04 100644
--- a/src/lib/corelib/tools/filetime_unix.cpp
+++ b/src/lib/corelib/tools/filetime_unix.cpp
@@ -62,6 +62,11 @@ FileTime FileTime::currentTime()
     return time(0);
 }
 
+FileTime FileTime::oldestTime()
+{
+    return 1;
+}
+
 QString FileTime::toString() const
 {
     QDateTime dt;
diff --git a/src/lib/corelib/tools/filetime_win.cpp b/src/lib/corelib/tools/filetime_win.cpp
index b3a7fab67..706dd4752 100644
--- a/src/lib/corelib/tools/filetime_win.cpp
+++ b/src/lib/corelib/tools/filetime_win.cpp
@@ -75,6 +75,20 @@ FileTime FileTime::currentTime()
     return result;
 }
 
+FileTime FileTime::oldestTime()
+{
+    SYSTEMTIME st = {
+        1601,
+        1,
+        5,
+        2
+    };
+    FileTime result;
+    FILETIME *const ft = reinterpret_cast<FILETIME *>(&result.m_fileTime);
+    SystemTimeToFileTime(&st, ft);
+    return result;
+}
+
 QString FileTime::toString() const
 {
     const FILETIME *const ft = reinterpret_cast<const FILETIME *>(&m_fileTime);
diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp
index e88246283..53064c6ee 100644
--- a/tests/auto/api/tst_api.cpp
+++ b/tests/auto/api/tst_api.cpp
@@ -112,8 +112,8 @@ void printProjectData(const qbs::ProjectData &project)
 void TestApi::buildSingleFile()
 {
     qbs::SetupProjectParameters setupParams = defaultSetupParameters();
-    setupParams.setProjectFilePath(QDir::cleanPath(m_workingDataDir +
-        "/build-single-file/project.qbs"));
+    const QString projectDirPath = QDir::cleanPath(m_workingDataDir + "/build-single-file");
+    setupParams.setProjectFilePath(projectDirPath + "/project.qbs");
     QScopedPointer<qbs::SetupProjectJob> setupJob(qbs::Project::setupProject(setupParams,
                                                                         m_logSink, 0));
     waitForFinished(setupJob.data());
@@ -121,7 +121,7 @@ void TestApi::buildSingleFile()
     qbs::Project project = setupJob->project();
     qbs::BuildOptions options;
     options.setDryRun(true);
-    options.setChangedFiles(QStringList(m_workingDataDir + "/build-single-file/compiled.cpp"));
+    options.setFilesToConsider(QStringList(projectDirPath + "/compiled.cpp"));
     options.setActiveFileTags(QStringList("obj"));
     m_logSink->setLogLevel(qbs::LoggerMaxLevel);
     QScopedPointer<qbs::BuildJob> buildJob(project.buildAllProducts(options));
@@ -130,7 +130,6 @@ void TestApi::buildSingleFile()
             SLOT(handleDescription(QString,QString)));
     waitForFinished(buildJob.data());
     QVERIFY2(!buildJob->error().hasError(), qPrintable(buildJob->error().toString()));
-    QEXPECT_FAIL(0, "QBS-537", Abort);
     QCOMPARE(receiver.descriptions.count("compiling"), 1);
     QVERIFY2(receiver.descriptions.contains("compiling compiled.cpp"),
              qPrintable(receiver.descriptions));
diff --git a/tests/auto/blackbox/testdata/changed-files/file1.cpp b/tests/auto/blackbox/testdata/changed-files/file1.cpp
new file mode 100644
index 000000000..1d6ea3b78
--- /dev/null
+++ b/tests/auto/blackbox/testdata/changed-files/file1.cpp
@@ -0,0 +1 @@
+void f1() {}
diff --git a/tests/auto/blackbox/testdata/changed-files/file2.cpp b/tests/auto/blackbox/testdata/changed-files/file2.cpp
new file mode 100644
index 000000000..8ccc02b45
--- /dev/null
+++ b/tests/auto/blackbox/testdata/changed-files/file2.cpp
@@ -0,0 +1 @@
+void f2() {}
diff --git a/tests/auto/blackbox/testdata/changed-files/main.cpp b/tests/auto/blackbox/testdata/changed-files/main.cpp
new file mode 100644
index 000000000..237c8ce18
--- /dev/null
+++ b/tests/auto/blackbox/testdata/changed-files/main.cpp
@@ -0,0 +1 @@
+int main() {}
diff --git a/tests/auto/blackbox/testdata/changed-files/project.qbs b/tests/auto/blackbox/testdata/changed-files/project.qbs
new file mode 100644
index 000000000..7fedd66d5
--- /dev/null
+++ b/tests/auto/blackbox/testdata/changed-files/project.qbs
@@ -0,0 +1,5 @@
+import qbs
+
+CppApplication {
+    files: ["file1.cpp", "file2.cpp", "main.cpp"]
+}
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index b28e7ba8d..c106d5d16 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -363,6 +363,26 @@ void TestBlackbox::changeDependentLib()
     QCOMPARE(runQbs(), 0);
 }
 
+void TestBlackbox::changedFiles()
+{
+    QDir::setCurrent(testDataDir + "/changed-files");
+    const QString changedFile = QDir::cleanPath(QDir::currentPath() + "/file1.cpp");
+    QbsRunParameters params(QStringList("--changed-files") << changedFile);
+
+    // Initial run: Build all files, even though only one of them was marked as changed.
+    QCOMPARE(runQbs(params), 0);
+    QCOMPARE(m_qbsStdout.count("compiling"), 3);
+
+    waitForNewTimestamp();
+    touch(QDir::currentPath() + "/main.cpp");
+
+    // Now only the file marked as changed must be compiled, even though it hasn't really
+    // changed and another one has.
+    QCOMPARE(runQbs(params), 0);
+    QCOMPARE(m_qbsStdout.count("compiling"), 1);
+    QVERIFY2(m_qbsStdout.contains("file1.cpp"), m_qbsStdout.constData());
+}
+
 void TestBlackbox::dependenciesProperty()
 {
     QDir::setCurrent(testDataDir + QLatin1String("/dependenciesProperty"));
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index b29b86c52..41b48aa49 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -104,6 +104,7 @@ private slots:
     void build_project_dry_run_data();
     void build_project_dry_run();
     void changeDependentLib();
+    void changedFiles();
     void dependenciesProperty();
     void disabledProduct();
     void disabledProject();