Skip to content
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

LibGit2Sharp.LockedFileException: The index is locked. This might be due to a concurrent or crashed process #1031

Closed
michaelnoonan opened this issue Sep 2, 2016 · 83 comments · Fixed by #2581
Labels
Milestone

Comments

@michaelnoonan
Copy link
Contributor

Wasn't sure if this was an issue or whether we shouldn't be using msbuild /m to enable concurrent builds.

[11:45:48]  [BuildSolution]     17>MSBUILD : warning : WARN [09/02/16 1:45:48:65] Could not determine assembly version: LibGit2Sharp.LockedFileException: The index is locked. This might be due to a concurrent or crashed process [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at LibGit2Sharp.Core.Ensure.HandleError(Int32 result) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at LibGit2Sharp.Core.Proxy.git_checkout_tree(RepositoryHandle repo, ObjectId treeId, GitCheckoutOpts& opts) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at LibGit2Sharp.Repository.CheckoutTree(Tree tree, IList`1 paths, IConvertableToGitCheckoutOpts opts) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at LibGit2Sharp.Repository.Checkout(Tree tree, CheckoutOptions checkoutOptions, String refLogHeadSpec) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at LibGit2Sharp.Repository.Checkout(Branch branch, CheckoutOptions options) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at LibGit2Sharp.Repository.Checkout(String committishOrBranchSpec, CheckoutOptions options) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at GitTools.Git.GitRepositoryHelper.EnsureLocalBranchExistsForCurrentBranch(Repository repo, String currentBranch) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at GitTools.Git.GitRepositoryHelper.NormalizeGitDirectory(String gitDirectory, AuthenticationInfo authentication, Boolean noFetch, String currentBranch) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at GitVersion.GitPreparer.Initialise(Boolean normaliseGitDirectory, String currentBranch) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at GitVersion.ExecuteCore.ExecuteGitVersion(String targetUrl, String dynamicRepositoryLocation, Authentication authentication, String targetBranch, Boolean noFetch, String workingDirectory, String commitId, Config overrideConfig) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]
[11:45:48]  [BuildSolution] MSBUILD : warning :    at GitVersion.ExecuteCore.TryGetVersion(String directory, VersionVariables& versionVariables, Boolean noFetch, Authentication authentication) [Y:\Work\refs\heads\master\source\Octopus.E2ETests\Octopus.E2ETests.csproj]

This occurs on TeamCity 10.0.0 building ~30 projects. We've only just upgraded to GitVersionTask.3.6.3 from GitVersionTask.3.6.1 which had the gitversion_cache issue.

In the meantime I'll try some more builds and see if it recurs frequently.

@michaelnoonan
Copy link
Contributor Author

The very next build through TeamCity failed with the exact same message and stack trace.

I'm going to revert to msbuild without the /m for now.

@JakeGinnivan
Copy link
Contributor

Id switch to the exe, run it with /updateAssemblyInfo before building.

We would have to grab a mutex, and block builds until the first task has finished calculating the version then unblock the others and they get the version from the cache.

@asbjornu
Copy link
Member

asbjornu commented Sep 5, 2016

Perhaps we could use .git/index.lock for this? I believe it would be semantically correct to lock the entire Git repository for all and any change meanwhile we calculate a version. If .git/index.lock exists, we could await version calculation until it's deleted or within a configurable time-out.

@JakeGinnivan
Copy link
Contributor

Im happy with that

@bbowman
Copy link

bbowman commented Mar 10, 2017

any updates here?

@asbjornu
Copy link
Member

@bbowman: I'm sorry, but nothing has happened besides agreeing on a solution that is yet to be implemented. We need someone in the team to have time to implement this, or get someone else to submit a PR. I unfortunately have my hands full. Perhaps you or @michaelnoonan can submit a PR?

@jnevins-gcm
Copy link

any update on this? builds take 4x as long without parallelism :(

@asbjornu
Copy link
Member

@jnevins-gcm: Sorry, there's nothing new to report on this. If you are able to provide a pull request that fixes this as described in #1031 (comment), it will be merged quickly provided it passes all tests.

@jnevins-gcm
Copy link

Thanks for the update. Even if I fix, how would I manage to get that into GitVersionTask?

Is there any short term workaround I could do with retries?

@asbjornu
Copy link
Member

@jnevins-gcm, if you submit a pull request to this repository, it will be merged and a new package of GitVersionTask will show up on NuGet that you can use.

@asbjornu
Copy link
Member

Now that we've upgraded to LibGit2Sharp version 0.26 in #1713, I hope this problem might be fixed. Please test with the latest v5 beta and reopen if the problem persists.

@StevenDevooght
Copy link

Hi,

We are having the same issue using the latest beta version (5.0.0-beta5.11).

@borrillis
Copy link

Also hitting this issue on linux and Windows (but not MacOS) build agents using GitVersion 5.0.1

@asbjornu
Copy link
Member

asbjornu commented Sep 4, 2019

@StevenDevooght and @borrillis, can you please tell me more about your build environments that might help us figure out why this is happening?

@asbjornu asbjornu reopened this Sep 4, 2019
@borrillis
Copy link

borrillis commented Sep 5, 2019

@asbjornu This is the repo and branch I am currently testing with: https://github.com/axiom3d/axiom/tree/azure-pipelines and I am only seeing it fail in the CI/CD builds in Azure DevOps (https://dev.azure.com/axiom3d/Axiom/_build/results?buildId=37&view=results), locally it is working using the command .\build.ps1 -Script build.cake -Target Build -Configuration Release -Verbosity Diagnostic --settings_skipverification=true

@asbjornu
Copy link
Member

asbjornu commented Sep 5, 2019

@StevenDevooght, are you also seeing this on Azure DevOps?

@dieterv
Copy link
Contributor

dieterv commented Sep 12, 2019

@asbjornu, We have been having the same issue in our CI/CD builds on our private GitLab instance

@StevenDevooght
Copy link

We also have the issue when building on Azure DevOps. Locally we haven't experienced any issues.

@maldworth
Copy link

Also see it in Azure Dev Ops, intermittently. Never happens when we run the cake build locally though.

@iamwyza
Copy link

iamwyza commented Oct 23, 2019

We also have this issue.

dotnet core 2.2.203
solution with a dozen or so projects

Often it's "build->fail, build->fail, build->fail, build->succeed". Doesn't ever seem to happen locally when building through VS. We've tried adding "-p:maxCpuCount=1" to the build command to see if we could force it to build without parallelization (thinking that might have something to do with it), but no dice.

edit: also this doesn't just happen on build, it also sometimes happens on "dotnet test" as well.

@asbjornu
Copy link
Member

@dieterv, since you're able to reproduce this in a local GitLab instance, we might have a chance to figure out why this is occurring. Are there any parallelization going on in your GitLab build that might cause this?

@dieterv
Copy link
Contributor

dieterv commented Oct 25, 2019

@asbjornu, I'm operating a single Gitlab build runner that accepts a single build job at a time (concurrent=1 in the config.toml file). Our CI pipeline in question runs a cake build script. The cake script runs a sequence of setup>clean>restore>validate>build>test>package>publish tasks.
The build task executes msbuild without /m parameter and with /nodeReuse:false parameters.

I've been experimenting with our setup these last couple of weeks and seem to have found something interesting. Since these changes we have seen 40 builds succeed without encountering the issue. Either we're simply extremely lucky or one or a combination of these changes "fixed" it for us. I was going to wait a bit longer to be sure, but well, maybe this information can help...

  1. Our cake script was executing GitVersion.exe /output json and stored the output for later use in the setup task. That is before msbuild was executed in the build task. I've switched the order around and now only ever execute GitVersion.exe after the build succeeds in the publish task.

  2. Without exception, all our projects contained a package reference to GitVersionTask. I've removed the reference for our xunit test projects (output type=class library)

  3. I simplified the gitversion.yml file a bit. No idea why it had grown so complex. I now have:

mode: ContinuousDeployment
branches:
  master:
    tag: master
    increment: Patch
    prevent-increment-of-merged-branch-version: true
    track-merge-target: false
  release:
    tag: beta
    increment: Patch
    prevent-increment-of-merged-branch-version: true
    track-merge-target: false
  feature:
    tag: useBranchName
    increment: Inherit
    prevent-increment-of-merged-branch-version: true
    track-merge-target: false
ignore:
  sha: []

Before we had:

diff --git a/gitversion.yml b/gitversion.yml
index 11204d4c..4bacb5b4 100644
--- a/gitversion.yml
+++ b/gitversion.yml
@@ -1,32 +1,19 @@
 mode: ContinuousDeployment
-tag-prefix: '[vV]'
-continuous-delivery-fallback-tag: ci
-major-version-bump-message: '\+semver:\s?(breaking|major)'
-minor-version-bump-message: '\+semver:\s?(feature|minor)'
-patch-version-bump-message: '\+semver:\s?(fix|patch)'
-no-bump-message: '\+semver:\s?(none|skip)'
-legacy-semver-padding: 4
-build-metadata-padding: 4
-commits-since-version-source-padding: 4
-commit-message-incrementing: Enabled
 branches:
   master:
-    mode: ContinuousDeployment
     tag: master
     increment: Patch
     prevent-increment-of-merged-branch-version: true
     track-merge-target: false
   release:
-    mode: ContinuousDeployment
     tag: beta                                                                                                                                                                                                                                     increment: Patch                                                                                                                                                                                                                              prevent-increment-of-merged-branch-version: true                                                                                                                                                                                              track-merge-target: false                                                                                                                                                                                                                   feature:                                                                                                                                                                                                                                   -    mode: ContinuousDeployment                                                                                                                                                                                                                    tag: useBranchName                                                                                                                                                                                                                            increment: Inherit                                                                                                                                                                                                                       -    prevent-increment-of-merged-branch-version: false                                                                                                                                                                                        +    prevent-increment-of-merged-branch-version: true                                                                                                                                                                                              track-merge-target: false                                                                                                                                                                                                                 ignore:                                                                                                                                                                                                                                         sha: []

Hope this helps!

@iamwyza
Copy link

iamwyza commented Oct 25, 2019

One thing I'm trying as a workaround right now (Azure DevOps Pipeline) is to do this:

  1. Run a powershell script that will remove the node for GitVersionTask from csproj for all the projects. This means that it won't use the msbuild functionality at all.
#Removes GitVersionTask from all csproj files before we build
Get-ChildItem .\*.csproj -Recurse | % {
    [Xml]$xml = Get-Content $_.FullName
    $xml | Select-Xml -XPath '//*[@Include="GitVersionTask"]' | ForEach-Object{$_.Node.ParentNode.RemoveChild($_.Node)}
    $xml.Save($_.FullName)
}
  1. Added a GitVersionTask execution in the pipeline which runs and sets the various version strings as pipeline variables
  2. Update the dotnet build step to include several arguments which sets the versions from step 2. That looks like this:
    /p:Version="$(GitVersion.AssemblySemVer)" /p:FileVersion="$(GitVersion.AssemblySemFileVer)" /p:InformationalVersion="$(GitVersion.InformationalVersion)" /p:PackageVersion="$(GitVersion.NugetVersion)"

Since we don't use AssemblyInfo.cs at all (dotnet core SDK), this seems to work. Whether it will fix the issue is unknown since it's wildly unpredictable as to when it happens.

edit: added the powershell script I used.

@cytron201
Copy link

cytron201 commented Dec 3, 2019

One thing I'm trying as a workaround right now (Azure DevOps Pipeline) is to do this:

  1. Run a powershell script that will remove the node for GitVersionTask from csproj for all the projects. This means that it won't use the msbuild functionality at all.
#Removes GitVersionTask from all csproj files before we build
Get-ChildItem .\*.csproj -Recurse | % {
    [Xml]$xml = Get-Content $_.FullName
    $xml | Select-Xml -XPath '//*[@Include="GitVersionTask"]' | ForEach-Object{$_.Node.ParentNode.RemoveChild($_.Node)}
    $xml.Save($_.FullName)
}
  1. Added a GitVersionTask execution in the pipeline which runs and sets the various version strings as pipeline variables
  2. Update the dotnet build step to include several arguments which sets the versions from step 2. That looks like this:
    /p:Version="$(GitVersion.AssemblySemVer)" /p:FileVersion="$(GitVersion.AssemblySemFileVer)" /p:InformationalVersion="$(GitVersion.InformationalVersion)" /p:PackageVersion="$(GitVersion.NugetVersion)"

Since we don't use AssemblyInfo.cs at all (dotnet core SDK), this seems to work. Whether it will fix the issue is unknown since it's wildly unpredictable as to when it happens.

edit: added the powershell script I used.

This is working for us - the only annoying thing is that package version does not contain the full semver for commits ahead in the same way the gitversion package does. i.e. 1.9.0-alpha0123 Vs. 1.9.0-alpha.122.

Much like the others, we're seeing this using on Azure DevOps, using the .Net Core build step. We're building a solution with 19 projects, of which around 16 have the gitversion NuGet package.

edit: using "Automatic package versioning" option on the .Net Core task with the Pack option, we can set the Environment Variable option to be GITVERSION_FullSemVer. This is working well.

@rose-a
Copy link
Contributor

rose-a commented Dec 3, 2019

We're experiencing this too on a local GitLab instance when building solutions which contain multiple projects that reference GitVersionTask.

My workaround:

Put the following in each project file (or a Directory.build.props file in the solution dir):

<PropertyGroup Condition=" '$(GitVersion_SemVer)' != ''">
    <GetVersion>false</GetVersion>
    <WriteVersionInfoToBuildLog>false</WriteVersionInfoToBuildLog>
    <UpdateAssemblyInfo>false</UpdateAssemblyInfo>

    <Version>$(GitVersion_FullSemVer)</Version>
    <VersionPrefix>$(GitVersion_MajorMinorPatch)</VersionPrefix>
    <VersionSuffix Condition=" '$(UseFullSemVerForNuGet)' == 'false' ">$(GitVersion_NuGetPreReleaseTag)</VersionSuffix>
    <VersionSuffix Condition=" '$(UseFullSemVerForNuGet)' == 'true' ">$(GitVersion_PreReleaseTag)</VersionSuffix>
    <PackageVersion Condition=" '$(UseFullSemVerForNuGet)' == 'false' ">$(GitVersion_NuGetVersion)</PackageVersion>
    <PackageVersion Condition=" '$(UseFullSemVerForNuGet)' == 'true' ">$(GitVersion_FullSemVer)</PackageVersion>
    <InformationalVersion Condition=" '$(InformationalVersion)' == '' ">$(GitVersion_InformationalVersion)</InformationalVersion>
    <AssemblyVersion Condition=" '$(AssemblyVersion)' == '' ">$(GitVersion_AssemblySemVer)</AssemblyVersion>
    <FileVersion Condition=" '$(FileVersion)' == '' ">$(GitVersion_AssemblySemFileVer)</FileVersion>
    <RepositoryBranch Condition=" '$(RepositoryBranch)' == '' ">$(GitVersion_BranchName)</RepositoryBranch>
    <RepositoryCommit Condition=" '$(RepositoryCommit)' == '' ">$(GitVersion_Sha)</RepositoryCommit>
  </PropertyGroup>
</Project>

Then run gitversion before executing the build and write the output into environment variables. This way gitversion is only run once and the result is then reused in each project being built.

Edit: PowerShell Script

gitversion /output json | Out-String | ConvertFrom-Json | ForEach-Object { foreach ($item in $_.PSObject.properties) { Set-Item -Path "env:GitVersion_$($item.Name)" -Value $item.Value } }

Edit 2

You also need to set the version properties (which is normally performed within the GetVersion task here) and disable other tasks like WriteVersionInfoToBuildLog.

@iamwyza
Copy link

iamwyza commented Dec 3, 2019

Just as a followup. The approach I posted back in October has been running fine since. I had to do a little bit of adjustments to the way the GitVersion.yml was setup because it seemed to behave just slightly differently than when using the msbuild way, but otherwise it's been running without issue since. So at least now things seem stable and reliable. Having to only run gitversion once at the beginning seems to be the way to go.

@rose-a
Copy link
Contributor

rose-a commented Dec 3, 2019

During my inquiries I realized that each task is executing gitVersionCalculator.CalculateVersionVariables() on its own, which tries to access the repository (and probably fails with the afforementioned error) before it even checks if a cache or a gitversion.properties file exists.

I'd suggest to try to restructure the build tasks, so that:

  1. There is a single task in the chain which actually runs gitVersionCalculator.CalculateVersionVariables(). All the other tasks depend on its output.
  2. It should be possible to enable the use of the gitversion.properties file most of the build-server extensions generate for populating the GitVersion variables

@isc30
Copy link

isc30 commented May 29, 2020

@admin-simeon thanks for this solution. I found some issues with it, maybe we can work together to get a fully working workaround:

  • GitVersion.yml is not taken in consideration
  • When using <GeneratePackageOnBuild>, the nuget package doesn't contain subdependencies with the proper version (they are 0.0.0)
  • This target cannot be run before Build or Restore, which makes it hard to fix some of these issues

isc30/blazor-lazy-loading#61

@admin-simeon
Copy link

@isc30

  • My GitVersion.yml is taken into consideration - it is in the same directory as the sln file/the root of the git repository. Do you have GitVersion.proj_ somewhere underneath your Git repo or is it somewhere else? That could be the difference

  • GeneratePackageOnBuild with subdependencies - good find...does this not happen with the default task? I would assume this is because Pack isn't triggering versioning of the referenced projects...

  • What do you mean by "it can't run"?

@teneko
Copy link
Contributor

teneko commented Jun 8, 2020

Somehow I feel sad about GitVersionTask. It seems not thought to the end. It does not take into account a multi project solution and even less it is built for concurrency. Why do you rely on locking machanism of LibGit2Sharp? It does not seems to work. So just don't rely on it. Caching is nice, but as long you need make lookup git-metadata for each call to GetVersion to look for such a cache on MSBuild-level (you call GetVersion each time), you will run into these lock exception at some time.

Currently I am working on an add-in, that just disables the task GetVersion of the package GitVersionTask and does this stuff by itself with the help of GitVersion.exe. What's the reason you not using GitVersion.exe by yourself in GitVersionTask(.MSBuild) (@asbjornu)? GitVesionTask could carry it on by itself and could replace the current implementation of the task GetVersion with some clean lines of code. In such an implementation it would be easy to implement a global lock mechanism and even to implement a kind of global identifier for caching purposes of solution with multi projects that rely heavily on GitVersionTask.

@asbjornu
Copy link
Member

asbjornu commented Jun 8, 2020

I'm sad about the current state of GitVersionTask too, @teroneko. Parallelism is something Microsoft introduced as default in later versions of .NET, which is why the issue with locking is "new". It has always existed, though.

I think the proper solution to this problem may come with version 6 of GitVersion (see #2275) , where we modularize and separate things so a build can be created from a sequence of steps instead of just a single GitVersion step as is the current way of operation. A suggested way to do this in version 6 would be:

  1. Execute gitversion calculate and store the version variables in gitversion.json
  2. Configure GitVersionTask to read version variables from gitversion.json instead of doing its own calculation.
  3. Perform dotnet build.

I think this should work and that it is doable. The only optimization I would like to do, if possible, is to add some sort of hook into dotnet build so step 1 and 3 can be merged (i.e. dotnet build executes gitversion calculate before compilation starts).

Fixing this in version 5 would require someone to submit a pull request that provides a working file locking implementation. Until such a PR exists, this issue is going to stay open, I'm afraid.

@teneko
Copy link
Contributor

teneko commented Jun 8, 2020

As I am in working for an add-in that also allows nested GitVersion.yml's, but same .git-folder, I would agree to contribute a proper locking implementation. I see two options: Mutex or a file lock with a simple "sleep-and-retry-until-lock-release"-implementation. I think I will use last option.

May I ask you, if your interpretation of dotnet build belongs to a solution or a set of projects, or only to a single project, like GitVersionTask is designed? Because then, you are going to shift logic from GitVersionTask.MSBuild to GitVersion(Core), for modularity and simplicity right?

I just want to give you here three scenarios in which GitVersionTask is necessary.

  1. You are building a single project from VS or command line that belongs to the same .git-folder.
  2. You are building a single solution from VS or command line that belongs to the same .git-folder. GitVersionTask should be involved in each project (scoped by next available gitversion.yml and to next higher .git-folder if you want to make nested gitversion.yml be possible).
  3. You are building a set of projects from VS (by selection), command line (multiple projects seperated by ; in dotnet msbuild -t:build or multiple projects with one or many --project= in dotnet build), or with cake/custom build project that also all belong to the same .git-folder. GitVersionTask should be involed in each project (see 2.)

As you can see, it is necessary, that each project should have a package reference to GitVersionTask for its independency. To have the calculation and such logic like described (scope resolving and scoped caching) above in one product, let call it GitVersion.exe, would be great.

I red your proposal, and I think you want GitVersionTask behave the same like before, but with just moved logic like I described above, right?

@asbjornu
Copy link
Member

asbjornu commented Jun 9, 2020

As I am in working for an add-in that also allows nested GitVersion.yml's, but same .git-folder, I would agree to contribute a proper locking implementation.

🙏 👏 🙏 👏 🙏

I see two options: Mutex or a file lock with a simple "sleep-and-retry-until-lock-release"-implementation. I think I will use last option.

As long as the method chosen is cross-platform, any method will do.

May I ask you, if your interpretation of dotnet build belongs to a solution or a set of projects, or only to a single project, like GitVersionTask is designed?

In version 6 we are moving towards executing gitversion calculate once at the start of the build, and then doing all the various transformations, outputs, code generation, etc., as individual steps that may occur once per project, once per AssemblyInfo file, etc. after.

Because then, you are going to shift logic from GitVersionTask.MSBuild to GitVersion(Core), for modularity and simplicity right?

Yes, absolutely.

As you can see, it is necessary, that each project should have a package reference to GitVersionTask for its independency. To have the calculation and such logic like described (scope resolving and scoped caching) above in one product, let call it GitVersion.exe, would be great.

That's the plan. Each GitVersionTask instance don't need to calculate their own version number, as the result will be the same each and every time.

I red your proposal, and I think you want GitVersionTask behave the same like before, but with just moved logic like I described above, right?

Yes.

@teneko
Copy link
Contributor

teneko commented Jun 10, 2020

I have implemented a simple and generic lock file utility. My image of current GitVersion implementation is, that each executable project implements his own concrete logic to grab the native binaries to be able to work with GitVersionCore.
So my question to you is, where you want to put me the lock? And another question: what can I lock?
Locking on .git/index.lock or any other file provided by git/LibGit2Sharp is not desirable, because you don't know if and when git or LibGit2Sharp are locking them. We should avoid any kind of possible nested locks.
At GitVersionCore level, it would be possible to lock on GitVersion.yml, and work with the filestream you get from the acquired lock. When finished with one action towards .git/gitversion_cache/ and GitVersion.yml that typically involves LibGit2Sharp, we can release lock on GitVersion.yml. What do you mean?

By the way here is the code related to file locking I want to contribute:

    public static class LockFileTools
    {
        public const FileMode DefaultFileMode = FileMode.OpenOrCreate;
        public const FileAccess DefaultFileAccess = FileAccess.ReadWrite;
        public const FileShare DefaultFileShare = FileShare.None;

        public static bool TryAcquire(string filePath, out FileStream? fileStream, FileMode fileMode = DefaultFileMode,
            FileAccess fileAccess = DefaultFileAccess, FileShare fileShare = DefaultFileShare)
        {
            filePath = filePath ?? throw new ArgumentNullException(nameof(filePath));

            try {
                fileStream = File.Open(filePath, fileMode, fileAccess, fileShare);
                return true;
            } catch (Exception error) when (error.GetType() == typeof(IOException)) {
                fileStream = null;
                return false;
            }
        }

        public static bool WaitUntilAcquired(string filePath, out FileStream? fileStream, FileMode fileMode = DefaultFileMode,
            FileAccess fileAccess = DefaultFileAccess, FileShare fileShare = DefaultFileShare, int timeoutMilliseconds = Timeout.Infinite)
        {
            FileStream spinningFileStream = null;

            var spinHasBeenFinished = SpinWait.SpinUntil(() => {
                return TryAcquire(filePath, out spinningFileStream, fileMode: fileMode, fileAccess: fileAccess, fileShare: fileShare);
            }, timeoutMilliseconds);

            if (spinHasBeenFinished) {
                fileStream = spinningFileStream ?? throw new ArgumentNullException(nameof(spinningFileStream));
                return true;
            } else {
                fileStream = null;
                return false;
            }
        }

        public static bool WaitUntilAcquired(string filePath, out FileStream? fileStream, FileMode fileMode = DefaultFileMode,
            FileAccess fileAccess = DefaultFileAccess, FileShare fileShare = DefaultFileShare, TimeSpan? timeout = null)
        {
            timeout = timeout ?? Timeout.InfiniteTimeSpan;
            var timeoutInMilliseconds = Convert.ToInt32(timeout.Value.TotalMilliseconds);
            return WaitUntilAcquired(filePath, out fileStream, fileMode: fileMode, fileAccess: fileAccess, fileShare: fileShare, timeoutMilliseconds: timeoutInMilliseconds);
        }
    }

@asbjornu
Copy link
Member

@teroneko, that's awesome! Would it be possible to put a lock on the cache file GitVersion already stores within the .git/gitversion_cache/ directory, or on the entire .git/gitversion_cache/ directory itself?

public string GetCacheDirectory()
{
var gitDir = options.Value.DotGitDirectory;
return Path.Combine(gitDir, "gitversion_cache");
}

I'd also like to see a non-static implementation of the LockFileTools where repeating arguments like filePath is being passed once via a public constructor. I'm also not a huge fan of words like Tools, Utility, etc. FileLocker is better, imho. And perhaps if we move all of this code closer in dependence to the GitVersionCache class, it doesn't even need to be public?

@teneko
Copy link
Contributor

teneko commented Jun 10, 2020

It is just the abstraction, and are we really talking about the name? If you want read that below. About your question, it would be somehow possible to lock the folder, but it seems not very straightforward, so we should keep on file locking. Please help me, is it correct, that you associate a cache by meta data from git? Because then you couldn't lock on cache before you didn't got the data from .git with LibGit2Sharp. So a simple .git/gitversion_cache/gitversion.lock would do it too. You have to rely on the fact, that only you are using this directory, so why worry about any behaviour that can interrupt you. Because then you would need to do a lot more to ensure a "atomic" action for access git and cache to prevent any influence from outside.

My view of this is: You always have first an abstraction of something, then you are going to concrete something. In this case you have a generic file lock mechanism, somewhere written now to LockFileTools. You can call it whatever you want like I_Can_Lock_A_File. Tools refers for me a kind of static construction that applies on something. In this case: Lock(ing a )File. As the string is too generic to be an extension, Because of this, I decided to enclose it to the topic Lock(ing a )File. Now based on code, it will will happen to craft a class, for DI, or whatever, to wrap the static functions to reduce arguments of course and increase encapsulation and complexity to serve its actual purpose.
In my opinion is the privatization of code in this project a huge problem. For example I ask myself why you decided to make GitVersionTasks.BuildServiceProvider private. It is the core to make extendibility possible from outside, by providing own implementation of IGitVersionTaskExecutor. For my wondering, no one will ever touch it unless soemone references it by his self:

    <Reference Include="$(PkgGitVersionTask)\tools\netstandard2.0\GitVersionCore.dll">
      <Private>false</Private>
    </Reference>
    <Reference Include="$(PkgGitVersionTask)\tools\netstandard2.0\GitVersionTask.MsBuild.dll">
      <Private>false</Private>
    </Reference>

@asbjornu
Copy link
Member

Of course, you are right. Yes, the cache file requires some information from LibGit2Sharp, specifically the SHA of the HEAD commit.

private string GetRepositorySnapshotHash()
{
using var repo = new Repository(options.Value.GitRootPath);
var head = repo.Head;
if (head.Tip == null)
{
return head.CanonicalName;
}
var hash = string.Join(":", head.CanonicalName, head.Tip.Sha);
return GetHash(hash);
}

Preferably, we would have a .git/gitversion/ folder in which we could create a lock file alongside a cache folder with cache files in it. Changing the name of gitversion_cache could be seen as a breaking change, though, so we should probably not change that until version 6.

My distaste for Tool and Utility stems from an architectural standpoint where such classes often end up being kitchen sink classes that have no real purpos, break all the SOLID principles and becomes impossible to both test and maintain over time. They are like magnets for bad practices. Not that it's impossible to uphold the Single Responsibility Principle in a class called Tool, just that it requires more thought and clear sight by the developer than classes named more succinctly.

In my opinion is the privatization of code in this project a huge problem. For example I ask myself why you decided to make GitVersionTasks.BuildServiceProvider private. It is the core to make extendibility possible from outside, by providing own implementation of IGitVersionTaskExecutor.

Even though we provide a GitVersion.Core assembly, none of the classes exposed by GitVersion are written in a way that promotes external usage. It is all there just to serve GitVersion itself. If someone are able to use GitVersion as a library, that is despite of GitVersion's design, not because of it.

The IOC work laid down by @arturcic is, however, a step in the direction of making GitVersion easier to consume as a library. So it is on the roadmap, although not a high-priority goal. With version 6, I think we might be closer and during the refactorings we are going to do over the lifetime of version 6, I hope we might be in a good place by version 7, where GitVersion should have its own core domain model independent of LibGit2Sharp and where all implementations of everything infrastructure related can be injected through the DI container.

@teneko
Copy link
Contributor

teneko commented Jun 10, 2020

Thanks for your answering. When I find time, I will try to look around the code in GitVersionCode and seek for a possiblity to wrap each single action that involves GitVersion.yml, caching or LibGit2Sharp by an file lock. Another approach would be to lock on startup and unlock on exit. But that is quick and dirty and reminds me of the time, where you instantiated one SqlConnection during lifetime. 😃
Yes would become a breaking change if someone externals is relying on exact these created caches. But not sure if someone is really working with those temporary cach files that are generated by GitVersion and not by someone elses. I won't change any structure.
I think I have now all infos to proceed when I find time. I hope for weekend or next week.
One question, what lets me open, is a test case: It is quite difficult to create such an environment to test it. My project with its 10+ sub projects that relies on GitVersionTask could serve that kind of test, but it does fail only each second or third time due to LibGit2Sharp.LockedFileException.
What is your idea to make a test case? Maybe a kind of penetration test of a huge project with many sub projects someone can provide us?

Regarding to your distate of Tool and Utility. I can understand you and agree with you, that such a Tool whatever static prupose it does serve, can be lost quickly. But a library is not a library without such functions. Those functions cannot be related to only one purpose easily, So when you have two classes that serves completely its own purposes, but have both one method that share a bit of the same logic like Directory.Exists or File.Exists, then you should outsource them definetely to let them become Directory.Exists and File.Exists. My taste for Tools (or Utilities) are coming for this reason, that I don't want to collide with existing classes from System.*, but want to state its relation to them. So the name LockFileTools comes more or less by habit. When I think more over it: FileLocker does associate a kind of encapsulation between these static functions, so even when I would call it FileLocker: The class itself does not provide a file locker, but each of its static functions. Moreover they do not provide a file locker instead they do lock a file it without an instanstiated context so the name would irritate somehow. But this would change if FileLocker would be instantiable and would require a file path and provides methods like "Lock", then the class itself would be a file locker and therefore named FileLocker. Haha, but I do respect your programming style. ☺️

To your last words: I am looking forward that this library will become a strong library. 😊

@asbjornu
Copy link
Member

Thanks for your answering. When I find time, I will try to look around the code in GitVersionCode and seek for a possiblity to wrap each single action that involves GitVersion.yml, caching or LibGit2Sharp by an file lock.

Sounds great! 👍

Another approach would be to lock on startup and unlock on exit. But that is quick and dirty and reminds me of the time, where you instantiated one SqlConnection during lifetime. 😃

It's a viable choice if other options are too difficult. It's much better than crashing and will still yield better performance with parallelization than a serial build would.

Yes would become a breaking change if someone externals is relying on exact these created caches. But not sure if someone is really working with those temporary cach files that are generated by GitVersion and not by someone elses.

I don't know either.

I won't change any structure.

Good. We can instead harmonize this a bit in v6 and gather everything GitVersion related into a .git/gitversion/ folder.

I think I have now all infos to proceed when I find time. I hope for weekend or next week.

Awesome!

What is your idea to make a test case? Maybe a kind of penetration test of a huge project with many sub projects someone can provide us?

I think we should be able to surface this bug with enough projects in a single solution, so a very simple repository with just a bare minimum solution with perhaps 100 projects all using GitVersionTask should be able to consistently reproduce this bug, I'd assume.

I've set up and invited you to GitVersion.TestCases so we can collaborate on the reproduction there. I'm thinking this one repository can have one branch per test case, starting with this parallelization problem.

But this would change if FileLocker would be instantiable and would require a file path and provides methods like "Lock", then the class itself would be a file locker and therefore named FileLocker.

My point exactly! I have a general distaste for static as well, since unless it's always paired with readonly and immutable objects, leads to global mutable state, meaning bugs and bad encapsulation. It's also impossible to mock in tests and generally doesn't play well IOC.

To your last words: I am looking forward that this library will become a strong library. 😊

So do I! 😃

@dmitrynovik
Copy link

Guys, the builds failing because of GitVersion creating *.lock files is a really annoying "feature". Any indication on when the PR "A lock file implementation applied to project GitVersionCore for resolving #1031" can be merged ? Looks like there was a work in progress abandoned in July :(
"

@asbjornu asbjornu mentioned this issue Sep 23, 2020
@MarkusHorstmann
Copy link
Contributor

Pending PR #2581 has some mitigations (retries) for this issue. Makes a big difference at least for my builds.

@github-actions
Copy link

github-actions bot commented Feb 7, 2021

🎉 This issue has been resolved in version 5.6.5 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

MindaugasLaganeckas pushed a commit to MindaugasLaganeckas/GitLabApiClient that referenced this issue Mar 9, 2021
@bording
Copy link
Contributor

bording commented Apr 16, 2021

@arturcic I don't think this issue should be closed. I'm currently using GitVersion.MsBuild 5.6.8 and I'm still seeing LockedFileException and NameConflictException when projects are built in parallel. #1381, which is still open seems to also be relevant here.

The problem is that there is a race condition in the NormalizeGitDirectory where two projects building at the same time will both attempt to create the needed branches, and one will fail with LockedFileException or NameConflictException.

Seems like this code needs to be more resilient to failures, because if it just attempted it again, it would see that the required branch does exist.

EDIT: Instead of a retry, seems like a mutex would be a good way to ensure that the normalization code is not being run in parallel since that's the part that needs to be protected.

@dominikjeske
Copy link

Unfortunately I'm getting this error also using GitVersion.MsBuild 5.6.8 on my TeamCity 2020.1.3.

ERROR [04/19/21 11:53:11:56] An unexpected error occurred:
11:53:11
      LibGit2Sharp.LockedFileException: failed to create locked file 'C:/BuildAgent/work/bb00531d049a7710/.git/refs/heads/pull-requests/716/from.lock': The file exists. 

@bording
Copy link
Contributor

bording commented Apr 21, 2021

I have opened #2669 with my attempt to fix this problem.

@rose-a
Copy link
Contributor

rose-a commented May 26, 2021

Yup, issue is still there with v5.6.9 (GitLab CI)

@bording
Copy link
Contributor

bording commented May 26, 2021

@rose-a I had a follow-up PR #2676 that has not been released yet. That will be needed to truly fix the issue.

@arturcic Any plans to get that out?

@arturcic
Copy link
Member

@bording i will have a look on the PRs in progress and see when we can release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet