[Xamarin.Android.Build.Tasks] Move XA4214 warning text into .resx file

[brendanzagaeski]

Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1009374/

This is a first step toward making the Xamarin.Android.Build.Tasks
MSBuild error and warning messages localizable. This first step covers
a few decisions:

• Use the MSBuild message codes like XA4214 as the names for the
  string resources.
• When an MSBuild message code like XA4214 is associated with more
  than one string, append single capital letters starting with A to
  the end of the code to create as many different names as needed.
• Name the .resx file Resources.resx, and place it in the
  src/Xamarin.Android.Build.Tasks/Properties/ directory, alongside
  the AssemblyInfo.cs file. This follows what the nonpublic
  xamarin/designer and xamarin/XamarinVS repositories do.
• Store the .resx file with LF line endings in the repo, but set
  .resx files to text mode in .gitattributes so the file can be
  checked out and edited with CRLF line endings on Windows. The
  Visual Studio .resx resource editor uses CRLF line endings.
• Use the <Generator>ResXFileCodeGenerator</Generator> Visual Studio
  IDE mechanism to generate a Resources.Designer.cs file that is
  explicitly included in the project as a @(Compile) item. The
  Resource.Designer.cs file provides C# property names to access the
  .resx resources. Visual Studio updates the file each time a
  change is saved to the corresponding .resx file. This follows
  what the nonpublic xamarin/designer and xamarin/XamarinVS repos do.
  In the future, if Xamarin.Android.Build.Tasks.csproj is converted
  to a short-form project, it would probably be best to add a
  dependency on dotnet/arcade and switch to using the
  GenerateResxSource mechanism instead. That would match what
  dotnet/sdk does.
• Use the comment style from dotnet/sdk to provide a brief
  explanation of each format item like {0}, especially in cases
  where the expected contents of the format item might not be obvious
  from context.
• Keep the MSBuild message codes as string literals in the .cs
  files. This differs from what dotnet/sdk and microsoft/msbuild do.
  This might be incorrect. It might be important to include the
  message code prefix in the string so that the localization team can
  decide on the appropriate capitalization after the : character.
  See for example the use of lowercase letters after the : character
  in the Italian translations in microsoft/msbuild and
  dotnet/sdk.

String values from .resx resources are not subject to interpolation,
so any strings that use C# 6 string interpolation will need to be
replaced with format strings that use format items like {0}. The
message for XA4214 did not use string interpolation, so no change was
needed for this first example.

This commit does not yet leverage the MSBuild tasks and targets from
xliff-tasks to generate satellite assemblies for localized string
resources. That will be the next step.

This commit intentionally skips adding anything to
Documentation/release-notes/ for now because these changes do not yet
affect anything interesting to end users.

[jonpryor]

When an MSBuild message code like XA4214 is associated with more
than one string, append single capital letters starting with A to
the end of the code to create as many different names as needed.

Convention-wise, I disagree. It should be CODE_SemanticDifferentiator. Properties.Resources.XA4214A vs. Properties.Resources.XA4214B looks annoying and will require reading the .resx file to differentiate. Properties.Resources.XA4214 vs. Properties.Resources.XA4214_References & Properties.Resources.XA4214_Resolution will make a bit more sense.

[jonpryor]

This line should also be translated. All instances of XA4214 should be handled in the same PR, in part to "demonstrate" how "identical codes but different messages" should be handled.

[jonpryor]

This commit does not yet leverage the MSBuild tasks and targets from
xliff-tasks to generate satellite assemblies for localized string
resources. That will be the next step.

Should that be part of this PR or a separate PR?

[brendanzagaeski]

Changes in the latest commits:

When an MSBuild message code like XA4214 is associated with more than one string, instead of appending letters, append short descriptive phrases to the end of the code to create additional property names as needed.

Move the second message string for XA4214 to the .resx file.

[brendanzagaeski]

This commit does not yet leverage the MSBuild tasks and targets from
xliff-tasks to generate satellite assemblies for localized string
resources. That will be the next step.

Should that be part of this PR or a separate PR?

It might be handy to keep them as separate smaller PRs, but I'm happy with either option. I think I'll have a draft of the PR to use xliff-tasks ready within the next day or two, building on top of the commits in this PR, so I could always just put those commits on this same branch if they look like a good addition.

At the least, it would be fine by me to wait to merge this PR until the commits to use xliff-tasks are ready. That way I could make sure nothing is missing from this first step.

[jonathanpeppers]

Since using the XA4214 resource made it a lot shorter, should this LogCodedWarning just be on one line?

Otherwise looks good to me. 👍

[brendanzagaeski]

Changes in the latest commit:

Collapse the LogCodedWarning() call down to a single line now that it's short.

[jonpryor]

At the least, it would be fine by me to wait to merge this PR until the commits to use xliff-tasks are ready. That way I could make sure nothing is missing from this first step.

Let's include xliff-tasks integration in this PR then. :-)

[brendanzagaeski]

Changes in the latest commit:

Add xliff-tasks and initial .xlf files.

I believe this first example for the XA4214 warning is now functionally complete, apart from having the translations team add actual translations to the .xlf files.

[jonpryor]

What is this property used by? I see no other references to UpdateXlfOnBuild within this PR.

[brendanzagaeski]

Ah yes, whoops. I meant to surface all the notes from the new commit message. For this question, step 3 in particular is the key info.

The dotnet/xliff-tasks tasks and targets find .resx files in a
project, generate or update corresponding .xlf XLIFF files as
needed, and then build satellite assemblies from the .xlf files.

Using xliff-tasks involves a few steps:

1. Add the dotnet-eng Azure DevOps NuGet package feed or the
   dotnet-core MyGet NuGet package feed to NuGet.config. This commit
   uses dotnet-eng because that appears to be the more up-to-date
   recommendation, based on the dotnet/arcade README.

2. Add the XliffTasks NuGet package to the project to localize.

3. Set $(UpdateXlfOnBuild) to true for the project when building
   locally, but not when building on CI. This follows the strategy
   suggested in the xliff-tasks README:

   Many teams using `XliffTasks` default `UpdateXlfOnBuild` to true
   for local developer builds, but leave it off for CI builds. This
   way the .xlf files are automatically updated as the developer
   works, and the CI build will fail if the developer forgets to
   include the changes to the .xlf files as part of their PR. This
   way the .xlf files are always in sync with the source files, and
   can be handed off to a localization team at any time.
   

   This commit sets the property globally via Configuration.props to
   help make it easily visible to contributors.

4. Build the project to generate the initial set of .xlf files.

5. Add the generated .xlf files to source control.

6. Update the targets for the installer packages so that they include
   the satellite assemblies generated by XliffTasks.

In the future, if Xamarin.Android.Build.Tasks.csproj is converted to a
short-form project, it would probably be best to add a dependency on
dotnet/arcade, set $(UsingToolXliff) to true, and then undo steps
1-3 since dotnet/arcade takes care of those steps automatically.

I locally verified that the satellite assemblies were included in the
expected locations in both the Windows and macOS installers with this
commit in place.

I also locally verified that MSBuild used the text from
Resources.fr.xlf for the XA4214 warning if I modified
GenerateJavaStubs.Run() to set both
Thread.CurrentThread.CurrentCulture() and
Thread.CurrentThread.CurrentUICulture() to
new CultureInfo ("fr-FR").

[jonpryor]

@pjcollins, @brendanzagaeski: Should we create a @(_LocalizationLanguages) item group and use an "expansion" here?

<ItemGroup>
  <_LocalizationLanguages Include="cs" />
  <_LocalizationLanguages Include="de" />
  <!-- ... -->
</ItemGroup>

<!-- ... -->

  <_MSBuildFiles Include="@(_LocalizationLanguages->'$(MSBuildSrcDir)\%(Identity)\Xamarin.Android.Build.Tasks.resources.dll')" />

[dellis1972]

I think an expansion would be good here.

[jonpryor]

Here is another place that @(_LocalizationLanguages) might be useful.

[brendanzagaeski]

Changes in the latest force-push:

Rebase on master to fix the merge conflicts.

Squash the earlier commits and update the commit message.

Add a document about how to make messages localizable.

Set $(XlfLanguages) (used by xliff-tasks) to the current default value from xliff-tasks.

Add a @(_LocalizationLanguages) item list that includes all of the languages from $(XlfLanguages).

Use @(_LocalizationLanguages) to define the localization item lists more compactly in create-installers.targets and Xamarin.Android.Build.Tasks.csproj.

[jonpryor]

Proposed commit message:

Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1009374/

This is a first step toward localizing the MSBuild error and warning
messages produced by `Xamarin.Android.Build.Tasks.dll`.

We will be following the [.NET Resource Localization pattern][0] and
satellite assemblies using [`.resx` files][1], in particular
`src/Xamarin.Android.Build.Tasks/Properties/Resources.resx`.

`Resources.resx` is an XML file, and will contain `/root/data`
elements in which `//data/@name` will start with the Xamarin.Android
error or warning code, and `//data/value` will be the error or
warning message:

        <root>
          <data name="XA4214" xml:space="preserve">
            <value>The managed type `{0}` exists in multiple assemblies: {1}. Please refactor the managed type names in these assemblies so that they are not identical.</value>
          </data>
        </root>

An optional `//data/comment` element may be provided to describe the
meaning within the `//data/value` element to translators:

        <data name="XA4214" xml:space="preserve">
          <value>The managed type `{0}` exists in multiple assemblies: {1}. Please refactor the managed type names in these assemblies so that they are not identical.</value>
          <comment>
            {0} - The managed type name
            {1} - Comma-separated list of all the assemblies where the managed type exists
          </comment>
        </data>

During the build, `Resources.resx` will be translated into a
`Resources.Designer.cs` file:

        namespace Xamarin.Android.Tasks.Properties {
          internal partial class Resources {
            internal static string XA4214 {
              get => ...
            }
          }
        }

The `Resources` members should be used to obtain all strings for use
in `LogCodedError()` and `LogCodedWarning()` calls:

        Log.LogCodedWarning ("XA4214", Properties.Resources.XA4214, kvp.Key, string.Join (", ", kvp.Value));

When an MSBuild error and warning code is used with more than one
output string, then a semantically meaningful suffix should be used
to distinguish between the two:

        <data name="XA4214_Result" xml:space="preserve">
          <value>References to the type `{0}` will refer to `{0}, {1}`.</value>
        </data>

Note that this infrastructure does not interoperate with C#6 string
interpolation.  Any error and warning messages currently using C#6
string interpolation will need to use .NET 1.0-style format strings.

Our translation team doesn't work directly with `.resx` files.
Instead, the translation team works with [XLIFF files][2].
`Resources.resx` is converted into a set of
`src/Xamarin.Android.Build.Tasks/Properties/xlf/Resources.*.xlf`
files via `XliffTasks.targets` from the [dotnet/xliff-tasks][3] repo.
The `Resources.*.xlf` files should be automatically updated whenever
`Resources.resx` is updated.


Other:

  * This approach leaves the error code `XA4214` as a string literal
    for now.  This differs from what dotnet/sdk and microsoft/msbuild
    do; they instead include the message code as part of the string
    resource in the `.resx` file.  That might sometimes provide useful
    additional context for the translation team, but it also requires
    using a different set of logging methods from
    `Microsoft.Build.Utilities.TaskLoggingHelper`.

  * Fix the Test MSBuild Azure Pipelines build

    Specify the `feedsToUse` and `nugetConfigPath` inputs for the
    [`NuGetCommand@2`][6] Azure Pipelines task so that the NuGet
    restore step will be able to restore XliffTasks successfully from
    the dotnet-eng Azure DevOps NuGet package feed.

    This resolves the following error:

        The nuget command failed with exit code(1) and error(Errors in packages.config projects
            Unable to find version '1.0.0-beta.19252.1' of package 'XliffTasks'.
              C:\Users\dlab14\.nuget\packages\: Package 'XliffTasks.1.0.0-beta.19252.1' is not found on source 'C:\Users\dlab14\.nuget\packages\'.
              https://api.nuget.org/v3/index.json: Package 'XliffTasks.1.0.0-beta.19252.1' is not found on source 'https://api.nuget.org/v3/index.json'.)

TODO:

  * When `Xamarin.Android.Build.Tasks.csproj` is converted into a
    [short-form project][4], add a dependency on dotnet/arcade and
    switch to using the [`GenerateResxSource` mechanism][5] instead
    of using `%(EmbeddedResource.Generator)`=ResXFileCodeGenerator
    and set `$(UsingToolXliff)`=True.  This would match dotnet/sdk.

[0]: https://docs.microsoft.com/en-us/dotnet/framework/resources/index
[1]: https://docs.microsoft.com/en-us/dotnet/framework/resources/creating-resource-files-for-desktop-apps#resources-in-resx-files
[2]: http://docs.oasis-open.org/xliff/v1.2/os/xliff-core.html
[3]: https://github.com/dotnet/xliff-tasks
[4]: https://docs.microsoft.com/visualstudio/msbuild/how-to-use-project-sdk
[5]: https://github.com/dotnet/arcade/blob/e67d9f098029ebecedf11012a749b532d68ad2a9/Documentation/ArcadeSdk.md#generateresxsource-bool
[6]: https://docs.microsoft.com/azure/devops/pipelines/tasks/package/nuget

[brendanzagaeski]

Looks good! The only things I noticed are some possible tiny changes:

•  We will be following the [.NET Resource Localization pattern][0] and
  -satellite assemblies using [`.resx` files][1]
  +generating satellite assemblies using [`.resx` files][1]

• -When an MSBuild error and warning code is used with more than one
  +When an MSBuild error or warning code is used with more than one
   output string

  -interpolation.  Any error and warning messages currently using C#6
  +interpolation.  Any error or warning messages currently using C#6
   string interpolation will need to use .NET 1.0-style format strings.

  "And" works too, so this is just to double-check which is preferred.

• The locale names could optionally be removed from links 0 and 1 for shorter links:

  -[0]: https://docs.microsoft.com/en-us/dotnet/framework/resources/index
  -[1]: https://docs.microsoft.com/en-us/dotnet/framework/resources/creating-resource-files-for-desktop-apps#resources-in-resx-files
  +[0]: https://docs.microsoft.com/dotnet/framework/resources/index
  +[1]: https://docs.microsoft.com/dotnet/framework/resources/creating-resource-files-for-desktop-apps#resources-in-resx-files

---

I think this proposed message has some nice succinct wording that could help shorten some parts of Localization.md too, but that's not urgent, so I can put that in a small follow-up docs PR.

I think the proposed message also looks good on skipping the verbose steps about how to use the XliffTasks NuGet package. I'll put that info in the monodroid wiki page to be thorough, and the comment in this PR with that info will be available for reference, and once the move to short-form projects happens, those details won't be relevant anymore anyway.