-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add file locking to support parallel runs. #427
Merged
Merged
Changes from 23 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
da8ba91
Add file locking to support parallel runs.
PercentBoat4164 6abda4b
Fixed formatting.
PercentBoat4164 218c079
Prevent double locking file.
PercentBoat4164 ea6411f
Fix SegFault from test 2.
PercentBoat4164 665ef68
Remove unnecessary debugging messages.
PercentBoat4164 f4ad1d6
Lock the package directory rather than the cache directory.
PercentBoat4164 d657584
Lock the version specific cache entry rather than the package specifi…
PercentBoat4164 773228e
Remove unnecessary arguments in conditional statements.
PercentBoat4164 3ecd96a
Change back to locking entire cache directory.
PercentBoat4164 83a4a51
Only check CPM_HAS_CACHE_LOCK.
PercentBoat4164 b2fba4e
Lock on a per-package basis rather than the entire cache.
PercentBoat4164 9d72172
Clean up the locked file.
PercentBoat4164 2250917
Unlock then remove to fix Windows.
PercentBoat4164 9f1747f
Merge branch 'cpm-cmake:master' into master
PercentBoat4164 9f4a952
Specify use of cmake.lock as the lock file.
PercentBoat4164 9660df6
Merge branch 'master' into master
TheLartians 5ec1db4
Merge branch 'cpm-cmake:master' into master
PercentBoat4164 579f272
- Changed CPM_HAS_CACHE_LOCK to ${CPM_ARGS_NAME}_CPM_HAS_CACHE_LOCK.
PercentBoat4164 85f3ebb
Add unit test.
PercentBoat4164 70e4f57
Actually test if resulting git cache is clean in unit test.
PercentBoat4164 b2eb097
- Added comments
PercentBoat4164 937abb1
convert parallelism test to integration test
TheLartians a5a33b8
remove comment
TheLartians 82c05f5
- Removed now unnecessary variable.
PercentBoat4164 0397a72
Forgot to change variable name.
PercentBoat4164 dd2ef70
Add similar changes to the missed section.
PercentBoat4164 0bc1f2d
Fixed formatting.
PercentBoat4164 c031a3d
Unlock the file, but do not delete it.
PercentBoat4164 88592ef
Only unlock the file if it exists.
PercentBoat4164 42ef7b6
Changed cache.cmake test to ignore non-directory entries.
PercentBoat4164 bc00dd7
Integration test lib make_project:
iboB dce53a1
- Moved checks to function.
PercentBoat4164 5789a87
- Fix formatting
PercentBoat4164 dffafed
Switch to snake case.
PercentBoat4164 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
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 | ||
|
||
[*1..4] | ||
.map{ |i| | ||
prj = make_project '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 { |t| t.join } | ||
|
||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this (local) variable useless since you are always going to lock and release the file anyways ?
Besides this doesnt need to be done when clones are operated into different directories (i.e. when
CPM_SOURCE_CACHE
is not used, which could be a somewhat decent condition to check ?).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It looks like these variables are useless. The reason it exists is that in the past I had come across an error that occurred because the point of execution entered the locked block then left it and went back into it without going past the unlocking line. This would cause an error along the lines of
file already locked!
to occur. The local variable was used to solve that problem. I just tried it again without the${CPM_ARGS_NAME}_CPM_HAS_CACHE_LOCK
and it succeeded, so I will happily remove the checks.As for the
CPM_SOURCE_CACHE
check, a while ago I moved all of my changes to within a block that checked for this. see line 700. If this is not a good place for it, I can move it elsewhere and perform my own checks.