diff options
author | Christian Kandeler <christian.kandeler@theqtcompany.com> | 2016-06-02 14:52:53 +0200 |
---|---|---|
committer | Christian Kandeler <christian.kandeler@theqtcompany.com> | 2016-06-02 14:28:45 +0000 |
commit | d8d7beb866b24793d9c04b6996276a4a8959bfa2 (patch) | |
tree | 73c378521d981a318ff006574ca4e85b4f23b59f | |
parent | 161a751d53b97dd706ac0108432cdc71c4c05c70 (diff) |
Executor: Do not assert on condition that can actually happen.v1.5.1
We asserted on the condition that a node's product must be in the list
of products to build, meaning that it needs to belong to the set of
user-selected products or one of their dependencies. However, there are
at least two ways in which one can trigger this condition to be false:
1) The user provides a faulty project in which a "Depends" item is
missing, but the respective artifact from the other product is still
found (e.g. a generated header file via a project-global include
path).
2) The project is actually okay, but our C++ scanner erroneously adds
dependencies from a different product, e.g. because it does not know
about #ifdefs.
Instead of the assertion, we now simply mark the respective node as
built and continue. Ideally, we'd log a warning, but we cannot do that
because of point 2) above.
Change-Id: I3549d732dea5cde84d1019132580a8e051c9db11
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>
-rw-r--r-- | src/lib/corelib/buildgraph/executor.cpp | 23 | ||||
-rw-r--r-- | src/lib/corelib/buildgraph/executor.h | 1 | ||||
-rw-r--r-- | tests/auto/blackbox/testdata/missing-dependency/main.cpp | 5 | ||||
-rw-r--r-- | tests/auto/blackbox/testdata/missing-dependency/missing-dependency.qbs | 39 | ||||
-rw-r--r-- | tests/auto/blackbox/testdata/missing-dependency/theHeader.h.in | 0 | ||||
-rw-r--r-- | tests/auto/blackbox/tst_blackbox.cpp | 15 | ||||
-rw-r--r-- | tests/auto/blackbox/tst_blackbox.h | 1 |
7 files changed, 81 insertions, 3 deletions
diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp index be5a93302..2706d0323 100644 --- a/src/lib/corelib/buildgraph/executor.cpp +++ b/src/lib/corelib/buildgraph/executor.cpp @@ -408,6 +408,9 @@ void Executor::buildArtifact(Artifact *artifact) QBS_CHECK(artifact->buildState == BuildGraphNode::Buildable); + if (artifact->artifactType != Artifact::SourceFile && !checkNodeProduct(artifact)) + return; + // skip artifacts without transformer if (artifact->artifactType != Artifact::Generated) { // For source artifacts, that were not reachable when initializing the build, we must @@ -430,6 +433,9 @@ void Executor::buildArtifact(Artifact *artifact) void Executor::executeRuleNode(RuleNode *ruleNode) { + if (!checkNodeProduct(ruleNode)) + return; + QBS_CHECK(!m_evalContext->isActive()); ArtifactSet changedInputArtifacts; if (ruleNode->rule()->isDynamic()) { @@ -972,6 +978,20 @@ void Executor::checkForUnbuiltProducts() } } +bool Executor::checkNodeProduct(BuildGraphNode *node) +{ + if (m_productsToBuild.contains(node->product)) + return true; + + // TODO: Turn this into a warning once we have a reliable C++ scanner. + m_logger.qbsTrace() << "Ignoring node " << node->toString() << ", which belongs to a " + "product that is not part of the list of products to build. " + "Possible reasons are an erroneous project design or a false " + "positive from a dependency scanner."; + finishNode(node); + return false; +} + void Executor::finish() { QBS_ASSERT(m_state != ExecutorIdle, /* ignore */); @@ -1007,8 +1027,6 @@ void Executor::checkForCancellation() bool Executor::visit(Artifact *artifact) { QBS_CHECK(artifact->buildState != BuildGraphNode::Untouched); - QBS_CHECK(artifact->artifactType == Artifact::SourceFile - || m_productsToBuild.contains(artifact->product)); buildArtifact(artifact); return false; } @@ -1016,7 +1034,6 @@ bool Executor::visit(Artifact *artifact) bool Executor::visit(RuleNode *ruleNode) { QBS_CHECK(ruleNode->buildState != BuildGraphNode::Untouched); - QBS_CHECK(m_productsToBuild.contains(ruleNode->product)); executeRuleNode(ruleNode); return false; } diff --git a/src/lib/corelib/buildgraph/executor.h b/src/lib/corelib/buildgraph/executor.h index d626cca3d..2018357b6 100644 --- a/src/lib/corelib/buildgraph/executor.h +++ b/src/lib/corelib/buildgraph/executor.h @@ -131,6 +131,7 @@ private: void finishTransformer(const TransformerPtr &transformer); void possiblyInstallArtifact(const Artifact *artifact); void checkForUnbuiltProducts(); + bool checkNodeProduct(BuildGraphNode *node); bool mustExecuteTransformer(const TransformerPtr &transformer) const; bool isUpToDate(Artifact *artifact) const; diff --git a/tests/auto/blackbox/testdata/missing-dependency/main.cpp b/tests/auto/blackbox/testdata/missing-dependency/main.cpp new file mode 100644 index 000000000..ee2bdf30e --- /dev/null +++ b/tests/auto/blackbox/testdata/missing-dependency/main.cpp @@ -0,0 +1,5 @@ +#include <theHeader.h> + +int main() +{ +} diff --git a/tests/auto/blackbox/testdata/missing-dependency/missing-dependency.qbs b/tests/auto/blackbox/testdata/missing-dependency/missing-dependency.qbs new file mode 100644 index 000000000..e3a37d415 --- /dev/null +++ b/tests/auto/blackbox/testdata/missing-dependency/missing-dependency.qbs @@ -0,0 +1,39 @@ +import qbs +import qbs.TextFile + +Project { + Product { + name: "theDep" + type: ["genheader"] + + // TODO: Remove in 1.6 + Group { + files: ["theHeader.h.in"] + fileTags: ["header.in"] + } + + Rule { + inputs: ["header.in"] + Artifact { + filePath: project.buildDirectory + "/theHeader.h" + fileTags: product.type + } + prepare: { + var cmd = new JavaScriptCommand(); + cmd.silent = true; + cmd.sourceCode = function() { + var f = new TextFile(output.filePath, TextFile.WriteOnly); + f.close(); + } + return [cmd]; + } + } + } + CppApplication { + name: "theApp" + cpp.includePaths: [project.buildDirectory] + files: ["main.cpp"] + } +} + + diff --git a/tests/auto/blackbox/testdata/missing-dependency/theHeader.h.in b/tests/auto/blackbox/testdata/missing-dependency/theHeader.h.in new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/tests/auto/blackbox/testdata/missing-dependency/theHeader.h.in diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index e1f05b534..a46d02fe5 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -3880,6 +3880,21 @@ void TestBlackbox::lrelease() QVERIFY(!regularFileExists(relativeProductBuildDir("lrelease-test") + "/hu.qm")); } +void TestBlackbox::missingDependency() +{ + QDir::setCurrent(testDataDir + "/missing-dependency"); + QbsRunParameters params; + params.expectFailure = true; + params.arguments << "-p" << "theApp"; + QVERIFY(runQbs(params) != 0); + QVERIFY2(!m_qbsStderr.contains("ASSERT"), m_qbsStderr.constData()); + QCOMPARE(runQbs(QbsRunParameters(QStringList() << "-p" << "theDep")), 0); + params.expectFailure = false; + params.arguments << "-vv"; + QCOMPARE(runQbs(params), 0); + QVERIFY(m_qbsStderr.contains("false positive")); +} + void TestBlackbox::badInterpreter() { if (!HostOsInfo::isAnyUnixHost()) diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h index be4c3a8fc..22a435c07 100644 --- a/tests/auto/blackbox/tst_blackbox.h +++ b/tests/auto/blackbox/tst_blackbox.h @@ -160,6 +160,7 @@ private slots: void listPropertyOrder(); void loadableModule(); void lrelease(); + void missingDependency(); void missingProfile(); void mixedBuildVariants(); void multipleChanges(); |