From 4ee5d68b33be324696a15a788a8b4cd34ff83cf0 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Mon, 21 Jan 2019 16:58:13 +0100 Subject: [PATCH] PchManager: Update only after generated files have been updated Task-number: QTCREATORBUG-21843 Change-Id: I881e86dac4074438880d657a07f2e473489ab42d Reviewed-by: Ivan Donchevskii --- src/libs/clangsupport/generatedfiles.cpp | 9 +++ src/libs/clangsupport/generatedfiles.h | 1 + .../clangsupport/generatedfilesinterface.h | 1 + src/libs/clangsupport/projectpartcontainer.h | 1 + .../source/pchmanagerserver.cpp | 34 ++++++++++- .../source/projectparts.cpp | 53 ++++++++++++++++ .../source/projectparts.h | 3 +- .../source/projectpartsinterface.h | 3 +- tests/unit/unittest/generatedfiles-test.cpp | 17 ++++++ tests/unit/unittest/mockgeneratedfiles.h | 1 + tests/unit/unittest/mockprojectparts.h | 12 ++-- tests/unit/unittest/pchmanagerserver-test.cpp | 60 ++++++++++++++++++- tests/unit/unittest/projectparts-test.cpp | 28 ++++++++- 13 files changed, 210 insertions(+), 13 deletions(-) diff --git a/src/libs/clangsupport/generatedfiles.cpp b/src/libs/clangsupport/generatedfiles.cpp index dfbcb145dd6..f4863baba5d 100644 --- a/src/libs/clangsupport/generatedfiles.cpp +++ b/src/libs/clangsupport/generatedfiles.cpp @@ -100,6 +100,15 @@ void GeneratedFiles::remove(const FilePaths &filePaths) m_fileContainers = std::move(differenceFileContainers); } +bool GeneratedFiles::isValid() const +{ + return std::all_of(m_fileContainers.begin(), + m_fileContainers.end(), + [](const V2::FileContainer &container) { + return container.unsavedFileContent.hasContent(); + }); +} + const V2::FileContainers &GeneratedFiles::fileContainers() const { return m_fileContainers; diff --git a/src/libs/clangsupport/generatedfiles.h b/src/libs/clangsupport/generatedfiles.h index 6d2c9cb86bd..933b0d111bf 100644 --- a/src/libs/clangsupport/generatedfiles.h +++ b/src/libs/clangsupport/generatedfiles.h @@ -35,6 +35,7 @@ public: void update(V2::FileContainers &&fileContainers); void update(const V2::FileContainers &fileContainers); void remove(const FilePaths &filePaths); + bool isValid() const; const V2::FileContainers &fileContainers() const; diff --git a/src/libs/clangsupport/generatedfilesinterface.h b/src/libs/clangsupport/generatedfilesinterface.h index 4c7c6762123..85a50ffa387 100644 --- a/src/libs/clangsupport/generatedfilesinterface.h +++ b/src/libs/clangsupport/generatedfilesinterface.h @@ -35,6 +35,7 @@ public: virtual void update(V2::FileContainers &&fileContainers) = 0; virtual void update(const V2::FileContainers &fileContainers) = 0; virtual void remove(const FilePaths &filePaths) = 0; + virtual bool isValid() const = 0; virtual const V2::FileContainers &fileContainers() const = 0; diff --git a/src/libs/clangsupport/projectpartcontainer.h b/src/libs/clangsupport/projectpartcontainer.h index d805ef5aad3..41ae071961d 100644 --- a/src/libs/clangsupport/projectpartcontainer.h +++ b/src/libs/clangsupport/projectpartcontainer.h @@ -156,6 +156,7 @@ public: Utils::Language language = Utils::Language::Cxx; Utils::LanguageVersion languageVersion = Utils::LanguageVersion::CXX98; Utils::LanguageExtension languageExtension = Utils::LanguageExtension::None; + bool updateIsDeferred = false; }; using ProjectPartContainers = std::vector; diff --git a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp index 2f535606321..30b6e566fa5 100644 --- a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp +++ b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp @@ -61,8 +61,12 @@ void PchManagerServer::updateProjectParts(UpdateProjectPartsMessage &&message) { m_toolChainsArgumentsCache.update(message.projectsParts, message.toolChainArguments); - m_pchTaskGenerator.addProjectParts( - m_projectParts.update(message.takeProjectsParts()), std::move(message.toolChainArguments)); + ProjectPartContainers newProjectParts = m_projectParts.update(message.takeProjectsParts()); + + if (m_generatedFiles.isValid()) { + m_pchTaskGenerator.addProjectParts(std::move(newProjectParts), + std::move(message.toolChainArguments)); + } } void PchManagerServer::removeProjectParts(RemoveProjectPartsMessage &&message) @@ -76,9 +80,35 @@ void PchManagerServer::removeProjectParts(RemoveProjectPartsMessage &&message) m_toolChainsArgumentsCache.remove(message.projectsPartIds); } +namespace { +Utils::SmallStringVector projectPartIds(const ProjectPartContainers &projectParts) +{ + Utils::SmallStringVector ids; + ids.reserve(projectParts.size()); + + std::transform(projectParts.cbegin(), + projectParts.cend(), + std::back_inserter(ids), + [](const ProjectPartContainer &projectPart) { return projectPart.projectPartId; }); + + return ids; +} +} + void PchManagerServer::updateGeneratedFiles(UpdateGeneratedFilesMessage &&message) { m_generatedFiles.update(message.takeGeneratedFiles()); + + if (m_generatedFiles.isValid()) { + ProjectPartContainers deferredProjectParts = m_projectParts.deferredUpdates(); + ArgumentsEntries entries = m_toolChainsArgumentsCache.arguments( + projectPartIds(deferredProjectParts)); + + for (ArgumentsEntry &entry : entries) { + m_pchTaskGenerator.addProjectParts(std::move(deferredProjectParts), + std::move(entry.arguments)); + } + } } void PchManagerServer::removeGeneratedFiles(RemoveGeneratedFilesMessage &&message) diff --git a/src/tools/clangpchmanagerbackend/source/projectparts.cpp b/src/tools/clangpchmanagerbackend/source/projectparts.cpp index bf2235fd134..a67c0721e00 100644 --- a/src/tools/clangpchmanagerbackend/source/projectparts.cpp +++ b/src/tools/clangpchmanagerbackend/source/projectparts.cpp @@ -66,6 +66,59 @@ ProjectPartContainers ProjectParts::projects(const Utils::SmallStringVector &pro return projectPartsWithIds; } +void ProjectParts::updateDeferred(const ProjectPartContainers &deferredProjectsParts) +{ + using ProjectPartContainerReferences = std::vector>; + + class BackInserterIterator : public std::back_insert_iterator + { + public: + BackInserterIterator(ProjectPartContainerReferences &container) + : std::back_insert_iterator(container) + {} + + BackInserterIterator &operator=(ProjectPartContainer &projectPart) + { + container->push_back(std::ref(projectPart)); + + return *this; + } + + BackInserterIterator &operator*() { return *this; } + }; + + ProjectPartContainerReferences deferredProjectPartPointers; + deferredProjectPartPointers.reserve(deferredProjectsParts.size()); + + std::set_intersection(m_projectParts.begin(), + m_projectParts.end(), + deferredProjectsParts.begin(), + deferredProjectsParts.end(), + BackInserterIterator(deferredProjectPartPointers), + [](const ProjectPartContainer &first, const ProjectPartContainer &second) { + return first.projectPartId < second.projectPartId; + }); + + for (ProjectPartContainer &projectPart : deferredProjectPartPointers) + projectPart.updateIsDeferred = true; +} + +ProjectPartContainers ProjectParts::deferredUpdates() +{ + ProjectPartContainers deferredProjectParts; + deferredProjectParts.reserve(m_projectParts.size()); + + std::copy_if(m_projectParts.cbegin(), + m_projectParts.cend(), + std::back_inserter(deferredProjectParts), + [](const ProjectPartContainer &projectPart) { return projectPart.updateIsDeferred; }); + + for (ProjectPartContainer &projectPart : m_projectParts) + projectPart.updateIsDeferred = false; + + return deferredProjectParts; +} + ProjectPartContainers ProjectParts::newProjectParts(ProjectPartContainers &&projectsParts) const { ProjectPartContainers updatedProjectPartContainers; diff --git a/src/tools/clangpchmanagerbackend/source/projectparts.h b/src/tools/clangpchmanagerbackend/source/projectparts.h index dfe03fa0377..4df413ce6f0 100644 --- a/src/tools/clangpchmanagerbackend/source/projectparts.h +++ b/src/tools/clangpchmanagerbackend/source/projectparts.h @@ -41,8 +41,9 @@ public: ProjectPartContainers update(ProjectPartContainers &&projectsParts) override; void remove(const Utils::SmallStringVector &projectPartIds) override; ProjectPartContainers projects(const Utils::SmallStringVector &projectPartIds) const override; + void updateDeferred(const ProjectPartContainers &projectsParts) override; + ProjectPartContainers deferredUpdates() override; -unittest_public: ProjectPartContainers newProjectParts(ProjectPartContainers &&projectsParts) const; void mergeProjectParts(const ProjectPartContainers &projectsParts); const ProjectPartContainers &projectParts() const; diff --git a/src/tools/clangpchmanagerbackend/source/projectpartsinterface.h b/src/tools/clangpchmanagerbackend/source/projectpartsinterface.h index f84ad980901..d37673af100 100644 --- a/src/tools/clangpchmanagerbackend/source/projectpartsinterface.h +++ b/src/tools/clangpchmanagerbackend/source/projectpartsinterface.h @@ -39,7 +39,8 @@ public: virtual ProjectPartContainers update(ProjectPartContainers &&projectsParts) = 0; virtual void remove(const Utils::SmallStringVector &projectPartIds) = 0; virtual ProjectPartContainers projects(const Utils::SmallStringVector &projectPartIds) const = 0; - + virtual void updateDeferred(const ProjectPartContainers &projectsParts) = 0; + virtual ProjectPartContainers deferredUpdates() = 0; protected: ~ProjectPartsInterface() = default; }; diff --git a/tests/unit/unittest/generatedfiles-test.cpp b/tests/unit/unittest/generatedfiles-test.cpp index a042f1679cd..9db9c4b0aee 100644 --- a/tests/unit/unittest/generatedfiles-test.cpp +++ b/tests/unit/unittest/generatedfiles-test.cpp @@ -69,4 +69,21 @@ TEST_F(GeneratedFiles, RemoveGeneratedFiles) ASSERT_THAT(generatedFiles.fileContainers(), ElementsAre(file1, file3)); } +TEST_F(GeneratedFiles, IsValidForNoGeneratedFiles) +{ + ASSERT_TRUE(generatedFiles.isValid()); } + +TEST_F(GeneratedFiles, IsNotValidIfFilesWithNotContentExists) +{ + generatedFiles.update({{"/file2", ""}}); + + ASSERT_FALSE(generatedFiles.isValid()); +} + +TEST_F(GeneratedFiles, IsValidIfAllFilesHasContent) +{ + generatedFiles.update({file1, file2, file3, file4}); + + ASSERT_TRUE(generatedFiles.isValid()); +}} diff --git a/tests/unit/unittest/mockgeneratedfiles.h b/tests/unit/unittest/mockgeneratedfiles.h index 171455a6360..75b182355c2 100644 --- a/tests/unit/unittest/mockgeneratedfiles.h +++ b/tests/unit/unittest/mockgeneratedfiles.h @@ -38,6 +38,7 @@ public: void (const ClangBackEnd::FilePaths &filePaths)); MOCK_CONST_METHOD0(fileContainers, const ClangBackEnd::V2::FileContainers &()); + MOCK_CONST_METHOD0(isValid, bool()); void update(ClangBackEnd::V2::FileContainers &&fileContainers) { diff --git a/tests/unit/unittest/mockprojectparts.h b/tests/unit/unittest/mockprojectparts.h index f81eabc91ca..7c7a2daf4aa 100644 --- a/tests/unit/unittest/mockprojectparts.h +++ b/tests/unit/unittest/mockprojectparts.h @@ -33,11 +33,13 @@ class MockProjectParts : public ClangBackEnd::ProjectPartsInterface { public: MOCK_METHOD1(update, - ClangBackEnd::ProjectPartContainers(const ClangBackEnd::ProjectPartContainers &projectsParts)); - MOCK_METHOD1(remove, - void(const Utils::SmallStringVector &projectPartIds)); - MOCK_CONST_METHOD1(projects, - ClangBackEnd::ProjectPartContainers(const Utils::SmallStringVector &projectPartIds)); + ClangBackEnd::ProjectPartContainers( + const ClangBackEnd::ProjectPartContainers &projectsParts)); + MOCK_METHOD1(remove, void(const Utils::SmallStringVector &projectPartIds)); + MOCK_CONST_METHOD1( + projects, ClangBackEnd::ProjectPartContainers(const Utils::SmallStringVector &projectPartIds)); + MOCK_METHOD1(updateDeferred, void(const ClangBackEnd::ProjectPartContainers &projectsParts)); + MOCK_METHOD0(deferredUpdates, ClangBackEnd::ProjectPartContainers()); ClangBackEnd::ProjectPartContainers update(ClangBackEnd::ProjectPartContainers &&projectsParts) override { diff --git a/tests/unit/unittest/pchmanagerserver-test.cpp b/tests/unit/unittest/pchmanagerserver-test.cpp index 9aab088b33f..9100b51dc30 100644 --- a/tests/unit/unittest/pchmanagerserver-test.cpp +++ b/tests/unit/unittest/pchmanagerserver-test.cpp @@ -54,8 +54,8 @@ class PchManagerServer : public ::testing::Test { server.setClient(&mockPchManagerClient); - ON_CALL(mockProjectParts, update(projectParts)) - .WillByDefault(Return(projectParts)); + ON_CALL(mockProjectParts, update(projectParts)).WillByDefault(Return(projectParts)); + ON_CALL(mockGeneratedFiles, isValid()).WillByDefault(Return(true)); } ClangBackEnd::FilePathId id(Utils::SmallStringView path) const @@ -183,7 +183,6 @@ TEST_F(PchManagerServer, SetProgress) server.setProgress(20, 30); } - TEST_F(PchManagerServer, RemoveToolChainsArguments) { server.updateProjectParts( @@ -195,5 +194,60 @@ TEST_F(PchManagerServer, RemoveToolChainsArguments) server.pathsWithIdsChanged({projectPart1.projectPartId}); } +TEST_F(PchManagerServer, DontGeneratePchIfGeneratedFilesAreNotValid) +{ + InSequence s; + + EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(false)); + EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _)).Times(0); + + server.updateProjectParts( + ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}}); +} + +TEST_F(PchManagerServer, GeneratePchIfGeneratedFilesAreValid) +{ + InSequence s; + + EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(true)); + EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _)); + + server.updateProjectParts( + ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}}); +} + +TEST_F(PchManagerServer, AfterUpdatingGeneratedFilesAreValidSoGeneratePchs) +{ + InSequence s; + ClangBackEnd::UpdateGeneratedFilesMessage updateGeneratedFilesMessage{{generatedFile}}; + ON_CALL(mockGeneratedFiles, isValid()).WillByDefault(Return(false)); + server.updateProjectParts( + ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}}); + + EXPECT_CALL(mockGeneratedFiles, update(updateGeneratedFilesMessage.generatedFiles)); + EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(true)); + EXPECT_CALL(mockProjectParts, deferredUpdates()) + .WillOnce(Return(ClangBackEnd::ProjectPartContainers{projectPart1})); + EXPECT_CALL(mockPchTaskGenerator, + addProjectParts(ElementsAre(projectPart1), ElementsAre("toolChainArgument"))); + + server.updateGeneratedFiles(updateGeneratedFilesMessage.clone()); +} + +TEST_F(PchManagerServer, AfterUpdatingGeneratedFilesAreStillInvalidSoNoPchsGeneration) +{ + InSequence s; + ClangBackEnd::UpdateGeneratedFilesMessage updateGeneratedFilesMessage{{generatedFile}}; + ON_CALL(mockGeneratedFiles, isValid()).WillByDefault(Return(false)); + server.updateProjectParts( + ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}}); + + EXPECT_CALL(mockGeneratedFiles, update(updateGeneratedFilesMessage.generatedFiles)); + EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(false)); + EXPECT_CALL(mockProjectParts, deferredUpdates()).Times(0); + EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _)).Times(0); + + server.updateGeneratedFiles(updateGeneratedFilesMessage.clone()); +} } diff --git a/tests/unit/unittest/projectparts-test.cpp b/tests/unit/unittest/projectparts-test.cpp index 87ab509206b..8ab342b2f18 100644 --- a/tests/unit/unittest/projectparts-test.cpp +++ b/tests/unit/unittest/projectparts-test.cpp @@ -191,7 +191,6 @@ TEST_F(ProjectParts, GetProjectById) ASSERT_THAT(projectPartContainers, ElementsAre(projectPartContainer1)); } - TEST_F(ProjectParts, GetProjectsByIds) { projectParts.update({projectPartContainer1, projectPartContainer2}); @@ -201,4 +200,31 @@ TEST_F(ProjectParts, GetProjectsByIds) ASSERT_THAT(projectPartContainers, UnorderedElementsAre(projectPartContainer1, projectPartContainer2)); } +TEST_F(ProjectParts, UpdateDeferred) +{ + auto projectPartContainers = projectParts.update({projectPartContainer1, projectPartContainer2}); + + projectParts.updateDeferred({projectPartContainer1}); + + ASSERT_THAT(projectParts.deferredUpdates(), ElementsAre(projectPartContainer1)); + +} + +TEST_F(ProjectParts, NotUpdateDeferred) +{ + auto projectPartContainers = projectParts.update({projectPartContainer1, projectPartContainer2}); + + ASSERT_THAT(projectParts.deferredUpdates(), IsEmpty()); +} + +TEST_F(ProjectParts, UpdateDeferredCleansDeferredUpdates) +{ + auto projectPartContainers = projectParts.update({projectPartContainer1, projectPartContainer2}); + projectParts.updateDeferred({projectPartContainer1}); + + projectParts.deferredUpdates(); + + ASSERT_THAT(projectParts.deferredUpdates(), IsEmpty()); + +} }