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 <TheLartians@users.noreply.github.com>
Co-authored-by: Lars Melchior <lars.melchior@gmail.com>
Co-authored-by: Borislav Stanimirov <b.stanimirov@abv.bg>

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()

test/integration/lib.rb

@@ -180,19 +180,20 @@ class IntegrationTest < Test::Unit::TestCase
     @@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
+    if from_template
-      template_dir = File.join(TestLib::TEMPLATES_DIR, template_dir)
+      from_template = File.join(TestLib::TEMPLATES_DIR, from_template)
-      raise "#{template_dir} is not a directory" if !File.directory?(template_dir)
+      raise "#{from_template} is not a directory" if !File.directory?(from_template)
-      FileUtils.copy_entry template_dir, src_dir
+      FileUtils.copy_entry from_template, src_dir
     end
 
     Project.new src_dir, base + '-bin'

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`.
+* `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 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.
+    * 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.
 

test/integration/test_basics.rb

@@ -5,7 +5,7 @@ require_relative './lib'
 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 @@ class Basics < IntegrationTest
   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
 

test/integration/test_fetchcontent_compatibility.rb

@@ -9,8 +9,8 @@ class FetchContentCompatibility < IntegrationTest
   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

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/testpack-fibadder@1.0.0")'
+      prj
+    }.map { |prj|
+      Thread.new do
+        assert_success prj.configure
+        assert_success prj.build
+      end
+    }.map(&:join)
+  end
+end

test/integration/test_remove_source_dir.rb

@@ -2,8 +2,8 @@ require_relative './lib'
 
 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

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
 

test/integration/test_source_cache.rb

@@ -9,7 +9,7 @@ class SourceCache < IntegrationTest
   end
 
   def test_add_remove_dependency
-    prj = make_project 'using-fibadder'
+    prj = make_project from_template: 'using-fibadder'
 
     ###################################
     # create
@@ -45,7 +45,7 @@ class SourceCache < IntegrationTest
   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/testpack-fibadder@1.1.0")'
     assert_success prj.configure
 

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,13 +51,7 @@ execute_process(
 assert_equal(${ret} "0")
 assert_exists("${CPM_SOURCE_CACHE_DIR}/fibonacci")
 
-file(GLOB FIBONACCI_VERSIONs "${CPM_SOURCE_CACHE_DIR}/fibonacci/*")
+assert_cache_directory_count("${CPM_SOURCE_CACHE_DIR}/fibonacci/*" 1)
-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")
 
 # Update dependency and keep CPM_SOURCE_CACHE
 
@@ -54,12 +59,9 @@ 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/*")
+assert_cache_directory_count("${CPM_SOURCE_CACHE_DIR}/fibonacci/*" 2)
-list(LENGTH FIBONACCI_VERSIONs FIBONACCI_VERSION_count)
-assert_equal(${FIBONACCI_VERSION_count} "2")
 
 # Clear cache and update