Add graceful error handling to Git branch fetch

[dagood]

If the current directory isn't a Git repo (such as during source-build's tarball build), this code ends up putting the Git output's newlines in a C++ string, which gives CoreCLR compile errors:

buildtools/src/Microsoft.DotNet.Build.Tasks/PackageFiles/versioning.targets

Lines 266 to 269 in 2f0dec8

	<!-- ############################### -->
	<Exec Command="git describe --all HEAD 2>&amp;1" StandardOutputImportance="Low" IgnoreExitCode="true" IgnoreStandardErrorWarningFormat="true" ConsoleToMSBuild="true" Condition="'$(RawGitBranchName)' == ''">
	<Output TaskParameter="ConsoleOutput" PropertyName="RawGitBranchName" />
	</Exec>

It should handle error exit codes like it does when getting the commit:

buildtools/src/Microsoft.DotNet.Build.Tasks/PackageFiles/versioning.targets

Lines 271 to 285 in 2f0dec8

	<!-- ############################### -->
	<PropertyGroup Condition="'$(LatestCommit)' != ''">
	<LatestCommitExitCode>0</LatestCommitExitCode>
	</PropertyGroup>
	<!-- Get the latest commit hash -->
	<Exec Command="git rev-parse HEAD 2>&amp;1" StandardOutputImportance="Low" IgnoreExitCode="true" IgnoreStandardErrorWarningFormat="true" ConsoleToMSBuild="true" Condition="'$(LatestCommit)' == ''">
	<Output TaskParameter="ConsoleOutput" PropertyName="LatestCommit" />
	<Output TaskParameter="ExitCode" PropertyName="LatestCommitExitCode" />
	</Exec>
	<!-- We shouldn't fail the build if we can't retreive the commit hash, so in this case just set it to N/A -->
	<PropertyGroup Condition="'$(LatestCommitExitCode)'!='0'">
	<LatestCommit>N/A</LatestCommit>
	</PropertyGroup>

This will also let us pass in the branch name from source-build if we know it.

[weshaggard]

As part of this fix we should change the message importance from high to normal. see 2f0dec8#r28341038

[vancem]

I don't understand why the original code was causing problems. The reason for the 'RawGitBranchName was that I only use it as a branch name if it looks like output I expect (namely if it begins with a 'heads' or 'remotes/origin/'

    <!-- If things match expected patterns, create $(GitBranchName) from $(RawGitBranchName) -->
    <Message Importance="high" Text="GIT Branch Name as seen by git describe: '$(RawGitBranchName)'" /> 
    <PropertyGroup Condition="$(RawGitBranchName.StartsWith('heads/'))">
      <GitBranchName>$(RawGitBranchName.SubString(6))</GitBranchName>
    </PropertyGroup>
    <PropertyGroup Condition="$(RawGitBranchName.StartsWith('remotes/origin/'))">
      <GitBranchName>$(RawGitBranchName.SubString(15))</GitBranchName>
    </PropertyGroup>
    <Message Importance="high" Text="GIT Branch Name: '$(GitBranchName)'" /> 

On errors, I would have expected it to not machine, and thus leave the GitBranchName empty (which means it should not be shown.

Presumably this did not happen, but I don't understand why my logic above is wrong...

[dagood]

I think you're actually right, and I misread the code... I hit this problem before your changes when the git command was getting GitBranchName directly, and I was thinking of the matching patterns you added as "extra" ways to try to get a meaningful branch name, rather than filters.

I tried a source-build tarball build of CoreCLR with a BuildTools that contains your change (and doesn't contain my workaround) and it works fine. Sorry about that. 😄

If there are any error messages or multiline responses that can start with heads/ or remotes/origin/, there's still a problem, but I don't think that's the case.

Closing as (already) fixed.