From e5ec0e52f4f15f78bb973cdb03a61ef9c707c2fa Mon Sep 17 00:00:00 2001 From: Alexey Edelev Date: Thu, 22 Jul 2021 12:13:48 +0200 Subject: AUTOUIC: Fix generating of dependency rules for UI header files We could not rely on .ui files when generating the ninja rules for the generated UI header files. .ui files might be added to the target sources but never processed by AUTOUIC afterward, since UI header files are never included in a source code. Instead of adding dependency rules based on the .ui files, this approach scans non-generated source files for includes of the UI header files, as AUTOUIC does. This gives the consistent set of UI header files at configure time, that could be used to generate byproducts rules for the AUTOUIC. Also, the path to the generated UI header file depends not on the .ui file location but on the include line is used in source files. Fixes: #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 | 4 +- .../RerunUicOnFileChange/UicOnFileChange/main.cpp | 4 +- .../UicOnFileChange/subdir/mainwindowsubdir.ui.in | 7 +++ .../UicOnFileChange/subdir/subdircheck.cpp | 9 ++++ 15 files changed, 144 insertions(+), 83 deletions(-) create mode 100644 Source/cmQtAutoUicHelpers.cxx create mode 100644 Source/cmQtAutoUicHelpers.h create mode 100644 Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in create mode 100644 Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 9a18184fd3..0142c07ecb 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -432,6 +432,8 @@ 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 57fcd2da76..898d862221 100644 --- a/Source/cmQtAutoGen.cxx +++ b/Source/cmQtAutoGen.cxx @@ -384,3 +384,39 @@ 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 466a954a98..b9ae360291 100644 --- a/Source/cmQtAutoGen.h +++ b/Source/cmQtAutoGen.h @@ -100,6 +100,9 @@ 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 2894201c9b..9d8086397c 100644 --- a/Source/cmQtAutoGenInitializer.cxx +++ b/Source/cmQtAutoGenInitializer.cxx @@ -902,6 +902,13 @@ 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 @@ -914,6 +921,10 @@ 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); @@ -921,6 +932,9 @@ 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); @@ -928,11 +942,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); @@ -942,35 +956,22 @@ 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 e76817b9c4..3ec87d212d 100644 --- a/Source/cmQtAutoGenInitializer.h +++ b/Source/cmQtAutoGenInitializer.h @@ -17,6 +17,7 @@ #include "cmFilePathChecksum.h" #include "cmQtAutoGen.h" +#include "cmQtAutoUicHelpers.h" class cmGeneratorTarget; class cmGlobalGenerator; @@ -170,6 +171,7 @@ 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 568926e3c5..0c6b5e6dc5 100644 --- a/Source/cmQtAutoGenerator.cxx +++ b/Source/cmQtAutoGenerator.cxx @@ -121,43 +121,6 @@ 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 5c3a8adccb..66399d7e8a 100644 --- a/Source/cmQtAutoGenerator.h +++ b/Source/cmQtAutoGenerator.h @@ -70,8 +70,6 @@ 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 2753fd57c0..86d54f9e1a 100644 --- a/Source/cmQtAutoMocUic.cxx +++ b/Source/cmQtAutoMocUic.cxx @@ -30,6 +30,7 @@ #include "cmGeneratedFileStream.h" #include "cmQtAutoGen.h" #include "cmQtAutoGenerator.h" +#include "cmQtAutoUicHelpers.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmWorkerPool.h" @@ -281,7 +282,7 @@ public: std::vector Options; std::unordered_map UiFiles; std::vector SearchPaths; - cmsys::RegularExpression RegExpInclude; + cmQtAutoUicHelpers AutoUicHelpers; }; /** Uic shared variables. */ @@ -761,11 +762,7 @@ std::string cmQtAutoMocUicT::MocSettingsT::MacrosString() const return res; } -cmQtAutoMocUicT::UicSettingsT::UicSettingsT() -{ - this->RegExpInclude.compile("(^|\n)[ \t]*#[ \t]*include[ \t]+" - "[\"<](([^ \">]+/)?ui_[^ \">/]+\\.h)[\">]"); -} +cmQtAutoMocUicT::UicSettingsT::UicSettingsT() = default; cmQtAutoMocUicT::UicSettingsT::~UicSettingsT() = default; @@ -1056,16 +1053,7 @@ void cmQtAutoMocUicT::JobParseT::UicIncludes() } std::set includes; - { - 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->UicConst().AutoUicHelpers.CollectUicIncludes(includes, this->Content); this->CreateKeys(this->FileHandle->ParseData->Uic.Include, includes, UiUnderscoreLength); } diff --git a/Source/cmQtAutoUicHelpers.cxx b/Source/cmQtAutoUicHelpers.cxx new file mode 100644 index 0000000000..751ae08575 --- /dev/null +++ b/Source/cmQtAutoUicHelpers.cxx @@ -0,0 +1,25 @@ +/* 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 new file mode 100644 index 0000000000..6b09a31daa --- /dev/null +++ b/Source/cmQtAutoUicHelpers.h @@ -0,0 +1,20 @@ +/* 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 1f636af451..a9ccece5d5 100644 --- a/Tests/QtAutogen/RerunUicOnFileChange/CMakeLists.txt +++ b/Tests/QtAutogen/RerunUicOnFileChange/CMakeLists.txt @@ -27,10 +27,12 @@ endmacro() configure_file("${testProjectTemplateDir}/mocwidget.h" "${testProjectSrc}/mocwidget.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}") @@ -94,6 +96,7 @@ 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 fa9dd6bee3..2a1998d10f 100644 --- a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/CMakeLists.txt.in +++ b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/CMakeLists.txt.in @@ -6,6 +6,8 @@ include("@CMAKE_CURRENT_LIST_DIR@/../AutogenGuiTest.cmake") # Enable CMAKE_AUTOUIC for all targets set(CMAKE_AUTOUIC ON) -add_executable(UicOnFileChange main.cpp mainwindow.ui) +add_executable(UicOnFileChange main.cpp mainwindow.ui + subdir/subdircheck.cpp subdir/mainwindowsubdir.ui +) target_include_directories(UicOnFileChange PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries(UicOnFileChange ${QT_QTCORE_TARGET} ${QT_LIBRARIES}) diff --git a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp index fd810fa247..3981268846 100644 --- a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp +++ b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/main.cpp @@ -1,9 +1,11 @@ #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" ? 0 : 1; + return mw.objectName() == "Widget2" && subdircheck() ? 0 : 1; } diff --git a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in new file mode 100644 index 0000000000..a6a31f6d42 --- /dev/null +++ b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/mainwindowsubdir.ui.in @@ -0,0 +1,7 @@ + + + WidgetSubdir + + + + diff --git a/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp new file mode 100644 index 0000000000..3b36a10df8 --- /dev/null +++ b/Tests/QtAutogen/RerunUicOnFileChange/UicOnFileChange/subdir/subdircheck.cpp @@ -0,0 +1,9 @@ +#include "ui_mainwindowsubdir.h" + +bool subdircheck() +{ + MocWidget mw; + Ui::WidgetSubdir mwUi; + mwUi.setupUi(&mw); + return mw.objectName() == "WidgetSubdir2"; +} -- cgit v1.2.1