From d2a6e160aa74479fc623918b084423739a9b80c7 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 19 Aug 2021 12:22:47 -0400 Subject: AUTOUIC: Revert "Fix generating of dependency rules for UI header files" Revert commit e5ec0e52f4 (AUTOUIC: Fix generating of dependency rules for UI header files, 2021-07-22, v3.21.1~8^2) because it caused regressions. For example, changing one C++ source can now cause many others to rebuild. Revert the change pending further investigation. Fixes: #22531 Issue: #16776 --- Source/CMakeLists.txt | 2 - Source/cmQtAutoGen.cxx | 36 --------------- Source/cmQtAutoGen.h | 3 -- Source/cmQtAutoGenInitializer.cxx | 53 +++++++++++----------- Source/cmQtAutoGenInitializer.h | 2 - Source/cmQtAutoGenerator.cxx | 37 +++++++++++++++ Source/cmQtAutoGenerator.h | 2 + Source/cmQtAutoMocUic.cxx | 20 ++++++-- Source/cmQtAutoUicHelpers.cxx | 25 ---------- Source/cmQtAutoUicHelpers.h | 20 -------- .../QtAutogen/RerunUicOnFileChange/CMakeLists.txt | 3 -- .../UicOnFileChange/CMakeLists.txt.in | 1 - .../RerunUicOnFileChange/UicOnFileChange/main.cpp | 4 +- .../UicOnFileChange/subdir/mainwindowsubdir.ui.in | 7 --- .../UicOnFileChange/subdir/subdircheck.cpp | 9 ---- 15 files changed, 82 insertions(+), 142 deletions(-) delete mode 100644 Source/cmQtAutoUicHelpers.cxx delete mode 100644 Source/cmQtAutoUicHelpers.h delete mode 100644 Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in delete mode 100644 Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 0142c07..9a18184 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -432,8 +432,6 @@ set(SRCS cmQtAutoMocUic.h cmQtAutoRcc.cxx cmQtAutoRcc.h - cmQtAutoUicHelpers.cxx - cmQtAutoUicHelpers.h cmRST.cxx cmRST.h cmRuntimeDependencyArchive.cxx diff --git a/Source/cmQtAutoGen.cxx b/Source/cmQtAutoGen.cxx index 898d862..57fcd2d 100644 --- a/Source/cmQtAutoGen.cxx +++ b/Source/cmQtAutoGen.cxx @@ -384,39 +384,3 @@ bool cmQtAutoGen::RccLister::list(std::string const& qrcFile, } return true; } - -bool cmQtAutoGen::FileRead(std::string& content, std::string const& filename, - std::string* error) -{ - content.clear(); - if (!cmSystemTools::FileExists(filename, true)) { - if (error != nullptr) { - *error = "Not a file."; - } - return false; - } - - unsigned long const length = cmSystemTools::FileLength(filename); - cmsys::ifstream ifs(filename.c_str(), (std::ios::in | std::ios::binary)); - - // Use lambda to save destructor calls of ifs - return [&ifs, length, &content, error]() -> bool { - if (!ifs) { - if (error != nullptr) { - *error = "Opening the file for reading failed."; - } - return false; - } - content.reserve(length); - using IsIt = std::istreambuf_iterator; - content.assign(IsIt{ ifs }, IsIt{}); - if (!ifs) { - content.clear(); - if (error != nullptr) { - *error = "Reading from the file failed."; - } - return false; - } - return true; - }(); -} diff --git a/Source/cmQtAutoGen.h b/Source/cmQtAutoGen.h index b9ae360..466a954 100644 --- a/Source/cmQtAutoGen.h +++ b/Source/cmQtAutoGen.h @@ -100,9 +100,6 @@ public: std::vector const& newOpts, bool isQt5); - static bool FileRead(std::string& content, std::string const& filename, - std::string* error = nullptr); - /** @class RccLister * @brief Lists files in qrc resource files */ diff --git a/Source/cmQtAutoGenInitializer.cxx b/Source/cmQtAutoGenInitializer.cxx index 6cc8328..4dd78e5 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -902,13 +902,6 @@ bool cmQtAutoGenInitializer::InitScanFiles() // The reason is that their file names might be discovered from source files // at generation time. if (this->MocOrUicEnabled()) { - std::set uicIncludes; - auto collectUicIncludes = [&](std::unique_ptr const& sf) { - std::string content; - FileRead(content, sf->GetFullPath()); - this->AutoUicHelpers.CollectUicIncludes(uicIncludes, content); - }; - for (const auto& sf : this->Makefile->GetSourceFiles()) { // sf->GetExtension() is only valid after sf->ResolveFullPath() ... // Since we're iterating over source files that might be not in the @@ -921,10 +914,6 @@ bool cmQtAutoGenInitializer::InitScanFiles() std::string const& extLower = cmSystemTools::LowerCase(sf->GetExtension()); - bool const skipAutogen = sf->GetPropertyAsBool(kw.SKIP_AUTOGEN); - bool const skipUic = - (skipAutogen || sf->GetPropertyAsBool(kw.SKIP_AUTOUIC) || - !this->Uic.Enabled); if (cm->IsAHeaderExtension(extLower)) { if (!cm::contains(this->AutogenTarget.Headers, sf.get())) { auto muf = makeMUFile(sf.get(), fullPath, {}, false); @@ -932,9 +921,6 @@ bool cmQtAutoGenInitializer::InitScanFiles() addMUHeader(std::move(muf), extLower); } } - if (!skipUic && !sf->GetIsGenerated()) { - collectUicIncludes(sf); - } } else if (cm->IsACLikeSourceExtension(extLower)) { if (!cm::contains(this->AutogenTarget.Sources, sf.get())) { auto muf = makeMUFile(sf.get(), fullPath, {}, false); @@ -942,11 +928,11 @@ bool cmQtAutoGenInitializer::InitScanFiles() addMUSource(std::move(muf)); } } - if (!skipUic && !sf->GetIsGenerated()) { - collectUicIncludes(sf); - } } else if (this->Uic.Enabled && (extLower == kw.ui)) { // .ui file + bool const skipAutogen = sf->GetPropertyAsBool(kw.SKIP_AUTOGEN); + bool const skipUic = + (skipAutogen || sf->GetPropertyAsBool(kw.SKIP_AUTOUIC)); if (!skipUic) { // Check if the .ui file has uic options std::string const uicOpts = sf->GetSafeProperty(kw.AUTOUIC_OPTIONS); @@ -956,22 +942,35 @@ bool cmQtAutoGenInitializer::InitScanFiles() this->Uic.UiFilesWithOptions.emplace_back(fullPath, cmExpandedList(uicOpts)); } + + auto uiHeaderRelativePath = cmSystemTools::RelativePath( + this->LocalGen->GetCurrentSourceDirectory(), + cmSystemTools::GetFilenamePath(fullPath)); + + // Avoid creating a path containing adjacent slashes + if (!uiHeaderRelativePath.empty() && + uiHeaderRelativePath.back() != '/') { + uiHeaderRelativePath += '/'; + } + + auto uiHeaderFilePath = cmStrCat( + '/', uiHeaderRelativePath, "ui_"_s, + cmSystemTools::GetFilenameWithoutLastExtension(fullPath), ".h"_s); + + ConfigString uiHeader; + std::string uiHeaderGenex; + this->ConfigFileNamesAndGenex( + uiHeader, uiHeaderGenex, cmStrCat(this->Dir.Build, "/include"_s), + uiHeaderFilePath); + + this->Uic.UiHeaders.emplace_back( + std::make_pair(uiHeader, uiHeaderGenex)); } else { // Register skipped .ui file this->Uic.SkipUi.insert(fullPath); } } } - - for (const auto& include : uicIncludes) { - ConfigString uiHeader; - std::string uiHeaderGenex; - this->ConfigFileNamesAndGenex(uiHeader, uiHeaderGenex, - cmStrCat(this->Dir.Build, "/include"_s), - cmStrCat("/"_s, include)); - this->Uic.UiHeaders.emplace_back( - std::make_pair(uiHeader, uiHeaderGenex)); - } } // Process GENERATED sources and headers diff --git a/Source/cmQtAutoGenInitializer.h b/Source/cmQtAutoGenInitializer.h index 3ec87d2..e76817b 100644 --- a/Source/cmQtAutoGenInitializer.h +++ b/Source/cmQtAutoGenInitializer.h @@ -17,7 +17,6 @@ #include "cmFilePathChecksum.h" #include "cmQtAutoGen.h" -#include "cmQtAutoUicHelpers.h" class cmGeneratorTarget; class cmGlobalGenerator; @@ -171,7 +170,6 @@ private: std::string ConfigDefault; std::vector ConfigsList; std::string TargetsFolder; - cmQtAutoUicHelpers AutoUicHelpers; /** Common directories. */ struct diff --git a/Source/cmQtAutoGenerator.cxx b/Source/cmQtAutoGenerator.cxx index 0c6b5e6..568926e 100644 --- a/Source/cmQtAutoGenerator.cxx +++ b/Source/cmQtAutoGenerator.cxx @@ -121,6 +121,43 @@ bool cmQtAutoGenerator::MakeParentDirectory(std::string const& filename) return success; } +bool cmQtAutoGenerator::FileRead(std::string& content, + std::string const& filename, + std::string* error) +{ + content.clear(); + if (!cmSystemTools::FileExists(filename, true)) { + if (error != nullptr) { + *error = "Not a file."; + } + return false; + } + + unsigned long const length = cmSystemTools::FileLength(filename); + cmsys::ifstream ifs(filename.c_str(), (std::ios::in | std::ios::binary)); + + // Use lambda to save destructor calls of ifs + return [&ifs, length, &content, error]() -> bool { + if (!ifs) { + if (error != nullptr) { + *error = "Opening the file for reading failed."; + } + return false; + } + content.reserve(length); + using IsIt = std::istreambuf_iterator; + content.assign(IsIt{ ifs }, IsIt{}); + if (!ifs) { + content.clear(); + if (error != nullptr) { + *error = "Reading from the file failed."; + } + return false; + } + return true; + }(); +} + bool cmQtAutoGenerator::FileWrite(std::string const& filename, std::string const& content, std::string* error) diff --git a/Source/cmQtAutoGenerator.h b/Source/cmQtAutoGenerator.h index 66399d7..5c3a8ad 100644 --- a/Source/cmQtAutoGenerator.h +++ b/Source/cmQtAutoGenerator.h @@ -70,6 +70,8 @@ public: // -- File system methods static bool MakeParentDirectory(std::string const& filename); + static bool FileRead(std::string& content, std::string const& filename, + std::string* error = nullptr); static bool FileWrite(std::string const& filename, std::string const& content, std::string* error = nullptr); diff --git a/Source/cmQtAutoMocUic.cxx b/Source/cmQtAutoMocUic.cxx index 86d54f9..2753fd5 100644 --- a/Source/cmQtAutoMocUic.cxx +++ b/Source/cmQtAutoMocUic.cxx @@ -30,7 +30,6 @@ #include "cmGeneratedFileStream.h" #include "cmQtAutoGen.h" #include "cmQtAutoGenerator.h" -#include "cmQtAutoUicHelpers.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmWorkerPool.h" @@ -282,7 +281,7 @@ public: std::vector Options; std::unordered_map UiFiles; std::vector SearchPaths; - cmQtAutoUicHelpers AutoUicHelpers; + cmsys::RegularExpression RegExpInclude; }; /** Uic shared variables. */ @@ -762,7 +761,11 @@ std::string cmQtAutoMocUicT::MocSettingsT::MacrosString() const return res; } -cmQtAutoMocUicT::UicSettingsT::UicSettingsT() = default; +cmQtAutoMocUicT::UicSettingsT::UicSettingsT() +{ + this->RegExpInclude.compile("(^|\n)[ \t]*#[ \t]*include[ \t]+" + "[\"<](([^ \">]+/)?ui_[^ \">/]+\\.h)[\">]"); +} cmQtAutoMocUicT::UicSettingsT::~UicSettingsT() = default; @@ -1053,7 +1056,16 @@ void cmQtAutoMocUicT::JobParseT::UicIncludes() } std::set includes; - this->UicConst().AutoUicHelpers.CollectUicIncludes(includes, this->Content); + { + const char* contentChars = this->Content.c_str(); + cmsys::RegularExpression const& regExp = this->UicConst().RegExpInclude; + cmsys::RegularExpressionMatch match; + while (regExp.find(contentChars, match)) { + includes.emplace(match.match(2)); + // Forward content pointer + contentChars += match.end(); + } + } this->CreateKeys(this->FileHandle->ParseData->Uic.Include, includes, UiUnderscoreLength); } diff --git a/Source/cmQtAutoUicHelpers.cxx b/Source/cmQtAutoUicHelpers.cxx deleted file mode 100644 index 751ae08..0000000 --- a/Source/cmQtAutoUicHelpers.cxx +++ /dev/null @@ -1,25 +0,0 @@ -/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying - file Copyright.txt or https://cmake.org/licensing for details. */ -#include "cmQtAutoUicHelpers.h" - -cmQtAutoUicHelpers::cmQtAutoUicHelpers() -{ - RegExpInclude.compile("(^|\n)[ \t]*#[ \t]*include[ \t]+" - "[\"<](([^ \">]+/)?ui_[^ \">/]+\\.h)[\">]"); -} - -void cmQtAutoUicHelpers::CollectUicIncludes(std::set& includes, - const std::string& content) const -{ - if (content.find("ui_") == std::string::npos) { - return; - } - - const char* contentChars = content.c_str(); - cmsys::RegularExpressionMatch match; - while (this->RegExpInclude.find(contentChars, match)) { - includes.emplace(match.match(2)); - // Forward content pointer - contentChars += match.end(); - } -} diff --git a/Source/cmQtAutoUicHelpers.h b/Source/cmQtAutoUicHelpers.h deleted file mode 100644 index 6b09a31..0000000 --- a/Source/cmQtAutoUicHelpers.h +++ /dev/null @@ -1,20 +0,0 @@ -/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying - file Copyright.txt or https://cmake.org/licensing for details. */ -#pragma once - -#include -#include - -#include "cmsys/RegularExpression.hxx" - -class cmQtAutoUicHelpers -{ -public: - cmQtAutoUicHelpers(); - virtual ~cmQtAutoUicHelpers() = default; - void CollectUicIncludes(std::set& includes, - const std::string& content) const; - -private: - cmsys::RegularExpression RegExpInclude; -}; diff --git a/Tests/QtAutogen/RerunUicOnFileChange/CMakeLists.txt b/Tests/QtAutogen/RerunUicOnFileChange/CMakeLists.txt index a3ba9fb..9b114e9 100644 --- a/Tests/QtAutogen/RerunUicOnFileChange/CMakeLists.txt +++ b/Tests/QtAutogen/RerunUicOnFileChange/CMakeLists.txt @@ -28,12 +28,10 @@ endmacro() configure_file("${testProjectTemplateDir}/mocwidget.h" "${testProjectSrc}/mocwidget.h" COPYONLY) configure_file("${testProjectTemplateDir}/mainwindow.h" "${testProjectSrc}/mainwindow.h" COPYONLY) configure_file("${testProjectTemplateDir}/main.cpp" "${testProjectSrc}/main.cpp" COPYONLY) -configure_file("${testProjectTemplateDir}/subdir/subdircheck.cpp" "${testProjectSrc}/subdir/subdircheck.cpp" COPYONLY) configure_file("${testProjectTemplateDir}/CMakeLists.txt.in" "${testProjectSrc}/CMakeLists.txt" @ONLY) set(Num 1) configure_file("${testProjectTemplateDir}/mainwindow.ui.in" "${testProjectSrc}/mainwindow.ui" @ONLY) -configure_file("${testProjectTemplateDir}/subdir/mainwindowsubdir.ui.in" "${testProjectSrc}/subdir/mainwindowsubdir.ui" @ONLY) if(CMAKE_GENERATOR_INSTANCE) set(_D_CMAKE_GENERATOR_INSTANCE "-DCMAKE_GENERATOR_INSTANCE=${CMAKE_GENERATOR_INSTANCE}") @@ -97,7 +95,6 @@ sleep() set(Num 2) configure_file("${testProjectTemplateDir}/mainwindow.ui.in" "${testProjectSrc}/mainwindow.ui" @ONLY) -configure_file("${testProjectTemplateDir}/subdir/mainwindowsubdir.ui.in" "${testProjectSrc}/subdir/mainwindowsubdir.ui" @ONLY) rebuild(2) execute_process(COMMAND "${testProjectBinDir}/${extra_bin_path}UicOnFileChange" RESULT_VARIABLE result) diff --git a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/CMakeLists.txt.in b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/CMakeLists.txt.in index c77075c..c787db1 100644 --- a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/CMakeLists.txt.in +++ b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/CMakeLists.txt.in @@ -8,7 +8,6 @@ set(CMAKE_AUTOUIC ON) set(CMAKE_AUTOMOC ON) add_executable(UicOnFileChange main.cpp mainwindow.ui mainwindow.h - subdir/subdircheck.cpp subdir/mainwindowsubdir.ui ) target_include_directories(UicOnFileChange PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}" diff --git a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp index 99de13d..4aab2ad 100644 --- a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp +++ b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp @@ -1,12 +1,10 @@ #include "mainwindow.h" #include "ui_mainwindow.h" -extern bool subdircheck(); - int main(int argc, char* argv[]) { MocWidget mw; Ui::Widget mwUi; mwUi.setupUi(&mw); - return mw.objectName() == "Widget2" && subdircheck() ? 0 : 1; + return mw.objectName() == "Widget2" ? 0 : 1; } diff --git a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in deleted file mode 100644 index a6a31f6..0000000 --- a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in +++ /dev/null @@ -1,7 +0,0 @@ - - - WidgetSubdir - - - - diff --git a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp deleted file mode 100644 index 3b36a10..0000000 --- a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include "ui_mainwindowsubdir.h" - -bool subdircheck() -{ - MocWidget mw; - Ui::WidgetSubdir mwUi; - mwUi.setupUi(&mw); - return mw.objectName() == "WidgetSubdir2"; -} -- cgit v0.12