8000 Support custom culture in RAR (#11000) · dotnet/msbuild@04ef516 · GitHub
[go: up one dir, main page]
Skip to content

Commit 04ef516

Browse files
JanKrivanekJanKrivanek
authored
Support custom culture in RAR (#11000)
* Support custom culture * Add ChangeWave info * Warn on unintended culture overwrite * Fix the warning call * Add and fix unittests * Apply PR comments
1 parent 4c6a5a9 commit 04ef516

22 files changed

+263
-37
lines changed

‎documentation/wiki/ChangeWaves.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
2626
### 17.14
2727
- [.SLNX support - use the new parser for .sln and .slnx](https://github.com/dotnet/msbuild/pull/10836)
2828
- [TreatWarningsAsErrors, WarningsAsMessages, WarningsAsErrors, WarningsNotAsErrors are now supported on the engine side of MSBuild](https://github.com/dotnet/msbuild/pull/10942)
29+
- [Support custom culture in RAR](https://github.com/dotnet/msbuild/pull/11000)
29< 9E88 /td>30

3031
### 17.12
3132
- [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908)

‎src/BuildCheck.UnitTests/EndToEndTests.cs

Lines changed: 74 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -64,54 +64,102 @@ public void PropertiesUsageAnalyzerTest(bool buildInOutOfProcessNode)
6464
Regex.Matches(output, "BC0203 .* Property").Count.ShouldBe(2);
6565
}
6666

67-
// <EmbeddedResource Update = "Resource1.cs.resx" />
68-
6967
[Theory]
68+
// The culture is not set explicitly, but the extension is a known culture
69+
// - a buildcheck warning will occur, but otherwise works
7070
[InlineData(
7171
"cs",
7272
"cs",
7373
"""<EmbeddedResource Update = "Resource1.cs.resx" />""",
74-
"warning BC0105: .* 'Resource1\\.cs\\.resx'")]
75-
// Following tests are prepared after the EmbeddedCulture handling fix is merged: https://github.com/dotnet/msbuild/pull/11000
76-
////[InlineData(
77-
//// "xyz",
78-
//// "xyz",
79-
//// """<EmbeddedResource Update = "Resource1.xyz.resx" />""",
80-
//// "warning BC0105: .* 'Resource1\\.xyz\\.resx'")]
81-
////[InlineData(
82-
//// "xyz",
83-
//// "zyx",
84-
//// """<EmbeddedResource Update = "Resource1.zyx.resx" Culture="xyz" />""",
85-
//// "")]
86-
public void EmbeddedResourceCheckTest(string culture, string resourceExtension, string resourceElement, string expectedDiagnostic)
74+
false,
75+
"warning BC0105: .* 'Resource1\\.cs\\.resx'",
76+
true)]
77+
// The culture is not set explicitly, and is not a known culture
78+
// - a buildcheck warning will occur, and resource is not recognized as culture specific - won't be copied around
79+
[InlineData(
80+
"xyz",
81+
"xyz",
82+
"""<EmbeddedResource Update = "Resource1.xyz.resx" />""",
83+
false,
84+
"warning BC0105: .* 'Resource1\\.xyz\\.resx'",
85+
false)]
86+
// The culture is explicitly set, and it is not a known culture, but $(RespectAlreadyAssignedItemCulture) is set to true
87+
// - no warning will occur, and resource is recognized as culture specific - and copied around
88+
[InlineData(
89+
"xyz",
90+
"xyz",
91+
"""<EmbeddedResource Update = "Resource1.xyz.resx" Culture="xyz" />""",
92+
true,
93+
"",
94+
true)]
95+
// The culture is explicitly set, and it is not a known culture and $(RespectAlreadyAssignedItemCulture) is not set to true
96+
// - so culture is overwritten, and resource is not recognized as culture specific - won't be copied around
97+
[InlineData(
98+
"xyz",
99+
"zyx",
100+
"""<EmbeddedResource Update = "Resource1.zyx.resx" Culture="xyz" />""",
101+
false,
102+
"warning MSB3002: Explicitly set culture .* was overwritten",
103+
false)]
104+
// The culture is explicitly set, and it is not a known culture, but $(RespectAlreadyAssignedItemCulture) is set to true
105+
// - no warning will occur, and resource is recognized as culture specific - and copied around
106+
[InlineData(
107+
"xyz",
108+
"zyx",
109+
"""<EmbeddedResource Update = "Resource1.zyx.resx" Culture="xyz" />""",
110+
true,
111+
"",
112+
true)]
113+
public void EmbeddedResourceCheckTest(
114+
string culture,
115+
string resourceExtension,
116+
string resourceElement,
117+
bool respectAssignedCulturePropSet,
118+
string expectedDiagnostic,
119+
bool resourceExpectedToBeRecognizedAsSatelite)
87120
{
88-
EmbedResourceTestOutput output = RunEmbeddedResourceTest(resourceElement, resourceExtension);
121+
EmbedResourceTestOutput output = RunEmbeddedResourceTest(resourceElement, resourceExtension, respectAssignedCulturePropSet);
89122

123+
int expectedWarningsCount = 0;
90124
// each finding should be found just once - but reported twice, due to summary
91125
if (!string.IsNullOrEmpty(expectedDiagnostic))
92126
{
93127
Regex.Matches(output.LogOutput, expectedDiagnostic).Count.ShouldBe(2);
128+
expectedWarningsCount = 1;
94129
}
95130

96-
AssertHasResourceForCulture("en");
97-
AssertHasResourceForCulture(culture);
98-
output.DepsJsonResources.Count.ShouldBe(2);
131+
AssertHasResourceForCulture("en", true);
132+
AssertHasResourceForCulture(culture, resourceExpectedToBeRecognizedAsSatelite);
133+
output.DepsJsonResources.Count.ShouldBe(resourceExpectedToBeRecognizedAsSatelite ? 2 : 1);
134+
GetWarningsCount(output.LogOutput).ShouldBe(expectedWarningsCount);
99135

100-
void AssertHasResourceForCulture(string culture)
136+
void AssertHasResourceForCulture(string culture, bool isResourceExpected)
101137
{
102138
KeyValuePair<string, JsonNode?> resource = output.DepsJsonResources.FirstOrDefault(
103139
o => o.Value?["locale"]?.ToString().Equals(culture, StringComparison.Ordinal) ?? false);
104-
resource.Equals(default(KeyValuePair<string, JsonNode?>)).ShouldBe(false,
105-
$"Resource for culture {culture} was not found in deps.json:{Environment.NewLine}{output.DepsJsonResources.ToString()}");
140+
// if not found - the KVP will be default
141+
resource.Equals(default(KeyValuePair<string, JsonNode?>)).ShouldBe(!isResourceExpected,
142+
$"Resource for culture {culture} was {(isResourceExpected ? "not " : "")}found in deps.json:{Environment.NewLine}{output.DepsJsonResources.ToString()}");
106143

107-
resource.Key.ShouldBeEquivalentTo($"{culture}/ReferencedProject.resources.dll",
108-
$"Unexpected resource for culture {culture} was found in deps.json:{Environment.NewLine}{output.DepsJsonResources.ToString()}");
144+
if (isResourceExpected)
145+
{
146+
resource.Key.ShouldBeEquivalentTo($"{culture}/ReferencedProject.resources.dll",
147+
$"Unexpected resource for culture {culture} was found in deps.json:{Environment.NewLine}{output.DepsJsonResources.ToString()}");
148+
}
149+
}
150+
151+
int GetWarningsCount(string output)
152+
{
153+
Regex regex = new Regex(@"(\d+) Warning\(s\)");
154+
Match match = regex.Match(output);
155+
match.Success.ShouldBeTrue("Expected Warnings section not found in the build output.");
156+
return int.Parse(match.Groups[1].Value);
109157
}
110158
}
111159

112160
private readonly record struct EmbedResourceTestOutput(String LogOutput, JsonObject DepsJsonResources);
113161

114-
private EmbedResourceTestOutput RunEmbeddedResourceTest(string resourceXmlToAdd, string resourceExtension)
162+
private EmbedResourceTestOutput RunEmbeddedResourceTest(string resourceXmlToAdd, string resourceExtension, bool respectCulture)
115163
{
116164
string testAssetsFolderName = "EmbeddedResourceTest";
117165
const string entryProjectName = "EntryProject";
@@ -128,7 +176,7 @@ private EmbedResourceTestOutput RunEmbeddedResourceTest(string resourceXmlToAdd,
128176

129177
_env.SetCurrentDirectory(Path.Combine(workFolder.Path, entryProjectName));
130178

131-
string output = RunnerUtilities.ExecBootstrapedMSBuild("-check -restore", out bool success);
179+
string output = RunnerUtilities.ExecBootstrapedMSBuild("-check -restore /p:RespectCulture=" + (respectCulture ? "True" : "\"\""), out bool success);
132180
_env.Output.WriteLine(output);
133181
_env.Output.WriteLine("=========================");
134182
success.ShouldBeTrue();

‎src/BuildCheck.UnitTests/TestAssets/EmbeddedResourceTest/ReferencedProject/ReferencedProject.csproj

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<TargetFramework>net9.0</TargetFramework>
4+
<!-- Target net8.0 - as from net9.0 the RespectAlreadyAssignedItemCulture is added by common targets. -->
5+
<TargetFramework>net8.0</TargetFramework>
56
<ImplicitUsings>enable</ImplicitUsings>
67
<Nullable>enable</Nullable>
78
</PropertyGroup>
@@ -15,7 +16,7 @@
1516
</ItemGroup>
1617

1718
<PropertyGroup>
18-
<RespectAlreadyAssignedItemCulture>True</RespectAlreadyAssignedItemCulture>
19+
<RespectAlreadyAssignedItemCulture>$(RespectCulture)</RespectAlreadyAssignedItemCulture>
1920
</PropertyGroup>
2021

2122
<ItemGroup>

‎src/Framework/StringUtils.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,24 @@ internal static string GenerateRandomString(int length)
3131
string randomBase64String = Convert.ToBase64String(randomBytes).Replace('/', '_');
3232
return randomBase64String.Substring(0, length);
3333
}
34+
35+
/// <summary>
36+
/// Removes last occurence of <paramref name="substring"/> from <paramref name="fromString"/>, if present.
37+
/// </summary>
38+
/// <param name="fromString">String to be altered.</param>
39+
/// <param name="substring">String to be removed.</param>
40+
/// <param name="comparison">The comparison to use for finding.</param>
41+
/// <returns>The original string (if no occurrences found) or a new string, with last instance of <paramref name="substring"/> removed.</returns>
42+
internal static string RemoveLastInstanceOf(this string fromString, string substring, StringComparison comparison = StringComparison.Ordinal)
43+
{
44+
int lastOccurrenceIndex = fromString.LastIndexOf(substring, comparison);
45+
46+
if (lastOccurrenceIndex != -1)
47+
{
48+
fromString = fromString.Substring(0, lastOccurrenceIndex) +
49+
fromString.Substring(lastOccurrenceIndex + substring.Length);
50+
}
51+
52+
return fromString;
53+
}
3454
}

‎src/Tasks/AssemblyDependency/ReferenceTable.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,8 @@ private void FindSatellites(
971971
// Is there a candidate satellite in that folder?
972972
string cultureName = Path.GetFileName(subDirectory);
973973

974-
if (CultureInfoCache.IsValidCultureString(cultureName))
974+
// Custom or unknown cultures can be met as well
975+
if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14) || CultureInfoCache.IsValidCultureString(cultureName))
975976
{
976977
string satelliteAssembly = Path.Combine(subDirectory, satelliteFilename);
977978
if (_fileExists(satelliteAssembly))

‎src/Tasks/AssignCulture.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using Microsoft.Build.Collections;
67
#if DEBUG
78
using System.Diagnostics;
89
#endif
@@ -158,6 +159,20 @@ public override bool Execute()
158159
// https://github.com/dotnet/msbuild/issues/3064
159160
ConversionUtilities.ValidBooleanFalse(AssignedFiles[i].GetMetadata(ItemMetadataNames.withCulture)));
160161

162+
// The culture was explicitly specified, but not opted in via 'RespectAlreadyAssignedItemCulture' and different will be used
163+
if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14) &&
164+
!string.IsNullOrEmpty(existingCulture) &&
165+
!MSBuildNameIgnoreCaseComparer.Default.Equals(existingCulture, info.culture))
166+
{
167+
Log.LogWarningWithCodeFromResources("AssignCulture.CultureOverwritten",
168+
existingCulture, AssignedFiles[i].ItemSpec, info.culture);
169+
// Remove the culture if it's not recognized
170+
if (string.IsNullOrEmpty(info.culture))
171+
{
172+
AssignedFiles[i].RemoveMetadata(ItemMetadataNames.culture);
173+
}
174+
}
175+
161176
if (!string.IsNullOrEmpty(info.culture))
162177
{
163178
AssignedFiles[i].SetMetadata(ItemMetadataNames.culture, info.culture);

‎src/Tasks/CreateCSharpManifestResourceName.cs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,26 @@ internal static string CreateManifestNameImpl(
101101
}
102102

103103
dependentUponFileName = FileUtilities.FixFilePath(dependentUponFileName);
104-
Culture.ItemCultureInfo info = Culture.GetItemCultureInfo(embeddedFileName, dependentUponFileName, treatAsCultureNeutral);
104+
Culture.ItemCultureInfo info;
105105

106-
// If the item has a culture override, respect that.
107-
if (!string.IsNullOrEmpty(culture))
106+
if (!string.IsNullOrEmpty(culture) && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))
108107
{
109-
info.culture = culture;
108+
info = new Culture.ItemCultureInfo()
109+
{
110+
culture = culture,
111+
cultureNeutralFilename =
112+
embeddedFileName.RemoveLastInstanceOf("." + culture, StringComparison.OrdinalIgnoreCase)
113+
};
114+
}
115+
else
116+
{
117+
info = Culture.GetItemCultureInfo(embeddedFileName, dependentUponFileName, treatAsCultureNeutral);
118+
// If the item has a culture override, respect that.
119+
// We need to recheck here due to changewave in condition above - after Wave17_14 removal, this should be unconditional.
120+
if (!string.IsNullOrEmpty(culture))
121+
{
122+
info.culture = culture;
123+
}
110124
}
111125

112126
var manifestName = StringBuilderCache.Acquire();

‎src/Tasks/CreateVisualBasicManifestResourceName.cs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,27 @@ internal static string CreateManifestNameImpl(
9999
embeddedFileName = fileName;
100100
}
101101

102-
Culture.ItemCultureInfo info = Culture.GetItemCultureInfo(embeddedFileName, dependentUponFileName, treatAsCultureNeutral);
102+
dependentUponFileName = FileUtilities.FixFilePath(dependentUponFileName);
103+
Culture.ItemCultureInfo info;
103104

104-
// If the item has a culture override, respect that.
105-
if (!string.IsNullOrEmpty(culture))
105+
if (!string.IsNullOrEmpty(culture) && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))
106106
{
107-
info.culture = culture;
107+
info = new Culture.ItemCultureInfo()
108+
{
109+
culture = culture,
110+
cultureNeutralFilename =
111+
embeddedFileName.RemoveLastInstanceOf("." + culture, StringComparison.OrdinalIgnoreCase)
112+
};
113+
}
114+
else
115+
{
116+
info = Culture.GetItemCultureInfo(embeddedFileName, dependentUponFileName, treatAsCultureNeutral);
117+
// If the item has a culture override, respect that.
118+
// We need to recheck here due to changewave in condition above - after Wave17_14 removal, this should be unconditional.
119+
if (!string.IsNullOrEmpty(culture))
120+
{
121+
info.culture = culture;
122+
}
108123
}
109124

110125
var manifestName = StringBuilderCache.Acquire();

‎src/Tasks/Resources/Strings.resx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,13 @@
149149
<data name="AssignCulture.Comment">
150150
<value>Culture of "{0}" was assigned to file "{1}".</value>
151151
</data>
152+
<data name="AssignCulture.CultureOverwritten">
153+
<value>MSB3002: Explicitly set culture "{0}" for item "{1}" was overwritten with inferred culture "{2}", because 'RespectAlreadyAssignedItemCulture' property was not set.</value>
154+
<comment>
155+
{StrBegin="MSB3002: "}
156+
'RespectAlreadyAssignedItemCulture' should not be translated
157+
</comment>
158+
</data>
152159
<!--
153160
The AxImp message bucket is: MSB3656 - MSB3660.
154161

‎src/Tasks/Resources/xlf/Strings.cs.xlf

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
0