From 721a338b5a8821ca7f0bb58b5a630e0f76d0ae66 Mon Sep 17 00:00:00 2001 From: Thomas Piskol <38790465+ThomasPiskol@users.noreply.github.com> Date: Sun, 6 Mar 2022 15:35:23 +0100 Subject: [PATCH 1/3] Use GITHUB_REF only for CI builds on branches --- .../BuildAgents/GitHubActionsTests.cs | 4 ++-- src/GitVersion.Core/BuildAgents/GitHubActions.cs | 13 ++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/GitVersion.Core.Tests/BuildAgents/GitHubActionsTests.cs b/src/GitVersion.Core.Tests/BuildAgents/GitHubActionsTests.cs index f0df2757eb..6668fed502 100644 --- a/src/GitVersion.Core.Tests/BuildAgents/GitHubActionsTests.cs +++ b/src/GitVersion.Core.Tests/BuildAgents/GitHubActionsTests.cs @@ -83,7 +83,7 @@ public void GetCurrentBranchShouldHandleTags() var result = this.buildServer.GetCurrentBranch(false); // Assert - result.ShouldBe("refs/tags/1.0.0"); + result.ShouldBeNull(); } [Test] @@ -96,7 +96,7 @@ public void GetCurrentBranchShouldHandlePullRequests() var result = this.buildServer.GetCurrentBranch(false); // Assert - result.ShouldBe("refs/pull/1/merge"); + result.ShouldBeNull(); } [Test] diff --git a/src/GitVersion.Core/BuildAgents/GitHubActions.cs b/src/GitVersion.Core/BuildAgents/GitHubActions.cs index f0a3fd2364..2e12b873cf 100644 --- a/src/GitVersion.Core/BuildAgents/GitHubActions.cs +++ b/src/GitVersion.Core/BuildAgents/GitHubActions.cs @@ -50,7 +50,18 @@ public override void WriteIntegration(Action writer, VersionVariables v } } - public override string? GetCurrentBranch(bool usingDynamicRepos) => Environment.GetEnvironmentVariable("GITHUB_REF"); + public override string? GetCurrentBranch(bool usingDynamicRepos) + { + // https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables + // GITHUB_REF must be used only for "real" branches, not for tags and pull requests. + // Bug fix for https://github.com/GitTools/GitVersion/issues/2838 + string? githubRef = Environment.GetEnvironmentVariable("GITHUB_REF"); + if (githubRef != null && githubRef.StartsWith("refs/heads/")) + { + return githubRef; + } + return null; + } public override bool PreventFetch() => true; } From 51b884a0afce503237435a66f362770af5d9cc9d Mon Sep 17 00:00:00 2001 From: Thomas Piskol <38790465+ThomasPiskol@users.noreply.github.com> Date: Sun, 20 Mar 2022 20:04:39 +0100 Subject: [PATCH 2/3] align BuildAgentBase implementations --- .../BuildAgents/AzurePipelinesTests.cs | 39 +++++++++++++++++++ .../BuildAgents/BuildKiteTests.cs | 2 +- .../BuildAgents/CodeBuildTests.cs | 32 ++++++++++++++- .../BuildAgents/SpaceAutomationTests.cs | 7 ++-- .../BuildAgents/AzurePipelines.cs | 12 +++++- src/GitVersion.Core/BuildAgents/BuildKite.cs | 6 +-- src/GitVersion.Core/BuildAgents/CodeBuild.cs | 16 +++++--- .../BuildAgents/SpaceAutomation.cs | 10 ++++- 8 files changed, 107 insertions(+), 17 deletions(-) diff --git a/src/GitVersion.Core.Tests/BuildAgents/AzurePipelinesTests.cs b/src/GitVersion.Core.Tests/BuildAgents/AzurePipelinesTests.cs index d14d9ec83c..0845c862d5 100644 --- a/src/GitVersion.Core.Tests/BuildAgents/AzurePipelinesTests.cs +++ b/src/GitVersion.Core.Tests/BuildAgents/AzurePipelinesTests.cs @@ -80,4 +80,43 @@ public void AzurePipelinesBuildNumberWithSemVer(string buildNumberFormat, string var logMessage = this.buildServer.GenerateSetVersionMessage(vars); logMessage.ShouldBe(logPrefix + expectedBuildNumber); } + + [Test] + public void GetCurrentBranchShouldHandleBranches() + { + // Arrange + this.environment.SetEnvironmentVariable("BUILD_SOURCEBRANCH", $"refs/heads/{MainBranch}"); + + // Act + var result = this.buildServer.GetCurrentBranch(false); + + // Assert + result.ShouldBe($"refs/heads/{MainBranch}"); + } + + [Test] + public void GetCurrentBranchShouldHandleTags() + { + // Arrange + this.environment.SetEnvironmentVariable("BUILD_SOURCEBRANCH", "refs/tags/1.0.0"); + + // Act + var result = this.buildServer.GetCurrentBranch(false); + + // Assert + result.ShouldBeNull(); + } + + [Test] + public void GetCurrentBranchShouldHandlePullRequests() + { + // Arrange + this.environment.SetEnvironmentVariable("BUILD_SOURCEBRANCH", "refs/pull/1/merge"); + + // Act + var result = this.buildServer.GetCurrentBranch(false); + + // Assert + result.ShouldBeNull(); + } } diff --git a/src/GitVersion.Core.Tests/BuildAgents/BuildKiteTests.cs b/src/GitVersion.Core.Tests/BuildAgents/BuildKiteTests.cs index 4bbd4541fb..a07fb28a68 100644 --- a/src/GitVersion.Core.Tests/BuildAgents/BuildKiteTests.cs +++ b/src/GitVersion.Core.Tests/BuildAgents/BuildKiteTests.cs @@ -72,7 +72,7 @@ public void GetCurrentBranchShouldHandlePullRequests() var result = this.buildServer.GetCurrentBranch(false); // Assert - result.ShouldBe("refs/pull/55/head"); + result.ShouldBeNull(); } [Test] diff --git a/src/GitVersion.Core.Tests/BuildAgents/CodeBuildTests.cs b/src/GitVersion.Core.Tests/BuildAgents/CodeBuildTests.cs index 649d6c5b18..6f0406bef5 100644 --- a/src/GitVersion.Core.Tests/BuildAgents/CodeBuildTests.cs +++ b/src/GitVersion.Core.Tests/BuildAgents/CodeBuildTests.cs @@ -1,5 +1,6 @@ using GitVersion.BuildAgents; using GitVersion.Core.Tests.Helpers; +using GitVersion.Helpers; using GitVersion.VersionCalculation; using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; @@ -54,7 +55,8 @@ public void PicksUpBranchNameFromEnvironmentFromWebHook() public void WriteAllVariablesToTheTextWriter() { var assemblyLocation = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); - var f = Path.Combine(assemblyLocation, "codebuild_this_file_should_be_deleted.properties"); + assemblyLocation.ShouldNotBeNull(); + var f = PathHelper.Combine(assemblyLocation, "codebuild_this_file_should_be_deleted.properties"); try { @@ -68,7 +70,7 @@ public void WriteAllVariablesToTheTextWriter() private void AssertVariablesAreWrittenToFile(string file) { - var writes = new List(); + var writes = new List(); var semanticVersion = new SemanticVersion { Major = 1, @@ -100,4 +102,30 @@ private void AssertVariablesAreWrittenToFile(string file) props.ShouldContain("GitVersion_Major=1"); props.ShouldContain("GitVersion_Minor=2"); } + + [Test] + public void GetCurrentBranchShouldHandleTags() + { + // Arrange + this.environment.SetEnvironmentVariable("CODEBUILD_WEBHOOK_HEAD_REF", "refs/tags/1.0.0"); + + // Act + var result = this.buildServer.GetCurrentBranch(false); + + // Assert + result.ShouldBeNull(); + } + + [Test] + public void GetCurrentBranchShouldHandlePullRequests() + { + // Arrange + this.environment.SetEnvironmentVariable("CODEBUILD_SOURCE_VERSION", "refs/pull/1/merge"); + + // Act + var result = this.buildServer.GetCurrentBranch(false); + + // Assert + result.ShouldBeNull(); + } } diff --git a/src/GitVersion.Core.Tests/BuildAgents/SpaceAutomationTests.cs b/src/GitVersion.Core.Tests/BuildAgents/SpaceAutomationTests.cs index f4160a9154..938d7834f2 100644 --- a/src/GitVersion.Core.Tests/BuildAgents/SpaceAutomationTests.cs +++ b/src/GitVersion.Core.Tests/BuildAgents/SpaceAutomationTests.cs @@ -1,11 +1,10 @@ -using GitVersion; using GitVersion.BuildAgents; using GitVersion.Core.Tests.Helpers; using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Shouldly; -namespace GitVersionCore.Tests.BuildAgents; +namespace GitVersion.Core.Tests.BuildAgents; [TestFixture] public class SpaceAutomationTests : TestBase @@ -71,7 +70,7 @@ public void GetCurrentBranchShouldHandleTags() var result = this.buildServer.GetCurrentBranch(false); // Assert - result.ShouldBe("refs/tags/1.0.0"); + result.ShouldBeNull(); } [Test] @@ -84,7 +83,7 @@ public void GetCurrentBranchShouldHandlePullRequests() var result = this.buildServer.GetCurrentBranch(false); // Assert - result.ShouldBe("refs/pull/1/merge"); + result.ShouldBeNull(); } [Test] diff --git a/src/GitVersion.Core/BuildAgents/AzurePipelines.cs b/src/GitVersion.Core/BuildAgents/AzurePipelines.cs index 30e08ee059..ebb223479d 100644 --- a/src/GitVersion.Core/BuildAgents/AzurePipelines.cs +++ b/src/GitVersion.Core/BuildAgents/AzurePipelines.cs @@ -21,7 +21,17 @@ public override string[] GenerateSetParameterMessage(string name, string value) $"##vso[task.setvariable variable=GitVersion.{name};isOutput=true]{value}" }; - public override string? GetCurrentBranch(bool usingDynamicRepos) => Environment.GetEnvironmentVariable("BUILD_SOURCEBRANCH"); + public override string? GetCurrentBranch(bool usingDynamicRepos) + { + // https://docs.microsoft.com/en-us/azure/devops/pipelines/build/variables + // BUILD_SOURCEBRANCH does not contain the branch name if the build was triggered by a tag or pull request. + string? branchName = Environment.GetEnvironmentVariable("BUILD_SOURCEBRANCH"); + if (branchName != null && branchName.StartsWith("refs/heads/")) + { + return branchName; + } + return null; + } public override bool PreventFetch() => true; diff --git a/src/GitVersion.Core/BuildAgents/BuildKite.cs b/src/GitVersion.Core/BuildAgents/BuildKite.cs index f54fb2a1ca..d6cff4b4d4 100644 --- a/src/GitVersion.Core/BuildAgents/BuildKite.cs +++ b/src/GitVersion.Core/BuildAgents/BuildKite.cs @@ -30,9 +30,9 @@ public override string[] GenerateSetParameterMessage(string name, string value) } else { - // For pull requests BUILDKITE_BRANCH refers to the head, so adjust the - // branch name for pull request versioning to function as expected - return string.Format("refs/pull/{0}/head", pullRequest); + // To align the behavior with the other BuildAgent implementations + // we return here also null. + return null; } } diff --git a/src/GitVersion.Core/BuildAgents/CodeBuild.cs b/src/GitVersion.Core/BuildAgents/CodeBuild.cs index a1d5301c48..1c1fbc7f99 100644 --- a/src/GitVersion.Core/BuildAgents/CodeBuild.cs +++ b/src/GitVersion.Core/BuildAgents/CodeBuild.cs @@ -25,18 +25,24 @@ public override string[] GenerateSetParameterMessage(string name, string value) public override string? GetCurrentBranch(bool usingDynamicRepos) { - var currentBranch = Environment.GetEnvironmentVariable(WebHookEnvironmentVariableName); - - if (currentBranch.IsNullOrEmpty()) + string? branchName = Environment.GetEnvironmentVariable(WebHookEnvironmentVariableName); + if (string.IsNullOrEmpty(branchName)) { - return Environment.GetEnvironmentVariable(SourceVersionEnvironmentVariableName); + branchName = Environment.GetEnvironmentVariable(SourceVersionEnvironmentVariableName); } - return currentBranch; + if (branchName != null && branchName.StartsWith("refs/heads/")) + { + return branchName; + } + return null; } public override void WriteIntegration(Action writer, VersionVariables variables, bool updateBuildNumber = true) { + if (this.file is null) + return; + base.WriteIntegration(writer, variables, updateBuildNumber); writer($"Outputting variables to '{this.file}' ... "); File.WriteAllLines(this.file, GenerateBuildLogOutput(variables)); diff --git a/src/GitVersion.Core/BuildAgents/SpaceAutomation.cs b/src/GitVersion.Core/BuildAgents/SpaceAutomation.cs index 43be822a07..bda08aaf28 100644 --- a/src/GitVersion.Core/BuildAgents/SpaceAutomation.cs +++ b/src/GitVersion.Core/BuildAgents/SpaceAutomation.cs @@ -13,7 +13,15 @@ public SpaceAutomation(IEnvironment environment, ILog log) : base(environment, l protected override string EnvironmentVariable => EnvironmentVariableName; - public override string? GetCurrentBranch(bool usingDynamicRepos) => Environment.GetEnvironmentVariable("JB_SPACE_GIT_BRANCH"); + public override string? GetCurrentBranch(bool usingDynamicRepos) + { + string? branchName = Environment.GetEnvironmentVariable("JB_SPACE_GIT_BRANCH"); + if (branchName != null && branchName.StartsWith("refs/heads/")) + { + return branchName; + } + return null; + } public override string[] GenerateSetParameterMessage(string name, string value) => Array.Empty(); From d4aa1db8d6c3427c52a10544e782bde17f846b1f Mon Sep 17 00:00:00 2001 From: Thomas Piskol <38790465+ThomasPiskol@users.noreply.github.com> Date: Sun, 20 Mar 2022 20:19:29 +0100 Subject: [PATCH 3/3] fixes #2928 --- src/GitVersion.Core/BuildAgents/GitLabCi.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/GitVersion.Core/BuildAgents/GitLabCi.cs b/src/GitVersion.Core/BuildAgents/GitLabCi.cs index 85915b2d30..3cf737974f 100644 --- a/src/GitVersion.Core/BuildAgents/GitLabCi.cs +++ b/src/GitVersion.Core/BuildAgents/GitLabCi.cs @@ -22,7 +22,9 @@ public override string[] GenerateSetParameterMessage(string name, string value) $"GitVersion_{name}={value}" }; - public override string? GetCurrentBranch(bool usingDynamicRepos) => Environment.GetEnvironmentVariable("CI_COMMIT_REF_NAME"); + // According to https://docs.gitlab.com/ee/ci/variables/predefined_variables.html + // the CI_COMMIT_BRANCH environment variable must be used. + public override string? GetCurrentBranch(bool usingDynamicRepos) => Environment.GetEnvironmentVariable("CI_COMMIT_BRANCH"); public override bool PreventFetch() => true;