Fix behavior with corrupt or outdated build graphs

Commit b6bf17cfd1 introduced a glitch in that a rebuild using a build
graph whose magic string does not match would throw an error at the
first attempt and silently succeed at the second one. The new behavior
is the same as for attempts to rebuild with property changes: A plain
"build" fails, a "resolve" does the trick. Some more code had to move
from the command line parser to the loader to achieve this.

Change-Id: I906748cc1686c0c5d413d9451ec9e2aa26ea2035
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

17 files changed, 149 insertions, 88 deletions

diff --git a/src/app/qbs/parser/commandlineparser.cpp b/src/app/qbs/parser/commandlineparser.cpp
index 8ff4a83ed..6e45aeb0a 100644
--- a/src/app/qbs/parser/commandlineparser.cpp
+++ b/src/app/qbs/parser/commandlineparser.cpp
@@ -428,66 +428,6 @@ QString CommandLineParser::CommandLineParserPrivate::generalHelp() const
 void CommandLineParser::CommandLineParserPrivate::setupProjectFile()
 {
     projectFilePath = optionPool.fileOption()->projectFilePath();
-    if (projectFilePath.isEmpty()) {
-        bool allBuildGraphsExist = true;
-        foreach (const QVariantMap &buildConfig, buildConfigurations) {
-            const QString configName = buildConfig.value(QLatin1String("qbs.configurationName"))
-                    .toString();
-            const QString profile = buildConfig.value(QLatin1String("qbs.profile")).toString();
-            QString buildDir = projectBuildDirectory;
-            Settings settings(settingsDir());
-            if (buildDir.isEmpty()) {
-                buildDir = Preferences(&settings, profile).defaultBuildDirectory();
-                if (buildDir.isEmpty())
-                    buildDir = QDir::currentPath();
-            }
-            if (!QFile::exists(buildDir + QLatin1Char('/') + configName + QLatin1Char('/')
-                               + configName + QLatin1String(".bg"))) {
-                allBuildGraphsExist = false;
-                break;
-            }
-        }
-        if (allBuildGraphsExist) {
-            qbsDebug() << "No project file given; using the one from the build graph.";
-            return;
-        }
-        qbsDebug() << "No project file given; looking in current directory.";
-        projectFilePath = QDir::currentPath();
-    }
-
-    const QFileInfo projectFileInfo(projectFilePath);
-    if (!projectFileInfo.exists())
-        throw ErrorInfo(Tr::tr("Project file '%1' cannot be found.").arg(projectFilePath));
-    if (projectFileInfo.isRelative())
-        projectFilePath = projectFileInfo.absoluteFilePath();
-    if (projectFileInfo.isFile())
-        return;
-    if (!projectFileInfo.isDir())
-        throw ErrorInfo(Tr::tr("Project file '%1' has invalid type.").arg(projectFilePath));
-
-    const QStringList namePatterns = QStringList()
-            << QLatin1String("*.qbs");
-
-    const QStringList &actualFileNames
-            = QDir(projectFilePath).entryList(namePatterns, QDir::Files);
-    if (actualFileNames.isEmpty()) {
-        QString error;
-        if (optionPool.fileOption()->projectFilePath().isEmpty())
-            error = Tr::tr("No project file given and none found in current directory.\n");
-        else
-            error = Tr::tr("No project file found in directory '%1'.").arg(projectFilePath);
-        throw ErrorInfo(error);
-    }
-    if (actualFileNames.count() > 1) {
-        throw ErrorInfo(Tr::tr("More than one project file found in directory '%1'.")
-                .arg(projectFilePath));
-    }
-    projectFilePath.append(QLatin1Char('/')).append(actualFileNames.first());
-
-    projectFilePath = QDir::current().filePath(projectFilePath);
-    projectFilePath = QDir::cleanPath(projectFilePath);
-
-    qbsDebug() << "Using project file '" << QDir::toNativeSeparators(projectFilePath) << "'.";
 }
 
 void CommandLineParser::CommandLineParserPrivate::setupBuildDirectory()
diff --git a/src/lib/corelib/buildgraph/buildgraphloader.cpp b/src/lib/corelib/buildgraph/buildgraphloader.cpp
index 6b094164a..e7c93c6fd 100644
--- a/src/lib/corelib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/corelib/buildgraph/buildgraphloader.cpp
@@ -166,12 +166,18 @@ void BuildGraphLoader::loadBuildGraphFromDisk()
     m_logger.qbsDebug() << "[BG] trying to load: " << buildGraphFilePath;
     try {
         pool.load(buildGraphFilePath);
-    } catch (const ErrorInfo &loadError) {
-        if (m_parameters.restoreBehavior() == SetupProjectParameters::RestoreOnly
-                || m_parameters.projectFilePath().isEmpty()) {
+    } catch (const NoBuildGraphError &e) {
+        if (m_parameters.restoreBehavior() == SetupProjectParameters::RestoreOnly)
             throw;
+        m_logger.qbsInfo() << e.toString();
+        return;
+    } catch (const ErrorInfo &loadError) {
+        if (!m_parameters.overrideBuildGraphData()) {
+            ErrorInfo fullError = loadError;
+            fullError.append(Tr::tr("Use the 'resolve' command to set up a new build graph."));
+            throw fullError;
         }
-        m_logger.qbsInfo() << loadError.toString();
+        m_logger.qbsWarning() << loadError.toString();
         return;
     }
 
@@ -198,6 +204,8 @@ bool BuildGraphLoader::checkBuildGraphCompatibility(const TopLevelProjectConstPt
 {
     if (m_parameters.projectFilePath().isEmpty())
         m_parameters.setProjectFilePath(project->location.filePath());
+    else
+        Loader::setupProjectFilePath(m_parameters);
     if (QFileInfo(project->location.filePath()) == QFileInfo(m_parameters.projectFilePath()))
         return true;
     QString message = Tr::tr("Stored build graph at '%1' is for project file '%2', but "
diff --git a/src/lib/corelib/language/loader.cpp b/src/lib/corelib/language/loader.cpp
index 821def454..d7da2ca67 100644
--- a/src/lib/corelib/language/loader.cpp
+++ b/src/lib/corelib/language/loader.cpp
@@ -105,9 +105,8 @@ void Loader::setStoredProfiles(const QVariantMap &profiles)
 
 TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters &_parameters)
 {
-    QBS_CHECK(QFileInfo(_parameters.projectFilePath()).isAbsolute());
-
     SetupProjectParameters parameters = _parameters;
+
     if (parameters.topLevelProfile().isEmpty()) {
         Settings settings(parameters.settingsDirectory());
         QString profileName = settings.defaultProfile();
@@ -119,6 +118,12 @@ TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters &_parameters
         parameters.setTopLevelProfile(profileName);
         parameters.expandBuildConfiguration();
     }
+
+    setupProjectFilePath(parameters);
+    QBS_CHECK(QFileInfo(parameters.projectFilePath()).isAbsolute());
+    m_logger.qbsDebug() << "Using project file '"
+                        << QDir::toNativeSeparators(parameters.projectFilePath()) << "'.";
+
     m_engine->setEnvironment(parameters.adjustedEnvironment());
     m_engine->clearExceptions();
     m_engine->clearImportsCache();
@@ -166,5 +171,45 @@ TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters &_parameters
     return project;
 }
 
+void Loader::setupProjectFilePath(SetupProjectParameters &parameters)
+{
+    QString projectFilePath = parameters.projectFilePath();
+    if (projectFilePath.isEmpty())
+        projectFilePath = QDir::currentPath();
+    const QFileInfo projectFileInfo(projectFilePath);
+    if (!projectFileInfo.exists())
+        throw ErrorInfo(Tr::tr("Project file '%1' cannot be found.").arg(projectFilePath));
+    if (projectFileInfo.isRelative())
+        projectFilePath = projectFileInfo.absoluteFilePath();
+    if (projectFileInfo.isFile()) {
+        parameters.setProjectFilePath(projectFilePath);
+        return;
+    }
+    if (!projectFileInfo.isDir())
+        throw ErrorInfo(Tr::tr("Project file '%1' has invalid type.").arg(projectFilePath));
+
+    const QStringList namePatterns = QStringList()
+            << QLatin1String("*.qbs");
+    const QStringList &actualFileNames
+            = QDir(projectFilePath).entryList(namePatterns, QDir::Files);
+    if (actualFileNames.isEmpty()) {
+        QString error;
+        if (parameters.projectFilePath().isEmpty())
+            error = Tr::tr("No project file given and none found in current directory.\n");
+        else
+            error = Tr::tr("No project file found in directory '%1'.").arg(projectFilePath);
+        throw ErrorInfo(error);
+    }
+    if (actualFileNames.count() > 1) {
+        throw ErrorInfo(Tr::tr("More than one project file found in directory '%1'.")
+                .arg(projectFilePath));
+    }
+    projectFilePath.append(QLatin1Char('/')).append(actualFileNames.first());
+
+    projectFilePath = QDir::current().filePath(projectFilePath);
+    projectFilePath = QDir::cleanPath(projectFilePath);
+    parameters.setProjectFilePath(projectFilePath);
+}
+
 } // namespace Internal
 } // namespace qbs
diff --git a/src/lib/corelib/language/loader.h b/src/lib/corelib/language/loader.h
index b94fb7553..c07380639 100644
--- a/src/lib/corelib/language/loader.h
+++ b/src/lib/corelib/language/loader.h
@@ -64,6 +64,8 @@ public:
     void setStoredProfiles(const QVariantMap &profiles);
     TopLevelProjectPtr loadProject(const SetupProjectParameters &parameters);
 
+    static void setupProjectFilePath(SetupProjectParameters &parameters);
+
 private:
     Logger m_logger;
     ProgressObserver *m_progressObserver;
diff --git a/src/lib/corelib/tools/persistence.cpp b/src/lib/corelib/tools/persistence.cpp
index 7cad14752..56868cae7 100644
--- a/src/lib/corelib/tools/persistence.cpp
+++ b/src/lib/corelib/tools/persistence.cpp
@@ -52,6 +52,11 @@ namespace Internal {
 
 static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-103";
 
+NoBuildGraphError::NoBuildGraphError()
+    : ErrorInfo(Tr::tr("No build graph exists yet for this configuration."))
+{
+}
+
 PersistentPool::PersistentPool(Logger &logger) : m_logger(logger)
 {
     Q_UNUSED(m_logger);
@@ -67,7 +72,7 @@ void PersistentPool::load(const QString &filePath)
 {
     QScopedPointer<QFile> file(new QFile(filePath));
     if (!file->exists())
-        throw ErrorInfo(Tr::tr("No build graph exists yet for this configuration."));
+        throw NoBuildGraphError();
     if (!file->open(QFile::ReadOnly)) {
         throw ErrorInfo(Tr::tr("Could not open open build graph file '%1': %2")
                     .arg(filePath, file->errorString()));
@@ -77,8 +82,6 @@ void PersistentPool::load(const QString &filePath)
     QByteArray magic;
     m_stream >> magic;
     if (magic != QBS_PERSISTENCE_MAGIC) {
-        file->close();
-        file->remove();
         m_stream.setDevice(0);
         throw ErrorInfo(Tr::tr("Cannot use stored build graph at '%1': Incompatible file format. "
                            "Expected magic token '%2', got '%3'.")
diff --git a/src/lib/corelib/tools/persistence.h b/src/lib/corelib/tools/persistence.h
index 5ae37eab6..f0d45c83e 100644
--- a/src/lib/corelib/tools/persistence.h
+++ b/src/lib/corelib/tools/persistence.h
@@ -40,6 +40,7 @@
 #ifndef QBS_PERSISTENCE
 #define QBS_PERSISTENCE
 
+#include "error.h"
 #include "persistentobject.h"
 #include <logging/logger.h>
 
@@ -56,6 +57,12 @@
 namespace qbs {
 namespace Internal {
 
+class NoBuildGraphError : public ErrorInfo
+{
+public:
+    NoBuildGraphError();
+};
+
 class PersistentPool
 {
 public:
diff --git a/tests/auto/blackbox/testdata/build-graph-versions/build-graph-versions.qbs b/tests/auto/blackbox/testdata/build-graph-versions/build-graph-versions.qbs
new file mode 100644
index 000000000..f6ae698a0
--- /dev/null
+++ b/tests/auto/blackbox/testdata/build-graph-versions/build-graph-versions.qbs
@@ -0,0 +1,5 @@
+import qbs
+
+CppApplication {
+    files: ["main.cpp"]
+}
diff --git a/tests/auto/blackbox/testdata/build-graph-versions/main.cpp b/tests/auto/blackbox/testdata/build-graph-versions/main.cpp
new file mode 100644
index 000000000..8b8d58de0
--- /dev/null
+++ b/tests/auto/blackbox/testdata/build-graph-versions/main.cpp
@@ -0,0 +1 @@
+int main() { }
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 77a6f8a81..3acf4ee37 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -485,6 +485,36 @@ void TestBlackbox::buildEnvChange()
     QVERIFY(runQbs(params) != 0);
 }
 
+void TestBlackbox::buildGraphVersions()
+{
+    QDir::setCurrent(testDataDir + "/build-graph-versions");
+    QCOMPARE(runQbs(), 0);
+    QVERIFY2(m_qbsStdout.contains("compiling main.cpp"), m_qbsStdout.constData());
+    QFile bgFile(relativeBuildGraphFilePath());
+    QVERIFY2(bgFile.open(QIODevice::ReadWrite), qPrintable(bgFile.errorString()));
+    bgFile.write("blubb");
+    bgFile.close();
+
+    // The first attempt at simple rebuilding as well as subsequent ones must fail.
+    QbsRunParameters params;
+    params.expectFailure = true;
+    QVERIFY(runQbs(params) != 0);
+    QVERIFY2(m_qbsStderr.contains("Cannot use stored build graph"), m_qbsStderr.constData());
+    QVERIFY2(m_qbsStderr.contains("Use the 'resolve' command"), m_qbsStderr.constData());
+    QVERIFY(runQbs(params) != 0);
+    QVERIFY2(m_qbsStderr.contains("Cannot use stored build graph"), m_qbsStderr.constData());
+    QVERIFY2(m_qbsStderr.contains("Use the 'resolve' command"), m_qbsStderr.constData());
+
+    // On re-resolving, the error turns into a warning and a new build graph is created.
+    QCOMPARE(runQbs(QbsRunParameters("resolve")), 0);
+    QVERIFY2(m_qbsStderr.contains("Cannot use stored build graph"), m_qbsStderr.constData());
+    QVERIFY2(!m_qbsStderr.contains("Use the 'resolve' command"), m_qbsStderr.constData());
+
+    QCOMPARE(runQbs(), 0);
+    QVERIFY2(!m_qbsStderr.contains("Cannot use stored build graph"), m_qbsStderr.constData());
+    QVERIFY2(m_qbsStdout.contains("compiling main.cpp"), m_qbsStdout.constData());
+}
+
 void TestBlackbox::changedFiles_data()
 {
     QTest::addColumn<bool>("useChangedFilesForInitialBuild");
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index 77f5762fb..4ae313946 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -47,6 +47,7 @@ private slots:
     void badInterpreter();
     void buildDirectories();
     void buildEnvChange();
+    void buildGraphVersions();
     void changedFiles_data();
     void changedFiles();
     void changeInDisabledProduct();
diff --git a/tests/auto/cmdlineparser/tst_cmdlineparser.cpp b/tests/auto/cmdlineparser/tst_cmdlineparser.cpp
index 92a202a45..de8ecc0b6 100644
--- a/tests/auto/cmdlineparser/tst_cmdlineparser.cpp
+++ b/tests/auto/cmdlineparser/tst_cmdlineparser.cpp
@@ -165,25 +165,6 @@ private slots:
         QVERIFY(!parser.parseCommandLine(QStringList() << fileArgs << "-123")); // Unknown numeric argument.
     }
 
-    void testProjectFileLookup()
-    {
-        const QString srcDir = QLatin1String(SRCDIR);
-        const QString noProjectsDir = srcDir + QLatin1String("/data/dirwithnoprojects");
-        const QString oneProjectDir = srcDir + QLatin1String("/data/dirwithoneproject");
-        const QString multiProjectsDir = srcDir + QLatin1String("/data/dirwithmultipleprojects");
-        QVERIFY(QDir(noProjectsDir).exists() && QDir(oneProjectDir).exists()
-                && QDir(multiProjectsDir).exists());
-        CommandLineParser parser;
-        const QStringList args(QLatin1String("-f"));
-        QString projectFilePath = multiProjectsDir + QLatin1String("/project.qbs");
-        QVERIFY(parser.parseCommandLine(args + QStringList(projectFilePath)));
-        QCOMPARE(projectFilePath, parser.projectFilePath());
-        projectFilePath = oneProjectDir + QLatin1String("/project.qbs");
-        QVERIFY(parser.parseCommandLine(args + QStringList(oneProjectDir)));
-        QCOMPARE(projectFilePath, parser.projectFilePath());
-        QVERIFY(!parser.parseCommandLine(args + QStringList(noProjectsDir)));
-        QVERIFY(!parser.parseCommandLine(args + QStringList(multiProjectsDir)));
-    }
 };
 
 QTEST_MAIN(TestCmdLineParser)
diff --git a/tests/auto/cmdlineparser/data/dirwithmultipleprojects/project.qbs b/tests/auto/language/testdata/dirwithmultipleprojects/project.qbs
index e69de29bb..e69de29bb 100644
--- a/tests/auto/cmdlineparser/data/dirwithmultipleprojects/project.qbs
+++ b/tests/auto/language/testdata/dirwithmultipleprojects/project.qbs
diff --git a/tests/auto/cmdlineparser/data/dirwithmultipleprojects/project2.qbs b/tests/auto/language/testdata/dirwithmultipleprojects/project2.qbs
index e69de29bb..e69de29bb 100644
--- a/tests/auto/cmdlineparser/data/dirwithmultipleprojects/project2.qbs
+++ b/tests/auto/language/testdata/dirwithmultipleprojects/project2.qbs
diff --git a/tests/auto/cmdlineparser/data/dirwithnoprojects/.gitignore b/tests/auto/language/testdata/dirwithnoprojects/.gitignore
index d6b7ef32c..d6b7ef32c 100644
--- a/tests/auto/cmdlineparser/data/dirwithnoprojects/.gitignore
+++ b/tests/auto/language/testdata/dirwithnoprojects/.gitignore
diff --git a/tests/auto/cmdlineparser/data/dirwithoneproject/project.qbs b/tests/auto/language/testdata/dirwithoneproject/project.qbs
index e69de29bb..e69de29bb 100644
--- a/tests/auto/cmdlineparser/data/dirwithoneproject/project.qbs
+++ b/tests/auto/language/testdata/dirwithoneproject/project.qbs
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index fdde1fae3..fdcd98da8 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -1797,6 +1797,42 @@ void TestLanguage::profileValuesAndOverriddenValues()
     QCOMPARE(exceptionCaught, false);
 }
 
+void TestLanguage::projectFileLookup()
+{
+    QFETCH(QString, projectFileInput);
+    QFETCH(QString, projectFileOutput);
+    QFETCH(bool, failureExpected);
+
+    try {
+        SetupProjectParameters params;
+        params.setProjectFilePath(projectFileInput);
+        Loader::setupProjectFilePath(params);
+        QVERIFY(!failureExpected);
+        QCOMPARE(params.projectFilePath(), projectFileOutput);
+    } catch (const ErrorInfo &) {
+        QVERIFY(failureExpected);
+    }
+}
+
+void TestLanguage::projectFileLookup_data()
+{
+    QTest::addColumn<QString>("projectFileInput");
+    QTest::addColumn<QString>("projectFileOutput");
+    QTest::addColumn<bool>("failureExpected");
+
+    const QString baseDir = QLatin1String(SRCDIR) + "/testdata";
+    const QString multiProjectsDir = baseDir + "/dirwithmultipleprojects";
+    const QString noProjectsDir = baseDir + "/dirwithnoprojects";
+    const QString oneProjectDir = baseDir + "/dirwithoneproject";
+    QVERIFY(QDir(noProjectsDir).exists() && QDir(oneProjectDir).exists()
+            && QDir(multiProjectsDir).exists());
+    const QString fullFilePath = multiProjectsDir + "/project.qbs";
+    QTest::newRow("full file path") << fullFilePath << fullFilePath << false;
+    QTest::newRow("base dir ") << oneProjectDir << (oneProjectDir + "/project.qbs") << false;
+    QTest::newRow("empty dir") << noProjectsDir << QString() << true;
+    QTest::newRow("ambiguous dir") << multiProjectsDir << QString() << true;
+}
+
 void TestLanguage::productConditions()
 {
     bool exceptionCaught = false;
diff --git a/tests/auto/language/tst_language.h b/tests/auto/language/tst_language.h
index 9d63414a5..62805ec2f 100644
--- a/tests/auto/language/tst_language.h
+++ b/tests/auto/language/tst_language.h
@@ -126,6 +126,8 @@ private slots:
     void productConditions();
     void productDirectories();
     void profileValuesAndOverriddenValues();
+    void projectFileLookup();
+    void projectFileLookup_data();
     void propertiesBlocks_data();
     void propertiesBlocks();
     void propertiesBlockInGroup();