Lock file #2410
Lock file #2410
Conversation
…an produce FileLockUses; FileLock wraps FileLockUse and implements IDisposable for being cleaned when disposing ServiceProvider
…executed action in assembly GitVersion.MSBuildTask
Remove because of false impression Co-authored-by: Asbjørn Ulsberg <asbjorn@ulsberg.no>
Remove because of false impresssion Co-authored-by: Asbjørn Ulsberg <asbjorn@ulsberg.no>
Remove due to implicit copyright Co-authored-by: Asbjørn Ulsberg <asbjorn@ulsberg.no>
Co-authored-by: Asbjørn Ulsberg <asbjorn@ulsberg.no>
Remove because of false impression Co-authored-by: Asbjørn Ulsberg <asbjorn@ulsberg.no>
|
push 🎉 |
| return fileStream; | ||
| } | ||
|
|
||
| private bool waitUntilAcquired(string filePath, out FileStream? fileStream, FileMode fileMode, |
There was a problem hiding this comment.
I guess in the code base we use the convention that all methods should follow Pascal Case. Can you adjust the method names to conform our conventions?
There was a problem hiding this comment.
Lot of work here!
My only suggestion would be to consider making he locking abstraction more general purpose, and have the "file based" locking be one implementation:
ILocker locker = new FileLocker(); // we can switch implementations
using(var lock = locker.Aquire());
{
// do stuff here
}
The reason for that is that we might want to switch to other sorts of locking if we encounter issues with file based locking. For example, on .net core we may want to try a global (named) mutex implementation: https://docs.microsoft.com/en-us/dotnet/standard/threading/mutexes#local-and-system-mutexes
Other than that, I couldn't see any tests specifically for the file locker - I assume we are relying on higher level integration tests to verify that it works?
|
@asbjornu looks like the Locking interfaces are not used in our code base. |
Good find! I haven't studied the code enough to discover that yet.
Yes, indeed. |
|
Hi, All. Can you publish beta version with this fix. |
|
@riksking this is still a draft, the code implementation is not finished |
|
@asbjornu starting from your asbjornu:feature/lockfile branch, I rebased and applied the following changes: I've been using GitVersion.MsBuild and GitVersion.CommandLine nuget packages with these two changes since yesterday for our CI builds on our private GitLab instance in addition to enabling parallel builds (msbuild /m parameter). I can confirm I haven't seen any |
|
#2581 should fix this in a satisfactory manner. If anyone wants to pick this up, please fork, complete the implementation and submit a new pull request. |
A rebase and
dotnet formatfixed version of #2333. Hopefully the tests turn green so we can make this available in a beta for testing.Resolves #2333. Hopefully fixes #1031. Hopefully fixes #1381.