From ce331bab929c5a38e048ba15d7393dcf96fad9e1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Dec 2014 15:12:51 -0500 Subject: [PATCH] find_library: Fix repeat call after changing directory content (#15293) We use cmGlobalGenerator::GetDirectoryContent to avoid repeating directory listings. However, GetDirectoryContent loads content from disk at most once. This breaks find_library calls that occur when disk content has changed since preceding find_library calls. Teach cmGlobalGenerator::GetDirectoryContent to save the directory modification time when content is loaded and re-load content if it changes. Create a RunCMake.find_library test with a case covering this. --- Source/cmGlobalGenerator.cxx | 33 ++++++++++++------- Source/cmGlobalGenerator.h | 17 +++++----- Tests/RunCMake/CMakeLists.txt | 1 + Tests/RunCMake/find_library/CMakeLists.txt | 3 ++ .../RunCMake/find_library/Created-stderr.txt | 2 ++ Tests/RunCMake/find_library/Created.cmake | 16 +++++++++ .../RunCMake/find_library/RunCMakeTest.cmake | 3 ++ 7 files changed, 55 insertions(+), 20 deletions(-) create mode 100644 Tests/RunCMake/find_library/CMakeLists.txt create mode 100644 Tests/RunCMake/find_library/Created-stderr.txt create mode 100644 Tests/RunCMake/find_library/Created.cmake create mode 100644 Tests/RunCMake/find_library/RunCMakeTest.cmake diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index 29ab7d0ab..dd7fbc8a4 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -2725,7 +2725,9 @@ void cmGlobalGenerator::AddToManifest(const std::string& config, // Add to the content listing for the file's directory. std::string dir = cmSystemTools::GetFilenamePath(f); std::string file = cmSystemTools::GetFilenameName(f); - this->DirectoryContentMap[dir].insert(file); + DirectoryContent& dc = this->DirectoryContentMap[dir]; + dc.Generated.insert(file); + dc.All.insert(file); } //---------------------------------------------------------------------------- @@ -2733,25 +2735,32 @@ std::set const& cmGlobalGenerator::GetDirectoryContent(std::string const& dir, bool needDisk) { DirectoryContent& dc = this->DirectoryContentMap[dir]; - if(needDisk && !dc.LoadedFromDisk) + if(needDisk) { - // Load the directory content from disk. - cmsys::Directory d; - if(d.Load(dir)) + long mt = cmSystemTools::ModifiedTime(dir); + if (mt != dc.LastDiskTime) { - unsigned long n = d.GetNumberOfFiles(); - for(unsigned long i = 0; i < n; ++i) + // Reset to non-loaded directory content. + dc.All = dc.Generated; + + // Load the directory content from disk. + cmsys::Directory d; + if(d.Load(dir)) { - const char* f = d.GetFile(i); - if(strcmp(f, ".") != 0 && strcmp(f, "..") != 0) + unsigned long n = d.GetNumberOfFiles(); + for(unsigned long i = 0; i < n; ++i) { - dc.insert(f); + const char* f = d.GetFile(i); + if(strcmp(f, ".") != 0 && strcmp(f, "..") != 0) + { + dc.All.insert(f); + } } } + dc.LastDiskTime = mt; } - dc.LoadedFromDisk = true; } - return dc; + return dc.All; } //---------------------------------------------------------------------------- diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h index 6b75298ff..08f061a6f 100644 --- a/Source/cmGlobalGenerator.h +++ b/Source/cmGlobalGenerator.h @@ -255,9 +255,9 @@ public: cmTargetManifest const& GetTargetManifest() const { return this->TargetManifest; } - /** Get the content of a directory. Directory listings are loaded - from disk at most once and cached. During the generation step - the content will include the target files to be built even if + /** Get the content of a directory. Directory listings are cached + and re-loaded from disk only when modified. During the generation + step the content will include the target files to be built even if they do not yet exist. */ std::set const& GetDirectoryContent(std::string const& dir, bool needDisk = true); @@ -486,13 +486,14 @@ private: virtual const char* GetBuildIgnoreErrorsFlag() const { return 0; } // Cache directory content and target files to be built. - struct DirectoryContent: public std::set + struct DirectoryContent { - typedef std::set derived; - bool LoadedFromDisk; - DirectoryContent(): LoadedFromDisk(false) {} + long LastDiskTime; + std::set All; + std::set Generated; + DirectoryContent(): LastDiskTime(-1) {} DirectoryContent(DirectoryContent const& dc): - derived(dc), LoadedFromDisk(dc.LoadedFromDisk) {} + LastDiskTime(dc.LastDiskTime), All(dc.All), Generated(dc.Generated) {} }; std::map DirectoryContentMap; diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index 54fe2d9ca..2bfd4d6e3 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -104,6 +104,7 @@ add_RunCMake_test(export) add_RunCMake_test(cmake_minimum_required) add_RunCMake_test(continue) add_RunCMake_test(file) +add_RunCMake_test(find_library) add_RunCMake_test(find_package) add_RunCMake_test(get_filename_component) add_RunCMake_test(if) diff --git a/Tests/RunCMake/find_library/CMakeLists.txt b/Tests/RunCMake/find_library/CMakeLists.txt new file mode 100644 index 000000000..ef2163c29 --- /dev/null +++ b/Tests/RunCMake/find_library/CMakeLists.txt @@ -0,0 +1,3 @@ +cmake_minimum_required(VERSION 3.1) +project(${RunCMake_TEST} NONE) +include(${RunCMake_TEST}.cmake) diff --git a/Tests/RunCMake/find_library/Created-stderr.txt b/Tests/RunCMake/find_library/Created-stderr.txt new file mode 100644 index 000000000..67b347482 --- /dev/null +++ b/Tests/RunCMake/find_library/Created-stderr.txt @@ -0,0 +1,2 @@ +CREATED_LIBRARY='CREATED_LIBRARY-NOTFOUND' +CREATED_LIBRARY='[^']*/Tests/RunCMake/find_library/Created-build/lib/libcreated.a' diff --git a/Tests/RunCMake/find_library/Created.cmake b/Tests/RunCMake/find_library/Created.cmake new file mode 100644 index 000000000..c0fd823c7 --- /dev/null +++ b/Tests/RunCMake/find_library/Created.cmake @@ -0,0 +1,16 @@ +list(APPEND CMAKE_FIND_LIBRARY_PREFIXES lib) +list(APPEND CMAKE_FIND_LIBRARY_SUFFIXES .a) +find_library(CREATED_LIBRARY + NAMES created + PATHS ${CMAKE_CURRENT_BINARY_DIR}/lib + NO_DEFAULT_PATH + ) +message("CREATED_LIBRARY='${CREATED_LIBRARY}'") +file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/lib) +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/lib/libcreated.a" "created") +find_library(CREATED_LIBRARY + NAMES created + PATHS ${CMAKE_CURRENT_BINARY_DIR}/lib + NO_DEFAULT_PATH + ) +message("CREATED_LIBRARY='${CREATED_LIBRARY}'") diff --git a/Tests/RunCMake/find_library/RunCMakeTest.cmake b/Tests/RunCMake/find_library/RunCMakeTest.cmake new file mode 100644 index 000000000..40006799b --- /dev/null +++ b/Tests/RunCMake/find_library/RunCMakeTest.cmake @@ -0,0 +1,3 @@ +include(RunCMake) + +run_cmake(Created)