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

GitVersionTask Parallel restore issue when multi targeting #1381

Closed
dazinator opened this issue Mar 11, 2018 · 39 comments · Fixed by #2669
Closed

GitVersionTask Parallel restore issue when multi targeting #1381

dazinator opened this issue Mar 11, 2018 · 39 comments · Fixed by #2669
Milestone

Comments

@dazinator
Copy link
Member

dazinator commented Mar 11, 2018

I am noticing this issue in my NETSTANDARD PR branch, but I think it might impact master as well (although not tested master directly I am inferring it is a problem)

The issue is, the following target runs when restoring:

  <Target Name="WriteVersionInfoToBuildLog" BeforeTargets="DispatchToInnerBuilds;GenerateNuspec;_GenerateRestoreProjectSpec" Condition="$(WriteVersionInfoToBuildLog) == 'true'">
    <WriteVersionInfoToBuildLog SolutionDirectory="$(GitVersionPath)" NoFetch="$(GitVersion_NoFetchEnabled)"/>
  </Target>

The WriteVersionInfoToBuildLog msbuild task ends up calling ExecuteGitVersion which ends up using libgit2sharp.

If you are restoring a solution with dotnet restore (I am not sure about /T:Restore but I assume it's the same?) restoration happens in parallel unless you opt out via dotnet restore --disable-parallel switch.

Because of this, I am seeing libgit2sharp throw errors about locking as it's being used in parallel to inspect the same repo:

https://ci.appveyor.com/project/dazinator/gitversion-pr1269/build/0.1.1+5.build.1#L32

@dazinator
Copy link
Member Author

This might be down to an error in my PR branch, I'll reopen once I know for sure it's an issue in master.

@dazinator
Copy link
Member Author

This issue seems only relevant to dotnet build, if you use msbuild /t:build or msbuild /t:Restore its fine.

I beleive this is because dotnet build now implictly does a dotnet restore. And dotnet restore is done in parralel unless you opt out using --disable-parallel.

This causes a problem for our build task, libgit2sharp keeps locks and doesn't like to be used concurrently.

Therefore in my sample repo in order to work around this I have had to use dotnet restore --disable-parallel.

@stale
Copy link

stale bot commented Jun 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 29, 2019
@stale stale bot closed this as completed Jul 29, 2019
@dazinator dazinator reopened this Jul 29, 2019
@stale stale bot removed the stale label Jul 29, 2019
@dazinator
Copy link
Member Author

This should probably result in a docs change - i.e gitversiontask doesn't support building projects in parallel, which is a build switch that should be disabled

@stale
Copy link

stale bot commented Oct 27, 2019

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 27, 2019
@ivan-danilov
Copy link
Contributor

Not supporting parallel build is often a show-stopper for big solutions. Turning parallel build off easily makes 3-minutes build into 15-minutes. Do you take votes on what is the most important? Count mine here ;)

@stale stale bot removed the stale label Nov 13, 2019
@ivan-danilov
Copy link
Contributor

There is actually another related issue: when a repository contains more than one solution and those are built in parallel - GitVersion sometimes fails like this:

WARN [11/12/19 12:36:38:22] Could not determine assembly version: LibGit2Sharp.LockedFileException: the index is locked; this might be due to a concurrent or crashed process
 at LibGit2Sharp.Core.Ensure.HandleError(Int32 result)
 at LibGit2Sharp.Core.Proxy.git_checkout_tree(RepositoryHandle repo, ObjectId treeId, GitCheckoutOpts& opts)
 at LibGit2Sharp.Repository.CheckoutTree(Tree tree, IList`1 paths, IConvertableToGitCheckoutOpts opts)
 at LibGit2Sharp.Commands.Checkout(IRepository repository, Tree tree, CheckoutOptions checkoutOptions, String refLogHeadSpec)
 at LibGit2Sharp.Commands.Checkout(IRepository repository, Branch branch, CheckoutOptions options)
 at LibGit2Sharp.Commands.Checkout(IRepository repository, String committishOrBranchSpec, CheckoutOptions options)
 at GitVersion.GitRepositoryHelper.EnsureLocalBranchExistsForCurrentBranch(Repository repo, Remote remote, String currentBranch)
 at GitVersion.GitRepositoryHelper.NormalizeGitDirectory(String gitDirectory, AuthenticationInfo authentication, Boolean noFetch, String currentBranch)
 at GitVersion.GitPreparer.Initialise(Boolean normaliseGitDirectory, String currentBranch, Boolean shouldCleanUpRemotes)
 at GitVersion.ExecuteCore.ExecuteGitVersion(String targetUrl, String dynamicRepositoryLocation, Authentication authentication, String targetBranch, Boolean noFetch, String workingDirectory, String commitId, Config overrideConfig, Boolean noCache)
 at GitVersion.ExecuteCore.TryGetVersion(String directory, VersionVariables& versionVariables, Boolean noFetch, Authentication authentication)

@dazinator
Copy link
Member Author

My thoughts on a possible solution to this..
This issue seems to be because each gitversionask in the solution/s being built ends up calling gitversions GitPreparer Initialise repository method which is really only something that needs to happen once - as it does a git checkout and makes sure the required branches are present etc.

GitVersion.GitPreparer.Initialise(Boolean normaliseGitDirectory

We could make a change to gitversionask so that it no longer does this prepare logic, and just assumes that the git preparer logic has already been run. Then, at the start of your build process you'd need to run some gitversion command to ensure this this initialisation logic was done first.

I'm not sure it this is the sole cause of the locking issue but it is certainly one of them.

@arturcic
Copy link
Member

arturcic commented Nov 13, 2019

@dazinator, sounds like an interesting solution. I have my own solution. The first run of the task will cache it's results, and the next executions will just use the cache. We need to figure out a mechanism to make sure the task will run once and the others will use the cache

@dazinator
Copy link
Member Author

dazinator commented Nov 13, 2019

@arturcic yeah, I was thinking along those lines to begin with - the trouble is, you have multiple instances of the task (one for each project in a solution) - potentially all running at the same time, and also accross multiple solutions and processes (as msbuild likes to spawn multiple worker processes). Therefore to get this to work, we'd need to implement our own locking mechanism, so that the first task can grab the lock, run, and then all the other tasks wait for the lock to be released - and this lock would need to work accross process boundaries. We could use something like a named Mutex to achieve this (i.e OS level locking) - more on that here: https://stackoverflow.com/questions/46302756/in-dotnet-core-how-can-i-ensure-only-one-copy-of-my-application-is-running

It's a viable solution for sure - but I can see it opening up issues on various platforms if it's not done correctly. Therefore I am thinking it might be better to go with priming the cache at the start of your CI build (as a seperate step) - potentially just by running gitversion (i.e outside the msbuild process) then simplifying the msbuild tasks to just use the cache - (and possibly throw an informative exception if there is no cached value - saying you need to run gitversion in the working directory atleast once?).

@ivan-danilov
Copy link
Contributor

@dazinator priming the cache adds friction to usage versus adding one-time effort for developing, so if there's a way to avoid priming and make sure GitVersion just works - I think it is the best.

About locking - git uses lock file. Can GitVersion do the same? I mean, the process starts, checks in shared read-only mode if the files are up to date; if not - tries to create a lock file (e.g. under .git/gitversion_cache/. If it succeeds - it proceeds with creating another cache file with the updated data and then removes lock file. Otherwise, it starts watching the directory (or polling, but less effective) until the lock file is removed or timeout (like 10s expires). If the timeout expires - it fails with a message that lock file seems to be left unattended, possibly because of the crashed or hang process and asks user to ensure no gitversion is running and remove it.

Of course, it won't work if GitPreparer needs write access to the repository. But I don't see as a user I'd expect GitVersion to do any changes to my repo (at least without an explicit direction/command to do so). After all, I'm trying to figure out the version based on my current state, not change the state.

@arturcic
Copy link
Member

@ivan-danilov that's exactly the way I was thinking it should be implemented, with a lock file and watching for the cache file changes. We need to prototype this idea. Do you mind giving a try?

@jetersen
Copy link
Contributor

jetersen commented Nov 13, 2019

🙏 Would greatly appreciate this feature!
perhaps it should only lock if no cache is available? and otherwise just consume the gitversion_cache ?

@dazinator
Copy link
Member Author

dazinator commented Nov 13, 2019

I had similar thoughts about using a lock file ;-) I agree that asking a user to run gitversion on the repo first, before doing a build does add friction and isnt ideal but it would atleast allow gitversion task to work in parallel builds with probably the least amount of effort.

If someone wants to have a crack at the rolls Royce solution though that would be awesome. The way I saw this solution evolving was something like the following:

  1. Get gitversiontask to use the cache. If no cache, throw an exception saying gitversion needs to be run atleast once first (to populate cache).
  2. Replace the exception throwing! With actual implementation of the logic to consume a lock (via lock file, named mutex or whatever?) Do the fancy stuff (populates cache) then return the lock.
  3. Deal with orphaned locks. Can we detect an orphaned lock? I.e perhaps there could be some max expirey time to a lock file? Or perhaps we just throw an informative exception if waiting for a lock lasts longer than x?should x be configurable? Etc.

@DerAlbertCom
Copy link

We have tons a problems since we updated our buildserver from 1 to 2 core. We get all the Warnings and Errors mentioned here. We trying now some workarounds. But a real solution to that problem with be nice.

@stale
Copy link

stale bot commented Mar 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 17, 2020
@ivan-danilov
Copy link
Contributor

Ping, the issue is still there as far as I'm aware.

@asbjornu asbjornu mentioned this issue Sep 23, 2020
@goldsam
Copy link

goldsam commented Sep 23, 2020

Please greenlight #2410 ASAP please :) This is a blocker for my team.

@DerAlbertCom
Copy link

DerAlbertCom commented Sep 23, 2020

A good Workaround is set the Version on the BuildServer with running GitVersion al as Standalone tool after checking out the source code, so that Environment Variables with the Current Version Numbers will be set for that build.

For Azure DevOps we use.

https://marketplace.visualstudio.com/items?itemName=gittools.gitversion (seems deprecated, works here).

We disabled the AssemblyInfo Version File Update.

For your other build server you may do this manually after the checkout of the git repository in your build script.

Then add this into your root directory.build.props file, if you don't have one create one (in the root of the repository). If you don't know what this is, google it.

  <PropertyGroup Condition=" '$(GitVersion_MajorMinorPatch)' != ''">
    <DisableGitVersionTask>true</DisableGitVersionTask>
    <Version>$(GitVersion_FullSemVer)</Version>
    <FileVersion>$(GitVersion_AssemblySemFileVer)</FileVersion>
    <AssemblyVersion>$(GitVersion_AssemblySemVer)</AssemblyVersion>
    <AssemblyInformationalVersionAttribute>${GitVersion_InformationalVersion}</AssemblyInformationalVersionAttribute>
  </PropertyGroup>

This will be picked up by every project build, and if a GitVersion is set (e.g. the Build Task above) then GitVersion MS Build Task will be disabled and also set the Build Version Properties to the wanted version (your Version Numbers needs may vary).

With that you need no other work around, it just works on the build server, and locally the build task will be run. Also you will notice that the build will be faster with large solution, or tons of branches.

GitVersion runs ONCE with the Build Task from the Marketplace, not multiple times in different, builds, unit tests, publish etc.

And even if this issue will be fixed eventually, there is no need to change this.

@goldsam
Copy link

goldsam commented Sep 30, 2020

My team places a lot of value in doing as much as possible in the csproj/msbuild file because it goes father to ensure the same build result independent of build environment or workflow. We consider versioning to be an integral part of the build process which should be handled consistently through any build workflow such as through Visual Studio or using an automated build script. I appreciate your workaround, but I feel like it falls short of capturing the value added by an MsBuild task.

Perhaps my view is a bit too dogmatic? Getting a proper fix in soon would really be appreciated.

@djonasdev
Copy link

As @DerAlbertCom already said, we do it the same way to avoid FileLockException. In the root of the project we got a Directory.build.props file:

<?xml version="1.0" encoding="utf-8"?>
<Project>
  <PropertyGroup>
    <LangVersion>latest</LangVersion>
    <Authors>foo</Authors>
    <Company>foo</Company>
    <PackageProjectUrl>foo</PackageProjectUrl>
    <RepositoryUrl>foo</RepositoryUrl>
    <RepositoryType>git</RepositoryType>
		<NoWarn>CS0618;CS1591;CS1701;CS8618;CS8632;NU5048;NU5105;NU5125</NoWarn>
  </PropertyGroup>

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

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

@Dunge
Copy link

Dunge commented Nov 25, 2020

+1

Trying to convert my build server (TeamCity) from the Visual Studio toolset (msbuild?) to the dotnet toolset and encountering this lock file exception too, which is kind of a block in the conversion process. Is this not a normal method to use GitVersionTask nuget to stamp git versioning information to the assembly?

This issue is open since nearly 3 years already, what's the hold up?

@dazinator
Copy link
Member Author

dazinator commented Nov 25, 2020

@Dunge hold up from my perspective and I suspect many other contributors, is I have no real incentive to fix this - not that I am the sole maintainer of this repo. I've found workarounds for this issue which I am happy with. It needs someone with an incentive to see this fixed to push it over the line. A lot of people simply don't have time! There has been plenty of discussion for workarounds and even work in progress on this issue to add locking. I prefer to call dotnet build and pass in the version parameters as msbuild arguments, obtained from calling gitversion.exe earlier in the build process at the start of a build. I simply don't use GitVersionTask as I found I don't need it. I find not depending on GitVersionTask as a dependency is cleaner - I can pass in version variables as msbuild arguments at build time.

That said, in an ideal world, GitVersionTask would be fixed. It has a lot of history and originated for legacy .NET framework - such a lot has changed since then, including parallel builds now being enabled by default etc - it's been neglected a bit.

One last note: GitVersionTask code is complicated. It needs to execute on a mutitude of different environments, with potentially different version of MsBuild in play. MsBuild has had breaking changes itself, and there is lots of code (reflection etc) to allow GitVersionTask to run on .NET Core and legacy .NET. That's to say, making changes to it is not trivial. The test suite we have has way more coverage than it used to, but the GitVersionTask coverage wasn't in place last time I checked so it was hard to be sure that changes to code actually worked everywhere.

@Dunge
Copy link

Dunge commented Nov 25, 2020

@dazinator Thank you. Sorry for my somewhat rude comment yesterday (after a long day of configuration I ended up with this issue and wasn't too happy). I was surprised to see such a breaking issue on a nuget with so many daily downloads.

I used the aforementioned Directory.build.props trick and it seems to be working even though I can't understand how having a console step calling GitVersion.exe earlier in the build process would fill in the $(GitVersion_*) variables in the msbuild task. Is that what /updateassemblyinfo true does? The documentation seems to mention updating the AssemblyVersion fields directly in the AssemblyInfo.cs files (which if I'm not mistaken doesn't exist anymore in .NET Core and has been replaced by csproj fields), and does not talk about injecting $(GitVersion_*) variables. Or maybe I was just lucky this time and the lock issue will re-appear randomly in the next builds.

Otherwise I think I can just call gitversion.exe and send the result to dotnet build -p:Version=%something%? But this way I would need to remove the GitVersionTask nuget from my projects, and wouldn't have version on my debug build while running inside Visual Studio, so I would prefer to use the Directory.build.props trick.

@jetersen
Copy link
Contributor

jetersen commented Nov 25, 2020

@Dunge calling GitVersion.exe with gitversion /l console /output buildserver will attempt to detect which CI solution your using through environment variables and inject the gitversion variables into the CI run.

You can the usage here:
https://github.com/nmklotas/GitLabApiClient/blob/cc79852aba4e1cb06154d0da7fcd19259b49181e/.github/workflows/ci.yml#L40
https://github.com/nmklotas/GitLabApiClient/blob/cc79852aba4e1cb06154d0da7fcd19259b49181e/src/GitLabApiClient/GitLabApiClient.csproj#L36-L53

Here you can see what happens in the GitHub Action:
https://github.com/nmklotas/GitLabApiClient/runs/1445124269?check_suite_focus=true#step:7:88
And in the next step you can see the variables injected:
https://github.com/nmklotas/GitLabApiClient/runs/1445124269?check_suite_focus=true#step:8:4

In the new csproj project you can exclude the version field when using GitVersion and delete the AssemblyInfo.cs since MSBuild will generate it based on the values in the csproj file. See the csproj I linked to earlier.

@stale
Copy link

stale bot commented Mar 19, 2021

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs. Thank you for your contributions.

@bording
Copy link
Contributor

bording commented Apr 16, 2021

This is definitely still a problem when using the GitVersion.MsBuild package, including the 5.6.8 package. Yes, there are options to workaround it, but I think that it makes sense to fix things so that the package works without requiring everyone to disable parallel builds or otherwise alter how GitVersion.MsBuild works by default.

@stale stale bot removed the stale label Apr 16, 2021
@SeppPenner
Copy link

I was facing no problems with 5.6.7, but 5.6.8 seems to do some things differently again.

@bording
Copy link
Contributor

bording commented Apr 21, 2021

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

@arturcic arturcic linked a pull request Apr 28, 2021 that will close this issue
5 tasks
@arturcic
Copy link
Member

Closed by #2669

@arturcic arturcic added this to the 5.6.9 milestone Apr 28, 2021
@github-actions
Copy link

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

Your GitReleaseManager bot 📦🚀

@SeppPenner
Copy link

Nice work, I can confirm that the issue is now solved 👍

@arturcic
Copy link
Member

We should thank @bording for the fix

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