Skip to content

Commit f2c641c

Browse files
MattKotsenasMattKotsenas
authored
Align RepositoryBranch logic with .NET 9 (#50)
Fixes #46 Addresses the difference in behavior between the .NET 9 SDK's version of SourceLink and our own branch logic. Doing so has two implications: 1. We no longer attempt to shorten git refs like `refs/heads/` or `refs/tags/` 2. We prefer the tag over the branch over the PR ID (old logic was PR ID, tag, branch) For code reviewers I suggest reviewing each commit separately, as the first moves the test data and covers all the combinations of values, and the second is the logic and test result change
1 parent 94faeb1 commit f2c641c

File tree

6 files changed

+106
-38
lines changed

6 files changed

+106
-38
lines changed

‎src/DotNet.ReproducibleBuilds/DotNet.ReproducibleBuilds.targets‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,32 +15,31 @@
1515

1616
<PropertyGroup Condition="'$(RepositoryBranch)' == '' and '$(PublishRepositoryUrl)' == 'true'">
1717
<!-- GitHub Actions: https://docs.github.com/en/actions/reference/environment-variables#default-environment-variables -->
18-
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(GITHUB_REF)' != '' and $(GITHUB_REF.Contains('refs/pull/'))">pr$(GITHUB_REF.Replace('refs/pull/', '').Replace('/merge', ''))</RepositoryBranch>
19-
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(GITHUB_REF)' != ''">$(GITHUB_REF.Replace('refs/heads/', '').Replace('refs/tags/', ''))</RepositoryBranch>
18+
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(GITHUB_REF)' != ''">$(GITHUB_REF)</RepositoryBranch>
2019
<!-- Azure DevOps: https://docs.microsoft.com/en-us/azure/devops/pipelines/build/variables -->
21-
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(BUILD_SOURCEBRANCH)' != ''">$(BUILD_SOURCEBRANCH.Replace('refs/heads/', '').Replace('refs/tags/', ''))</RepositoryBranch>
20+
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(BUILD_SOURCEBRANCH)' != ''">$(BUILD_SOURCEBRANCH)</RepositoryBranch>
2221
<!-- AppVeyor: https://www.appveyor.com/docs/environment-variables/ -->
2322
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(APPVEYOR_PULL_REQUEST_NUMBER)' != ''">pr$(APPVEYOR_PULL_REQUEST_NUMBER)</RepositoryBranch>
2423
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(APPVEYOR_REPO_TAG_NAME)' != ''">$(APPVEYOR_REPO_TAG_NAME)</RepositoryBranch>
2524
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(APPVEYOR_REPO_BRANCH)' != ''">$(APPVEYOR_REPO_BRANCH)</RepositoryBranch>
2625
<!-- TeamCity: https://www.jetbrains.com/help/teamcity/predefined-build-parameters.html#Branch-Related+Parameters -->
2726
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(TEAMCITY_BUILD_BRANCH)' != ''">$(TEAMCITY_BUILD_BRANCH)</RepositoryBranch>
2827
<!--TravisCI: https://docs.travis-ci.com/user/environment-variables/ -->
29-
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(TRAVIS_PULL_REQUEST)' != '' and '$(TRAVIS_PULL_REQUEST)' != 'false'">pr$(TRAVIS_PULL_REQUEST)</RepositoryBranch>
3028
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(TRAVIS_BRANCH)' != ''">$(TRAVIS_BRANCH)</RepositoryBranch>
29+
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(TRAVIS_PULL_REQUEST)' != '' and '$(TRAVIS_PULL_REQUEST)' != 'false'">pr$(TRAVIS_PULL_REQUEST)</RepositoryBranch>
3130
<!-- CircleCI: https://circleci.com/docs/2.0/env-vars/ -->
32-
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CIRCLE_PR_NUMBER)' != ''">pr$(CIRCLE_PR_NUMBER)</RepositoryBranch>
3331
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CIRCLE_TAG)' != ''">$(CIRCLE_TAG)</RepositoryBranch>
3432
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CIRCLE_BRANCH)' != ''">$(CIRCLE_BRANCH)</RepositoryBranch>
33+
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CIRCLE_PR_NUMBER)' != ''">pr$(CIRCLE_PR_NUMBER)</RepositoryBranch>
3534
<!-- GitLab: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html -->
3635
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CI_COMMIT_TAG)' != ''">$(CI_COMMIT_TAG)</RepositoryBranch>
36+
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CI_COMMIT_BRANCH)' != ''">$(CI_COMMIT_BRANCH)</RepositoryBranch>
3737
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CI_MERGE_REQUEST_IID)' != ''">pr$(CI_MERGE_REQUEST_IID)</RepositoryBranch>
3838
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CI_EXTERNAL_PULL_REQUEST_IID)' != ''">pr$(CI_EXTERNAL_PULL_REQUEST_IID)</RepositoryBranch>
39-
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(CI_COMMIT_BRANCH)' != ''">$(CI_COMMIT_BRANCH)</RepositoryBranch>
4039
<!-- Buddy: https://buddy.works/docs/pipelines/environment-variables#default-environment-variables -->
41-
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(BUDDY_EXECUTION_PULL_REQUEST_NO)' != ''">pr$(BUDDY_EXECUTION_PULL_REQUEST_NO)</RepositoryBranch>
4240
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(BUDDY_EXECUTION_TAG)' != ''">$(BUDDY_EXECUTION_TAG)</RepositoryBranch>
4341
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(BUDDY_EXECUTION_BRANCH)' != ''">$(BUDDY_EXECUTION_BRANCH)</RepositoryBranch>
42+
<RepositoryBranch Condition="'$(RepositoryBranch)' == '' and '$(BUDDY_EXECUTION_PULL_REQUEST_NO)' != ''">pr$(BUDDY_EXECUTION_PULL_REQUEST_NO)</RepositoryBranch>
4443
</PropertyGroup>
4544

4645
<PropertyGroup>

‎tests/DotNet.ReproducibleBuilds.Tests/CollectionExtensions.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
namespace DotNet.ReproducibleBuilds.Tests;
2+
3+
internal static class CollectionExtensions
4+
{
5+
public static IDisposable ToDisposable(this IEnumerable<IDisposable> disposables) => new DisposableCollection(disposables);
6+
}

‎tests/DotNet.ReproducibleBuilds.Tests/ContinuousIntegrationTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void RespectsSetValue(bool? value, string expected)
2626

2727
[Theory]
2828
[MemberData(nameof(MemberData))]
29-
public void RespectsGlobalProperites(Dictionary<string, string> envVars)
29+
public void RespectsGlobalProperties(Dictionary<string, string> envVars)
3030
{
3131
using EnvironmentVariableSuppressor hostSuppressor = new("TF_BUILD"); // Suppress our own CI provider variables (i.e. Azure DevOps)
3232

‎tests/DotNet.ReproducibleBuilds.Tests/DisposableCollection.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
namespace DotNet.ReproducibleBuilds.Tests;
2+
3+
internal sealed class DisposableCollection : IDisposable
4+
{
5+
private readonly List<IDisposable> _disposables = [];
6+
7+
public DisposableCollection(IEnumerable<IDisposable> disposables) => _disposables.AddRange(disposables);
8+
9+
public void Add(IDisposable disposable) => _disposables.Add(disposable);
10+
11+
public void Dispose()
12+
{
13+
foreach (IDisposable disposable in _disposables)
14+
{
15+
disposable.Dispose();
16+
}
17+
}
18+
}

‎tests/DotNet.ReproducibleBuilds.Tests/ProjectCreatorExtensions.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,14 @@ public static Project ProjectWithGlobalProperties(this ProjectCreator creator, I
1111

1212
return project;
1313
}
14+
15+
public static ProjectCreator Properties(this ProjectCreator creator, IEnumerable<KeyValuePair<string, string?>> properties)
16+
{
17+
foreach (KeyValuePair<string, string?> property in properties)
18+
{
19+
creator.Property(property.Key, property.Value);
20+
}
21+
22+
return creator;
23+
}
1424
}

‎tests/DotNet.ReproducibleBuilds.Tests/SourceLinkTests.cs‎

Lines changed: 65 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,42 +51,16 @@ public void EmbedUntrackedSourcesIsSet(bool? embedUntrackedSources, bool expecte
5151
}
5252

5353
[Theory]
54-
[InlineData("GITHUB_REF", "refs/pull/1234/merge", "pr1234")]
55-
[InlineData("GITHUB_REF", "refs/heads/my-branch", "my-branch")]
56-
[InlineData("GITHUB_REF", "refs/tags/v1.2.3", "v1.2.3")]
57-
58-
[InlineData("BUILD_SOURCEBRANCH", "refs/heads/my-branch", "my-branch")]
59-
[InlineData("BUILD_SOURCEBRANCH", "refs/tags/v1.2.3", "v1.2.3")]
60-
61-
[InlineData("APPVEYOR_PULL_REQUEST_NUMBER", "1234", "pr1234")]
62-
[InlineData("APPVEYOR_REPO_TAG_NAME", "refs/tags/v1.2.3", "refs/tags/v1.2.3")]
63-
[InlineData("APPVEYOR_REPO_BRANCH", "refs/heads/my-branch", "refs/heads/my-branch")]
64-
65-
[InlineData("TEAMCITY_BUILD_BRANCH", "refs/heads/my-branch", "refs/heads/my-branch")]
66-
67-
[InlineData("TRAVIS_PULL_REQUEST", "1234", "pr1234")]
68-
[InlineData("TRAVIS_BRANCH", "refs/heads/my-branch", "refs/heads/my-branch")]
69-
70-
[InlineData("CIRCLE_PR_NUMBER", "1234", "pr1234")]
71-
[InlineData("CIRCLE_TAG", "refs/heads/v1.2.3", "refs/heads/v1.2.3")]
72-
[InlineData("CIRCLE_BRANCH", "refs/heads/my-branch", "refs/heads/my-branch")]
73-
74-
[InlineData("CI_COMMIT_TAG", "refs/tags/v1.2.3", "refs/tags/v1.2.3")]
75-
[InlineData("CI_MERGE_REQUEST_IID", "1234", "pr1234")]
76-
[InlineData("CI_COMMIT_BRANCH", "refs/heads/my-branch", "refs/heads/my-branch")]
77-
78-
[InlineData("BUDDY_EXECUTION_PULL_REQUEST_NO", "1234", "pr1234")]
79-
[InlineData("BUDDY_EXECUTION_TAG", "refs/tags/v1.2.3", "refs/tags/v1.2.3")]
80-
[InlineData("BUDDY_EXECUTION_BRANCH", "refs/heads/my-branch", "refs/heads/my-branch")]
81-
public void RepositoryBranchIsSet(string ci, string original, string expected)
54+
[MemberData(nameof(RepositoryBranchData))]
55+
public void RepositoryBranchIsSet(Dictionary<string, string?> env, string expected)
8256
{
8357
using EnvironmentVariableSuppressor hostSuppressor = new("BUILD_SOURCEBRANCH"); // Suppress our own CI provider variables (i.e. Azure DevOps)
84-
using EnvironmentVariableSuppressor ciSuppressor = new(ci); // Suppress the mock CI provider (just in case)
58+
using IDisposable ciSuppressors = env.Select(kvp => new EnvironmentVariableSuppressor(kvp.Key)).ToDisposable(); // Suppress the mock CI provider (just in case)
8559

8660
ProjectCreator project = ProjectCreator.Templates
8761
.ReproducibleBuildProject(GetRandomFile(".csproj"))
8862
.PropertyGroup()
89-
.Property(ci, original);
63+
.Properties(env);
9064

9165
// If RepositoryBranch is not set, it should be set from the CI provider property
9266
project.Project
@@ -99,4 +73,65 @@ public void RepositoryBranchIsSet(string ci, string original, string expected)
9973
.GetPropertyValue("RepositoryBranch")
10074
.Should().Be("explicitly-set", "because explicitly setting `RepositoryBranch` should always win.");
10175
}
76+
77+
public static TheoryData<Dictionary<string, string?>, string> RepositoryBranchData()
78+
{
79+
TheoryData<Dictionary<string, string?>, string> data = new()
80+
{
81+
{ new() { ["GITHUB_REF"] = "refs/pull/1234/merge" }, "refs/pull/1234/merge" },
82+
{ new() { ["GITHUB_REF"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
83+
{ new() { ["GITHUB_REF"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
84+
85+
{ new() { ["BUILD_SOURCEBRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
86+
{ new() { ["BUILD_SOURCEBRANCH"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
87+
88+
{ new() { ["APPVEYOR_PULL_REQUEST_NUMBER"] = "1234" }, "pr1234" },
89+
{ new() { ["APPVEYOR_REPO_TAG_NAME"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
90+
{ new() { ["APPVEYOR_REPO_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
91+
{ new() { ["APPVEYOR_PULL_REQUEST_NUMBER"] = "1234" , ["APPVEYOR_REPO_BRANCH"] = "refs/heads/my-branch" }, "pr1234" },
92+
{ new() { ["APPVEYOR_REPO_TAG_NAME"] = "refs/tags/v1.2.3" , ["APPVEYOR_REPO_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
93+
{ new() { ["APPVEYOR_PULL_REQUEST_NUMBER"] = "1234", ["APPVEYOR_REPO_TAG_NAME"] = "refs/tags/v1.2.3", ["APPVEYOR_REPO_BRANCH"] = "refs/heads/my-branch" }, "pr1234" },
94+
95+
{ new() { ["TEAMCITY_BUILD_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
96+
97+
{ new() { ["TRAVIS_PULL_REQUEST"] = "1234" }, "pr1234" },
98+
{ new() { ["TRAVIS_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
99+
{ new() { ["TRAVIS_PULL_REQUEST"] = "1234", ["TRAVIS_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
100+
{ new() { ["TRAVIS_PULL_REQUEST"] = "false", ["TRAVIS_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
101+
102+
{ new() { ["CIRCLE_PR_NUMBER"] = "1234" }, "pr1234" },
103+
{ new() { ["CIRCLE_TAG"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
104+
{ new() { ["CIRCLE_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
105+
{ new() { ["CIRCLE_PR_NUMBER"] = "1234", ["CIRCLE_TAG"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
106+
{ new() { ["CIRCLE_PR_NUMBER"] = "1234", ["CIRCLE_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
107+
{ new() { ["CIRCLE_TAG"] = "refs/tags/v1.2.3", ["CIRCLE_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
108+
{ new() { ["CIRCLE_PR_NUMBER"] = "1234", ["CIRCLE_TAG"] = "refs/tags/v1.2.3", ["CIRCLE_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
109+
110+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
111+
{ new() { ["CI_MERGE_REQUEST_IID"] = "1234" }, "pr1234" },
112+
{ new() { ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678" }, "pr5678" },
113+
{ new() { ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
114+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3", ["CI_MERGE_REQUEST_IID"] = "1234" }, "refs/tags/v1.2.3" },
115+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3", ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678" }, "refs/tags/v1.2.3" },
116+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3", ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
117+
{ new() { ["CI_MERGE_REQUEST_IID"] = "1234", ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678" }, "pr1234" },
118+
{ new() { ["CI_MERGE_REQUEST_IID"] = "1234", ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
119+
{ new() { ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678", ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
120+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3", ["CI_MERGE_REQUEST_IID"] = "1234", ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678" }, "refs/tags/v1.2.3" },
121+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3", ["CI_MERGE_REQUEST_IID"] = "1234", ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
122+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3", ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678", ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
123+
{ new() { ["CI_MERGE_REQUEST_IID"] = "1234", ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678", ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
124+
{ new() { ["CI_COMMIT_TAG"] = "refs/tags/v1.2.3", ["CI_MERGE_REQUEST_IID"] = "1234", ["CI_EXTERNAL_PULL_REQUEST_IID"] = "5678", ["CI_COMMIT_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
125+
126+
{ new() { ["BUDDY_EXECUTION_PULL_REQUEST_NO"] = "1234" }, "pr1234" },
127+
{ new() { ["BUDDY_EXECUTION_TAG"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
128+
{ new() { ["BUDDY_EXECUTION_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
129+
{ new() { ["BUDDY_EXECUTION_PULL_REQUEST_NO"] = "1234", ["BUDDY_EXECUTION_TAG"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
130+
{ new() { ["BUDDY_EXECUTION_PULL_REQUEST_NO"] = "1234", ["BUDDY_EXECUTION_BRANCH"] = "refs/heads/my-branch" }, "refs/heads/my-branch" },
131+
{ new() { ["BUDDY_EXECUTION_TAG"] = "refs/tags/v1.2.3", ["BUDDY_EXECUTION_BRANCH"] = "refs/heads/my-branch" }, "refs/tags/v1.2.3" },
132+
{ new() { ["BUDDY_EXECUTION_PULL_REQUEST_NO"] = "1234", ["BUDDY_EXECUTION_BRANCH"] = "refs/heads/my-branch", ["BUDDY_EXECUTION_TAG"] = "refs/tags/v1.2.3" }, "refs/tags/v1.2.3" },
133+
};
134+
135+
return data;
136+
}
102137
}

0 commit comments

Comments
 (0)