From 40215ac87a15c41c6531714da2d43728df75235b Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 6 Feb 2019 10:11:09 +0100 Subject: Module providers: Fix some change tracking problems - We must not remember our temporary files. - We need to use a different reference time stamp for the created modules. - Collecting the results of File.exists() & friends can lead to false positives, so we disable it for now. Change-Id: Id64685b510606f1991e83eb825c36a1b3ec4a4e1 Reviewed-by: Joerg Bornemann --- src/lib/corelib/api/project.cpp | 8 ++++---- src/lib/corelib/buildgraph/buildgraphloader.cpp | 24 ++++++++++++++++++------ src/lib/corelib/buildgraph/buildgraphloader.h | 2 +- src/lib/corelib/language/language.cpp | 2 +- src/lib/corelib/language/language.h | 7 ++++--- src/lib/corelib/language/loader.cpp | 3 ++- src/lib/corelib/language/moduleloader.cpp | 15 ++++++++------- src/lib/corelib/language/moduleloader.h | 1 + src/lib/corelib/language/scriptengine.cpp | 24 ++++++++++++++++++------ src/lib/corelib/language/scriptengine.h | 6 +++++- src/lib/corelib/tools/persistence.cpp | 2 +- tests/auto/blackbox/tst_blackbox.cpp | 3 ++- 12 files changed, 65 insertions(+), 32 deletions(-) diff --git a/src/lib/corelib/api/project.cpp b/src/lib/corelib/api/project.cpp index d1cb351be..f8d50df9b 100644 --- a/src/lib/corelib/api/project.cpp +++ b/src/lib/corelib/api/project.cpp @@ -1256,7 +1256,7 @@ ErrorInfo Project::addGroup(const ProductData &product, const QString &groupName QBS_CHECK(isValid()); d->prepareChangeToProject(); d->addGroup(product, groupName); - d->internalProject->lastResolveTime = FileTime::currentTime(); + d->internalProject->lastStartResolveTime = FileTime::currentTime(); d->internalProject->store(d->logger); return ErrorInfo(); } catch (ErrorInfo errorInfo) { @@ -1283,7 +1283,7 @@ ErrorInfo Project::addFiles(const ProductData &product, const GroupData &group, QBS_CHECK(isValid()); d->prepareChangeToProject(); d->addFiles(product, group, filePaths); - d->internalProject->lastResolveTime = FileTime::currentTime(); + d->internalProject->lastStartResolveTime = FileTime::currentTime(); d->internalProject->store(d->logger); return ErrorInfo(); } catch (ErrorInfo errorInfo) { @@ -1309,7 +1309,7 @@ ErrorInfo Project::removeFiles(const ProductData &product, const GroupData &grou QBS_CHECK(isValid()); d->prepareChangeToProject(); d->removeFiles(product, group, filePaths); - d->internalProject->lastResolveTime = FileTime::currentTime(); + d->internalProject->lastStartResolveTime = FileTime::currentTime(); d->internalProject->store(d->logger); return ErrorInfo(); } catch (ErrorInfo errorInfo) { @@ -1330,7 +1330,7 @@ ErrorInfo Project::removeGroup(const ProductData &product, const GroupData &grou QBS_CHECK(isValid()); d->prepareChangeToProject(); d->removeGroup(product, group); - d->internalProject->lastResolveTime = FileTime::currentTime(); + d->internalProject->lastStartResolveTime = FileTime::currentTime(); d->internalProject->store(d->logger); return ErrorInfo(); } catch (ErrorInfo errorInfo) { diff --git a/src/lib/corelib/buildgraph/buildgraphloader.cpp b/src/lib/corelib/buildgraph/buildgraphloader.cpp index e6d1cb75a..51f7403b0 100644 --- a/src/lib/corelib/buildgraph/buildgraphloader.cpp +++ b/src/lib/corelib/buildgraph/buildgraphloader.cpp @@ -307,7 +307,7 @@ void BuildGraphLoader::trackProjectChanges() bool reResolvingNecessary = false; if (!checkConfigCompatibility()) reResolvingNecessary = true; - if (hasProductFileChanged(allRestoredProducts, restoredProject->lastResolveTime, + if (hasProductFileChanged(allRestoredProducts, restoredProject->lastStartResolveTime, buildSystemFiles, changedProducts)) { reResolvingNecessary = true; } @@ -317,7 +317,7 @@ void BuildGraphLoader::trackProjectChanges() // having been touched. In such a case, the build data for that product will have to be set up // anew. if (probeExecutionForced(restoredProject, allRestoredProducts) - || hasBuildSystemFileChanged(buildSystemFiles, restoredProject->lastResolveTime) + || hasBuildSystemFileChanged(buildSystemFiles, restoredProject.get()) || hasEnvironmentChanged(restoredProject) || hasCanonicalFilePathResultChanged(restoredProject) || hasFileExistsResultChanged(restoredProject) @@ -342,7 +342,7 @@ void BuildGraphLoader::trackProjectChanges() ldr.setOldProjectProbes(restoredProject->probes); if (!m_parameters.forceProbeExecution()) ldr.setStoredModuleProviderInfo(restoredProject->moduleProviderInfo); - ldr.setLastResolveTime(restoredProject->lastResolveTime); + ldr.setLastResolveTime(restoredProject->lastStartResolveTime); QHash> restoredProbes; for (const auto &restoredProduct : qAsConst(allRestoredProducts)) restoredProbes.insert(restoredProduct->uniqueName(), restoredProduct->probes); @@ -630,12 +630,24 @@ bool BuildGraphLoader::hasProductFileChanged(const std::vector &buildSystemFiles, - const FileTime &referenceTime) + const TopLevelProject *restoredProject) { for (const QString &file : buildSystemFiles) { const FileInfo fi(file); - if (!fi.exists() || referenceTime < fi.lastModified()) { - qCDebug(lcBuildGraph) << "A qbs or js file changed, must re-resolve project."; + if (!fi.exists()) { + qCDebug(lcBuildGraph) << "Project file" << file + << "no longer exists, must re-resolve project."; + return true; + } + const auto generatedChecker = [&file, restoredProject](const ModuleProviderInfo &mpi) { + return file.startsWith(mpi.outputDirPath(restoredProject->buildDirectory)); + }; + const bool fileWasCreatedByModuleProvider = any_of(restoredProject->moduleProviderInfo, + generatedChecker); + const FileTime referenceTime = fileWasCreatedByModuleProvider + ? restoredProject->lastEndResolveTime : restoredProject->lastStartResolveTime; + if (referenceTime < fi.lastModified()) { + qCDebug(lcBuildGraph) << "Project file" << file << "changed, must re-resolve project."; return true; } } diff --git a/src/lib/corelib/buildgraph/buildgraphloader.h b/src/lib/corelib/buildgraph/buildgraphloader.h index 30d7b9b45..761c80603 100644 --- a/src/lib/corelib/buildgraph/buildgraphloader.h +++ b/src/lib/corelib/buildgraph/buildgraphloader.h @@ -97,7 +97,7 @@ private: Set &remainingBuildSystemFiles, std::vector &productsWithChangedFiles); bool hasBuildSystemFileChanged(const Set &buildSystemFiles, - const FileTime &referenceTime); + const TopLevelProject *restoredProject); void markTransformersForChangeTracking(const std::vector &restoredProducts); void checkAllProductsForChanges(const std::vector &restoredProducts, std::vector &changedProducts); diff --git a/src/lib/corelib/language/language.cpp b/src/lib/corelib/language/language.cpp index 2a9eb5b0e..86056f2a6 100644 --- a/src/lib/corelib/language/language.cpp +++ b/src/lib/corelib/language/language.cpp @@ -588,7 +588,7 @@ void ResolvedProject::store(PersistentPool &pool) TopLevelProject::TopLevelProject() - : bgLocker(nullptr), locked(false), lastResolveTime(FileTime::oldestTime()) + : bgLocker(nullptr), locked(false), lastStartResolveTime(FileTime::oldestTime()) { } diff --git a/src/lib/corelib/language/language.h b/src/lib/corelib/language/language.h index 994ad6c55..82d9f94a2 100644 --- a/src/lib/corelib/language/language.h +++ b/src/lib/corelib/language/language.h @@ -703,7 +703,8 @@ public: bool locked; // This is the API-level lock for the project instance. Set buildSystemFiles; - FileTime lastResolveTime; + FileTime lastStartResolveTime; + FileTime lastEndResolveTime; QList warningsEncountered; void setBuildConfiguration(const QVariantMap &config); @@ -724,8 +725,8 @@ private: pool.serializationOp(m_id, canonicalFilePathResults, fileExistsResults, directoryEntriesResults, fileLastModifiedResults, environment, probes, profileConfigs, overriddenValues, buildSystemFiles, - lastResolveTime, warningsEncountered, buildData, - moduleProviderInfo); + lastStartResolveTime, lastEndResolveTime, warningsEncountered, + buildData, moduleProviderInfo); } void load(PersistentPool &pool) override; void store(PersistentPool &pool) override; diff --git a/src/lib/corelib/language/loader.cpp b/src/lib/corelib/language/loader.cpp index 4d2eef983..e27ccca74 100644 --- a/src/lib/corelib/language/loader.cpp +++ b/src/lib/corelib/language/loader.cpp @@ -170,7 +170,8 @@ TopLevelProjectPtr Loader::loadProject(const SetupProjectParameters &_parameters ProjectResolver resolver(&evaluator, loadResult, parameters, m_logger); resolver.setProgressObserver(m_progressObserver); const TopLevelProjectPtr project = resolver.resolve(); - project->lastResolveTime = resolveTime; + project->lastStartResolveTime = resolveTime; + project->lastEndResolveTime = FileTime::currentTime(); // E.g. if the top-level project is disabled. if (m_progressObserver) diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp index 58dab062a..fe71df43f 100644 --- a/src/lib/corelib/language/moduleloader.cpp +++ b/src/lib/corelib/language/moduleloader.cpp @@ -361,7 +361,7 @@ ModuleLoaderResult ModuleLoader::load(const SetupProjectParameters ¶meters) handleTopLevelProject(&result, root, buildDirectory, Set() << QDir::cleanPath(parameters.projectFilePath())); result.root = root; - result.qbsFiles = m_reader->filesRead(); + result.qbsFiles = m_reader->filesRead() - m_tempQbsFiles; for (auto it = m_localProfiles.cbegin(); it != m_localProfiles.cend(); ++it) result.profileConfigs.remove(it.key()); printProfilingInfo(); @@ -2156,7 +2156,10 @@ void ModuleLoader::setSearchPathsForProduct(ModuleLoader::ProductContext *produc if (!m_moduleProviderInfo.empty()) { const QVariantMap configForProduct = moduleProviderConfig(*product); for (const ModuleProviderInfo &c : m_moduleProviderInfo) { - if (configForProduct.value(c.name.toString()) == c.config) { + if (configForProduct.value(c.name.toString()).toMap() == c.config) { + qCDebug(lcModuleLoader) << "re-using search paths" << c.searchPaths + << "from module provider" << c.name + << "for product" << product->name; product->knownModuleProviders.insert(c.name); product->searchPaths << c.searchPaths; } @@ -2622,10 +2625,6 @@ void ModuleLoader::resolveDependsItem(DependsContext *dependsContext, Item *pare } ErrorInfo e(Tr::tr("Dependency '%1' not found for product '%2'.") .arg(moduleName.toString(), productName), dependsItem->location()); - if (moduleName.size() == 2 && moduleName.front() == QStringLiteral("Qt")) { - e.append(Tr::tr("Please create a Qt profile using the qbs-setup-qt tool " - "if you haven't already done so.")); - } throw e; } if (result.isProduct && !m_dependsChain.empty() && !m_dependsChain.back().isProduct) { @@ -3761,6 +3760,7 @@ ModuleLoader::ModuleProviderResult ModuleLoader::findModuleProvider(const Qualif "for dependency '%1': %2").arg(name.toString(), dummyItemFile.errorString())); } + m_tempQbsFiles << dummyItemFile.fileName(); qCDebug(lcModuleLoader) << "Instantiating module provider at" << providerFile; const QString projectBuildDir = product.project->item->variantProperty( StringConstants::buildDirectoryProperty())->value().toString(); @@ -3789,6 +3789,7 @@ ModuleLoader::ModuleProviderResult ModuleLoader::findModuleProvider(const Qualif .arg(providerFile, providerItem->typeName(), BuiltinDeclarations::instance().nameForType(ItemType::ModuleProvider))); } + providerItem->setParent(product.item); const QVariantMap configMap = moduleConfig.toMap(); for (auto it = configMap.begin(); it != configMap.end(); ++it) { const PropertyDeclaration decl = providerItem->propertyDeclaration(it.key()); @@ -3798,7 +3799,7 @@ ModuleLoader::ModuleProviderResult ModuleLoader::findModuleProvider(const Qualif } providerItem->setProperty(it.key(), VariantValue::create(it.value())); } - EvalContextSwitcher contextSwitcher(m_evaluator->engine(), EvalContext::ProbeExecution); + EvalContextSwitcher contextSwitcher(m_evaluator->engine(), EvalContext::ModuleProvider); const QStringList searchPaths = m_evaluator->stringListValue(providerItem, QStringLiteral("searchPaths")); if (searchPaths.empty()) { diff --git a/src/lib/corelib/language/moduleloader.h b/src/lib/corelib/language/moduleloader.h index 9e45908c3..a0a555748 100644 --- a/src/lib/corelib/language/moduleloader.h +++ b/src/lib/corelib/language/moduleloader.h @@ -450,6 +450,7 @@ private: Set m_exportsWithDeferredDependsItems; ModuleProviderInfoList m_moduleProviderInfo; + Set m_tempQbsFiles; SetupProjectParameters m_parameters; std::unique_ptr m_settings; diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp index fb4706b3b..7b2c69aa1 100644 --- a/src/lib/corelib/language/scriptengine.cpp +++ b/src/lib/corelib/language/scriptengine.cpp @@ -234,6 +234,7 @@ void ScriptEngine::checkContext(const QString &operation, + Tr::tr("Should this call be in a JavaScriptCommand instead?"); } break; + case EvalContext::ModuleProvider: case EvalContext::ProbeExecution: case EvalContext::JsCommand: QBS_ASSERT(false, continue); @@ -574,25 +575,30 @@ void ScriptEngine::releaseResourcesOfScriptObjects() void ScriptEngine::addCanonicalFilePathResult(const QString &filePath, const QString &resultFilePath) { - m_canonicalFilePathResult.insert(filePath, resultFilePath); + if (gatherFileResults()) + m_canonicalFilePathResult.insert(filePath, resultFilePath); } void ScriptEngine::addFileExistsResult(const QString &filePath, bool exists) { - m_fileExistsResult.insert(filePath, exists); + if (gatherFileResults()) + m_fileExistsResult.insert(filePath, exists); } void ScriptEngine::addDirectoryEntriesResult(const QString &path, QDir::Filters filters, const QStringList &entries) { - m_directoryEntriesResult.insert( - std::pair(path, static_cast(filters)), - entries); + if (gatherFileResults()) { + m_directoryEntriesResult.insert( + std::pair(path, static_cast(filters)), + entries); + } } void ScriptEngine::addFileLastModifiedResult(const QString &filePath, const FileTime &fileTime) { - m_fileLastModifiedResult.insert(filePath, fileTime); + if (gatherFileResults()) + m_fileLastModifiedResult.insert(filePath, fileTime); } Set ScriptEngine::imports() const @@ -655,6 +661,12 @@ void ScriptEngine::abort() abortEvaluation(m_cancelationError); } +bool ScriptEngine::gatherFileResults() const +{ + return evalContext() == EvalContext::PropertyEvaluation + || evalContext() == EvalContext::ProbeExecution; +} + class JSTypeExtender { public: diff --git a/src/lib/corelib/language/scriptengine.h b/src/lib/corelib/language/scriptengine.h index 226cf16a9..89863189a 100644 --- a/src/lib/corelib/language/scriptengine.h +++ b/src/lib/corelib/language/scriptengine.h @@ -72,7 +72,9 @@ class PrepareScriptObserver; class ScriptImporter; class ScriptPropertyObserver; -enum class EvalContext { PropertyEvaluation, ProbeExecution, RuleExecution, JsCommand }; +enum class EvalContext { + PropertyEvaluation, ProbeExecution, ModuleProvider, RuleExecution, JsCommand +}; class DubiousContext { public: @@ -279,6 +281,8 @@ private: void abort(); + bool gatherFileResults() const; + void installQbsBuiltins(); void extendJavaScriptBuiltins(); void installFunction(const QString &name, int length, QScriptValue *functionValue, diff --git a/src/lib/corelib/tools/persistence.cpp b/src/lib/corelib/tools/persistence.cpp index b50084702..5591c9761 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-125"; +static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-126"; NoBuildGraphError::NoBuildGraphError(const QString &filePath) : ErrorInfo(Tr::tr("Build graph not found for configuration '%1'. Expected location was '%2'.") diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index 82f29f320..8154e7262 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -6396,7 +6396,8 @@ void TestBlackbox::fallbackModuleProvider() static const auto b2s = [](bool b) { return QString(b ? "true" : "false"); }; QbsRunParameters resolveParams("resolve", QStringList{"modules.pkgconfig.libDirs:" + pkgConfigLibDirs.join(','), - "products.p.fallbacksEnabled:" + b2s(fallbacksEnabledInProduct)}); + "products.p.fallbacksEnabled:" + b2s(fallbacksEnabledInProduct), + "--force-probe-execution"}); if (!fallbacksEnabledGlobally) resolveParams.arguments << "--no-fallback-module-provider"; QCOMPARE(runQbs(resolveParams), 0); -- cgit v1.2.3