Add file locking to support parallel runs. (#427)

* Add file locking to support parallel runs. * Fixed formatting. * Prevent double locking file. * Fix SegFault from test 2. * Remove unnecessary debugging messages. * Lock the package directory rather than the cache directory. Only synchronize if CPM_SOURCE_CACHE is defined. * Lock the version specific cache entry rather than the package specific entry. * Remove unnecessary arguments in conditional statements. * Change back to locking entire cache directory. * Only check CPM_HAS_CACHE_LOCK. * Lock on a per-package basis rather than the entire cache. * Clean up the locked file. * Unlock then remove to fix Windows. * Specify use of cmake.lock as the lock file. * - Changed CPM_HAS_CACHE_LOCK to ${CPM_ARGS_NAME}_CPM_HAS_CACHE_LOCK. - Removed redundant variable initialization. * Add unit test. * Actually test if resulting git cache is clean in unit test. * - Added comments - Fixed formatting - Removed unnecessary imports * convert parallelism test to integration test * remove comment * - Removed now unnecessary variable. - Only delete file instead of unlocking it then deleting it. * Forgot to change variable name. * Add similar changes to the missed section. * Fixed formatting. * Unlock the file, but do not delete it. * Only unlock the file if it exists. * Changed cache.cmake test to ignore non-directory entries. * Integration test lib make_project: * keyword args * 'name' arg to allow multiple projects from the same test * - Moved checks to function. - Fixed small grammatical errors. * - Fix formatting * Switch to snake case. --------- Co-authored-by: Lars Melchior <[email protected]> Co-authored-by: Lars Melchior <[email protected]> Co-authored-by: Borislav Stanimirov <[email protected]>
cpm-cmake · Jan 28, 2023 · 09b056a · 09b056a
1 parent d34d2a8
commit 09b056a
Show file tree

Hide file tree

Showing 10 changed files with 60 additions and 28 deletions.
diff --git a/cmake/CPM.cmake b/cmake/CPM.cmake
@@ -712,6 +712,9 @@ function(CPMAddPackage)
     # relative paths.
     get_filename_component(download_directory ${download_directory} ABSOLUTE)
     list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS SOURCE_DIR ${download_directory})
+
+    file(LOCK ${download_directory}/../cmake.lock)
+
     if(EXISTS ${download_directory})
       cpm_store_fetch_properties(
         ${CPM_ARGS_NAME} "${download_directory}"
@@ -790,6 +793,10 @@ function(CPMAddPackage)
     cpm_get_fetch_properties("${CPM_ARGS_NAME}")
   endif()
 
+  if(EXISTS ${download_directory}/../cmake.lock)
+    file(LOCK ${download_directory}/../cmake.lock RELEASE)
+  endif()
+
   set(${CPM_ARGS_NAME}_ADDED YES)
   cpm_export_variables("${CPM_ARGS_NAME}")
 endfunction()

diff --git a/test/integration/lib.rb b/test/integration/lib.rb
@@ -180,19 +180,20 @@ def cur_test_dir
     @@test_dir
   end
 
-  def make_project(template_dir = nil)
+  def make_project(name: nil, from_template: nil)
     test_name = local_name
     test_name = test_name[5..] if test_name.start_with?('test_')
 
     base = File.join(cur_test_dir, test_name)
+    base += "-#{name}" if name
     src_dir = base + '-src'
 
     FileUtils.mkdir_p src_dir
 
-    if template_dir
-      template_dir = File.join(TestLib::TEMPLATES_DIR, template_dir)
-      raise "#{template_dir} is not a directory" if !File.directory?(template_dir)
-      FileUtils.copy_entry template_dir, src_dir
+    if from_template
+      from_template = File.join(TestLib::TEMPLATES_DIR, from_template)
+      raise "#{from_template} is not a directory" if !File.directory?(from_template)
+      FileUtils.copy_entry from_template, src_dir
     end
 
     Project.new src_dir, base + '-bin'

diff --git a/test/integration/reference.md b/test/integration/reference.md
@@ -1,4 +1,4 @@
-# Integration Test Framework Refernce
+# Integration Test Framework Reference
 
 ## `TestLib`
 
@@ -60,6 +60,7 @@ The class which must be a parent of all integration test case classes. It itself
 ### Utils
 
 * `cur_test_dir` - the directory of the current test case. A subdirectory of `TestLib::TMP_DIR`
-* `make_project(template_dir = nil)` - create a project from a test method. Will create a the project's source and binary directories as subdirectories of `cur_test_dir`.
-    * Optionally work with a template directory, in which case it will copy the contents of the template directory (one from `templates`) in the project's source directory.
+* `make_project(name: nil, from_template: nil)` - create a project from a test method. Will create the project's source and binary directories as subdirectories of `cur_test_dir`.
+    * Optionally provide a name which will be concatenated to the project directory. This allows creating multiple projects in a test
+    * Optionally work with a template, in which case it will copy the contents of the template directory (one from `templates`) in the project's source directory.
 
diff --git a/test/integration/test_basics.rb b/test/integration/test_basics.rb
@@ -5,7 +5,7 @@
 class Basics < IntegrationTest
   # Test cpm caches with no cpm-related env vars
   def test_cpm_default
-    prj = make_project 'no-deps'
+    prj = make_project from_template: 'no-deps'
     prj.create_lists_from_default_template
     assert_success prj.configure
 
@@ -38,7 +38,7 @@ def test_cpm_default
   def test_env_cpm_source_cache
     ENV['CPM_SOURCE_CACHE'] = cur_test_dir
 
-    prj = make_project 'no-deps'
+    prj = make_project from_template: 'no-deps'
     prj.create_lists_from_default_template
     assert_success prj.configure
 

diff --git a/test/integration/test_fetchcontent_compatibility.rb b/test/integration/test_fetchcontent_compatibility.rb
@@ -9,8 +9,8 @@ def setup
   end
 
   def test_add_dependency_cpm_and_fetchcontent
-    prj = make_project 'using-adder'
-    
+    prj = make_project from_template: 'using-adder'
+
     prj.create_lists_from_default_template package: <<~PACK
       CPMAddPackage(
         NAME testpack-adder

diff --git a/test/integration/test_parallelism.rb b/test/integration/test_parallelism.rb
@@ -0,0 +1,21 @@
+require_relative './lib'
+
+class Parallelism < IntegrationTest
+  def setup
+    @cache_dir = File.join(cur_test_dir, 'cpmcache')
+    ENV['CPM_SOURCE_CACHE'] = @cache_dir
+  end
+
+  def test_populate_cache_in_parallel
+    4.times.map { |i|
+      prj = make_project name: i.to_s, from_template: 'using-fibadder'
+      prj.create_lists_from_default_template package: 'CPMAddPackage("gh:cpm-cmake/[email protected]")'
+      prj
+    }.map { |prj|
+      Thread.new do
+        assert_success prj.configure
+        assert_success prj.build
+      end
+    }.map(&:join)
+  end
+end
diff --git a/test/integration/test_remove_source_dir.rb b/test/integration/test_remove_source_dir.rb
@@ -2,8 +2,8 @@
 
 class RemoveSourceDir < IntegrationTest
   def test_remove_source_dir
-    prj = make_project 'using-adder'
-    
+    prj = make_project from_template: 'using-adder'
+
     prj.create_lists_from_default_template package: <<~PACK
       CPMAddPackage(
         NAME testpack-adder

diff --git a/test/integration/test_simple.rb b/test/integration/test_simple.rb
@@ -4,7 +4,7 @@ class Simple < IntegrationTest
   ADDER_PACKAGE_NAME = 'testpack-adder'
 
   def test_update_single_package
-    prj = make_project 'using-adder'
+    prj = make_project from_template: 'using-adder'
     adder_cache0 = nil
     adder_ver_file = nil
 

diff --git a/test/integration/test_source_cache.rb b/test/integration/test_source_cache.rb
@@ -9,7 +9,7 @@ def setup
   end
 
   def test_add_remove_dependency
-    prj = make_project 'using-fibadder'
+    prj = make_project from_template: 'using-fibadder'
 
     ###################################
     # create
@@ -45,7 +45,7 @@ def test_add_remove_dependency
   end
 
   def test_second_project
-    prj = make_project 'using-fibadder'
+    prj = make_project from_template: 'using-fibadder'
     prj.create_lists_from_default_template package: 'CPMAddPackage("gh:cpm-cmake/[email protected]")'
     assert_success prj.configure
 

diff --git a/test/unit/cache.cmake b/test/unit/cache.cmake
@@ -26,6 +26,17 @@ function(reset_test)
   update_cmake_lists()
 endfunction()
 
+function(assert_cache_directory_count directory count)
+  set(version_count 0)
+  file(GLOB potential_versions ${directory})
+  foreach(entry ${potential_versions})
+    if(IS_DIRECTORY ${entry})
+      math(EXPR version_count "${version_count} + 1")
+    endif()
+  endforeach()
+  assert_equal("${version_count}" "${count}")
+endfunction()
+
 set(FIBONACCI_VERSION 1.0)
 
 # Read CPM_SOURCE_CACHE from arguments
@@ -40,26 +51,17 @@ execute_process(
 assert_equal(${ret} "0")
 assert_exists("${CPM_SOURCE_CACHE_DIR}/fibonacci")
 
-file(GLOB FIBONACCI_VERSIONs "${CPM_SOURCE_CACHE_DIR}/fibonacci/*")
-list(LENGTH FIBONACCI_VERSIONs FIBONACCI_VERSION_count)
-assert_equal(${FIBONACCI_VERSION_count} "1")
-
-file(GLOB fibonacci_versions "${CPM_SOURCE_CACHE_DIR}/fibonacci/*")
-list(LENGTH fibonacci_versions fibonacci_version_count)
-assert_equal(${fibonacci_version_count} "1")
+assert_cache_directory_count("${CPM_SOURCE_CACHE_DIR}/fibonacci/*" 1)
 
 # Update dependency and keep CPM_SOURCE_CACHE
 
 set(FIBONACCI_VERSION 2.0)
 update_cmake_lists()
 
 execute_process(COMMAND ${CMAKE_COMMAND} ${TEST_BUILD_DIR} RESULT_VARIABLE ret)
-
 assert_equal(${ret} "0")
 
-file(GLOB FIBONACCI_VERSIONs "${CPM_SOURCE_CACHE_DIR}/fibonacci/*")
-list(LENGTH FIBONACCI_VERSIONs FIBONACCI_VERSION_count)
-assert_equal(${FIBONACCI_VERSION_count} "2")
+assert_cache_directory_count("${CPM_SOURCE_CACHE_DIR}/fibonacci/*" 2)
 
 # Clear cache and update