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

Weird race condition with targets #321

Closed
gitfool opened this issue Oct 19, 2023 · 2 comments · Fixed by #330
Closed

Weird race condition with targets #321

gitfool opened this issue Oct 19, 2023 · 2 comments · Fixed by #330
Labels

Comments

@gitfool
Copy link
Contributor

gitfool commented Oct 19, 2023

Describe the Bug

I'm hitting a weird race condition with the GitInfo target due to incremental build Inputs and Outputs short-circuiting targets causing their returned properties to sometimes be empty.

Specifically, I'm trying to add a custom target that runs after the GitInfo target to use its properties to derive a base version using the GitCommitDate; however sometimes this property is empty due to being skipped based on incremental build Inputs and Outputs properties.

I've been investigating this weirdness with MSBuild Log Viewer and other properties associated with the GitInfo target are also randomly affected.

Steps to Reproduce

Add the following files to the root directory next to the solution file:

Directory.Build.props:

<Project>
  ...
  <Import Project="Version.targets" />
</Project>

Version.targets:

<?xml version="1.0" encoding="utf-8"?>
<Project>

    <PropertyGroup>
        <GitBranchSlugLength Condition="'$(GitBranchSlugLength)' == ''">0</GitBranchSlugLength>
        <!-- <GitCachePath>$([MSBuild]::MakeRelative($(MSBuildProjectDirectory), $(SolutionDir)))</GitCachePath> -->
        <!-- <GitCommitDateUtcVersion Condition="'$(GitCommitDateUtcVersion)' == ''">true</GitCommitDateUtcVersion> -->
        <GitInfoReportImportance Condition="'$(Configuration)' == 'Debug'">high</GitInfoReportImportance>
        <GitThisAssemblyMetadata Condition="'$(GitThisAssemblyMetadata)' == ''">true</GitThisAssemblyMetadata>
        <GitVersion>false</GitVersion>
    </PropertyGroup>

    <ItemGroup>
        <PackageReference Include="GitInfo" Version="3.3.3" PrivateAssets="all" />
    </ItemGroup>

    <Target Name="_GitBaseVersionCommitDate" Returns="$(GitBaseVersion)"
        AfterTargets="GitInfo"
        Condition="'$(GitBaseVersion)' == '' And '$(GitIgnoreCommitDateVersion)' != 'true'">

        <PropertyGroup Condition="'$(GitCommitDateUtcVersion)' == 'true'">
            <GitBaseVersion>$([System.DateTimeOffset]::Parse("$(GitCommitDate)").ToUniversalTime().ToString("yyyy.Mdd.Hmm"))</GitBaseVersion>
            <GitBaseVersionSource>GitCommitDateUtc</GitBaseVersionSource>
            <GitSemVerSource>CommitDateUtc</GitSemVerSource>
        </PropertyGroup>

        <PropertyGroup Condition="'$(GitCommitDateUtcVersion)' != 'true'">
            <GitBaseVersion>$([System.DateTimeOffset]::Parse("$(GitCommitDate)").ToString("yyyy.Mdd.Hmm"))</GitBaseVersion>
            <GitBaseVersionSource>GitCommitDate</GitBaseVersionSource>
            <GitSemVerSource>CommitDate</GitSemVerSource>
        </PropertyGroup>

        <Exec Command="$(GitExe) rev-list --count --first-parent $(GitDefaultBranch)..HEAD"
            EchoOff="true"
            StandardErrorImportance="low"
            StandardOutputImportance="low"
            ConsoleToMSBuild="true"
            WorkingDirectory="$(GitRoot)"
            ContinueOnError="true"
            StdOutEncoding="utf-8">
            <Output TaskParameter="ConsoleOutput" PropertyName="GitCommits" />
            <Output TaskParameter="ExitCode" PropertyName="MSBuildLastExitCode" />
        </Exec>

        <PropertyGroup Condition="'$(MSBuildLastExitCode)' != '0'">
            <GitCommits>0</GitCommits>
        </PropertyGroup>

    </Target>

    <Target Name="_GitSetVersion"
        AfterTargets="GitVersion"
        Condition="'$(GitVersion)' == 'false'">

        <PropertyGroup>
            <_BranchSlug>$(GitBranch.ToLowerInvariant())</_BranchSlug>
            <_BranchSlug Condition="$(GitBranchSlugLength) > 0 And $(_BranchSlug.Length) > $(GitBranchSlugLength)">$(_BranchSlug.Substring(0, $(GitBranchSlugLength)))</_BranchSlug>
            <_BranchSlug>$([System.Text.RegularExpressions.Regex]::Replace($(_BranchSlug), "[^0-9a-z-]", "-").Trim('-'))</_BranchSlug>
            <Version>$(GitBaseVersionMajor).0.0.0</Version>
            <FileVersion>$(GitBaseVersionMajor).$(GitBaseVersionMinor).$(GitBaseVersionPatch).0</FileVersion>
            <InformationalVersion Condition="'$(GitBranch)' == '$(GitDefaultBranch)'">$(GitBaseVersionMajor).$(GitBaseVersionMinor).$(GitBaseVersionPatch)+$(_BranchSlug).$(GitSha)</InformationalVersion>
            <InformationalVersion Condition="'$(GitBranch)' != '$(GitDefaultBranch)'">$(GitBaseVersionMajor).$(GitBaseVersionMinor).$(GitBaseVersionPatch)-$(_BranchSlug).$(GitCommits)</InformationalVersion>
            <PackageVersion Condition="'$(GitBranch)' == '$(GitDefaultBranch)'">$(GitBaseVersionMajor).$(GitBaseVersionMinor).$(GitBaseVersionPatch)</PackageVersion>
            <PackageVersion Condition="'$(GitBranch)' != '$(GitDefaultBranch)'">$(GitBaseVersionMajor).$(GitBaseVersionMinor).$(GitBaseVersionPatch)-$(_BranchSlug).$(GitCommits)</PackageVersion>
        </PropertyGroup>

    </Target>

</Project>

In my scenario I have a solution with only 4 projects but they all want to generate the same version - which highlights another issue that the git info cache default directory should probably be common to all projects; either in the .git directory (preferred) like GitVersion or next to the solution file. Is there any reason this is not the default?

Anyway, if I repeatedly rebuild the solution it eventually fails, usually within the first 10 attempts, with the following:

error MSB4184: The expression "[System.DateTimeOffset]::Parse('')" cannot be evaluated. String '' was not recognized as a valid DateTime.

Expected Behavior

Always succeed rather than randomly fail.

Version Info

3.3.3

Additional Info

MSBuild Log Viewer screenshot (redacted):

image

Shows that after the GitInfo target was run, several of its properties, including GitCommitDate where I expand its target _GitCommitDate, were not populated due to the incremental build skipping them:

Skipping target ... because all output files are up-to-date with respect to the input files.

Looking around this repo's source it seems to me that the targets that populate the GitInfo properties do not need to leverage the incremental build properties (Inputs and Outputs) since they are also short circuited by their conditions when the cache is hit and the values can be read from the previous build.

Am I doing something wrong or missing something?

PS. I'm happy to submit a pull request if we can understand the issue and decide how best to fix it. Along the same lines, would you accept a pull request to add this "git commit date" based versioning strategy once I get it working robustly? 🙏

@gitfool gitfool added the bug label Oct 19, 2023
@gitfool
Copy link
Contributor Author

gitfool commented Oct 25, 2023

See https://github.com/gitfool/GitInfo.Dungeon for a repro. I injected some sleeps after _GitReadCache for a slightly more deterministic build that flushes out the race condition between _GitReadCache and _GitWriteCache, when using root level cache files.

It doesn't want to repro within 10 test builds on github but it always fails within 10 runs on my local machine:

❯ .\test-build.ps1

**************************************************
Build #1
VERBOSE: Performing the operation "Remove File" on target "D:\Devel\gitfool\GitInfo.Dungeon\.GitHead.cache".
VERBOSE: Performing the operation "Remove File" on target "D:\Devel\gitfool\GitInfo.Dungeon\.GitInfo.cache".
VERBOSE: Performing the operation "Remove File" on target "D:\Devel\gitfool\GitInfo.Dungeon\.GitIsDirty.cache".
Shutting down MSBuild server...
Shutting down VB/C# compiler server...
VB/C# compiler server shut down successfully.
MSBuild server shut down successfully.
Tool 'cake.tool' (version '3.1.0') was restored. Available commands: dotnet-cake

Restore was successful.
Analyzing build script...
Processing build script...
Installing tools...
Tool dotnet-reportgenerator-globaltool is already installed, with required version.
Tool GitVersion.Tool is already installed, with required version.
Installing addins...
Running cached build script...
Executing: "D:/Devel/gitfool/GitInfo.Dungeon/tools/dotnet-gitversion.exe" -output json -verbosity Verbose

========================================
Info
========================================
Executing task: Info
Version 0.1.0+4.Branch.main.Sha.bcfdad60b162eb2a67b31377cfc7c902b818e609
Finished executing task: Info
Completed in 00:00:00.0308242

========================================
BuildSolutions
========================================
Executing task: BuildSolutions
Cleaning directory D:/Devel/gitfool/GitInfo.Dungeon/Cli/bin
Cleaning directory D:/Devel/gitfool/GitInfo.Dungeon/Library/bin
Cleaning directory D:/Devel/gitfool/GitInfo.Dungeon/Tests/bin
Cleaning directory D:/Devel/gitfool/GitInfo.Dungeon/Cli/obj
Cleaning directory D:/Devel/gitfool/GitInfo.Dungeon/Library/obj
Cleaning directory D:/Devel/gitfool/GitInfo.Dungeon/Tests/obj
Executing: "C:/Program Files/dotnet/dotnet.exe" build "D:/Devel/gitfool/GitInfo.Dungeon/GitInfo.Dungeon.sln" --configuration Release /property:Version=0.0.0.0 /property:FileVersion=0.1.0.0 /property:InformationalVersion=0.1.0+4.Branch.main.Sha.bcfdad60b162eb2a67b31377cfc7c902b818e609 /property:PackageVersion=0.1.0 /property:EmbedAllSources=true /property:RestoreLockedMode=false /property:ContinuousIntegrationBuild=false
MSBuild version 17.8.0+6cdef4241 for .NET
  Determining projects to restore...
  Restored D:\Devel\gitfool\GitInfo.Dungeon\Cli\Cli.csproj (in 244 ms).
  Restored D:\Devel\gitfool\GitInfo.Dungeon\Library\Library.csproj (in 244 ms).
  Restored D:\Devel\gitfool\GitInfo.Dungeon\Tests\Tests.csproj (in 449 ms).
  Sleeping 1ms...
  Sleeping 1ms...
  Sleeping 1000ms...
D:\Devel\gitfool\GitInfo.Dungeon\Version.targets(15,13): error MSB4184: The expression "[System.DateTimeOffset]::Parse('')" cannot be evaluated. String '' was not recognized as a valid DateTime. [D:\Devel\gitfool\GitInfo.Dungeon\Tests\Tests.csproj]
  Library -> D:\Devel\gitfool\GitInfo.Dungeon\Library\bin\Release\net7.0\Library.dll
  Cli -> D:\Devel\gitfool\GitInfo.Dungeon\Cli\bin\Release\net7.0\Cli.dll

Build FAILED.

D:\Devel\gitfool\GitInfo.Dungeon\Version.targets(15,13): error MSB4184: The expression "[System.DateTimeOffset]::Parse('')" cannot be evaluated. String '' was not recognized as a valid DateTime. [D:\Devel\gitfool\GitInfo.Dungeon\Tests\Tests.csproj]
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:05.08
An error occurred when executing task 'BuildSolutions'.
Completed in 00:00:05.3824110
Error: .NET CLI: Process returned an error (exit code 1).

@gitfool
Copy link
Contributor Author

gitfool commented Oct 25, 2023

I tweaked the sleeps and after a few runs via MSBuild Log Viewer I captured the following binlog: GitInfo.Dungeon.binlog.zip

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

Successfully merging a pull request may close this issue.

1 participant