[Xamarin.Android.Build.Tasks] Ignore `global` Alias when emitting code. #4554

dellis1972 · 2020-04-14T09:35:16Z

Our previous fix d18c824 did not take into account that users might
need to add global to their list of aliases in the ProjectReferece
MetaData. This then causes the following error

Error CS1681: You cannot redefine the global extern alias (CS1681)

So to fix this we need to ignore the global entry if one is present.

brendanzagaeski · 2020-04-14T18:13:56Z

I have one little question that wouldn't change this PR about global, so it can be probably be considered in a separate PR (if it sounds sensible).

In 49d12fd, the declaringTypeName line was was updated so it always uses the first alias in the list: https://github.com/xamarin/xamarin-android/blob/49d12fdce20f1d0abd926dae3fbbeb56f037d36a/src/Xamarin.Android.Build.Tasks/Utilities/ResourceDesignerImportGenerator.cs#L63

To minimize potential conflicts in Resource.designer.cs, is it worth making the same adjustment to the line that emits the extern alias statements?

https://github.com/xamarin/xamarin-android/blob/d48cf04f9749664bf48fc16bcb920d5d941cccab/src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesigner.cs#L206-L207

brendanzagaeski

Draft release notes

Here's a candidate release note for this change. Feel free edit as desired and add it to this pull request. Thanks much! (Additional info about preparing draft release notes can now be found in the release notes README.)

#### Application and library build and deployment

  * *error CS1681: You cannot redefine the global extern alias* prevented
    building app projects successfully if the *.csproj* file included `global`
    in the `<Aliases></Aliases>` metadata element of a referenced
    Xamarin.Android class library.

brendanzagaeski · 2020-04-15T17:19:52Z

src/Xamarin.Android.Build.Tasks/Utilities/ResourceDesignerImportGenerator.cs

@@ -49,7 +49,7 @@ public void CreateImportMethods (IEnumerable<ITaskItem> libraries)
 						continue;
 					}
 					string alias = assemblyPath.GetMetadata ("Aliases");
-					bool hasAlias = !string.IsNullOrEmpty (alias);
+					bool hasAlias = !string.IsNullOrEmpty (alias) && !aliases.Contains ("global");


I just noticed yesterday that Visual Studio includes global in the Aliases property input by default, so it's somewhat likely for projects to have global included even if they don't need it. With that in mind, I'm wondering if it would be best to handle the following scenario:

There are two assemblies that provide the same namespace, and both contain Android resources.

Each assembly includes global in the %(Aliases) along with a unique name, so for example:

<Aliases>global,assembly1</Aliases>

<Aliases>global,assembly2</Aliases>

Everywhere in hand-written C# files, the project is careful to use extern alias assembly1 and extern alias assembly2 and to prefix each type name or using statement with the appropriate alias.

With this && !aliases.Contains ("global") change, I think the generated Resource.designer.cs file will not prefix the types from either assembly?

Perhaps even more interesting is to add two more points to the scenario:

In addition to the conflicting namespaces, each assembly also contains its own namespace that does not conflict with the other assembly.

Some of the hand-written C# files use those non-conflicting namespaces, and they don't use any extern alias statements because both namespaces are accessible directly under global without conflicts.

I think if there is a global in the Aliases Metadata provided by the user, we don't need to emit ANY extern alias values. since the normal global one will just work.

If global is not present in that metadata, then global will NOT work, so we need the alias.

Here's a test that covers the scenario I'm picturing. Removing && !aliasMetaData.Contains ("global") from ResourceDesignerImportGenerator.cs and GenerateResourceDesigner.cs allows this test to pass:

[Test] public void CheckMultipleLibraryProjectReferenceAlias ([Values (true, false)] bool withGlobal) { var path = Path.Combine (Root, "temp", TestName); var library1 = new XamarinAndroidLibraryProject () { ProjectName = "Library1", }; var library2 = new XamarinAndroidLibraryProject () { ProjectName = "Library2", RootNamespace = "Library1" }; var proj = new XamarinAndroidApplicationProject () { References = { new BuildItem.ProjectReference (Path.Combine("..", library1.ProjectName, Path.GetFileName (library1.ProjectFilePath)), "Library1") { Metadata = { { "Aliases", withGlobal ? "global,Lib1A,Lib1B" : "Lib1A,Lib1B" } }, }, new BuildItem.ProjectReference (Path.Combine("..", library2.ProjectName, Path.GetFileName (library2.ProjectFilePath)), "Library2") { Metadata = { { "Aliases", withGlobal ? "global,Lib2A,Lib2B" : "Lib2A,Lib2B" } }, }, }, }; using (var builder1 = CreateDllBuilder (Path.Combine (path, library1.ProjectName), cleanupAfterSuccessfulBuild: false, cleanupOnDispose: false)) { builder1.ThrowOnBuildFailure = false; Assert.IsTrue (builder1.Build (library1), "Library should have built."); using (var builder2 = CreateDllBuilder (Path.Combine (path, library2.ProjectName), cleanupAfterSuccessfulBuild: false, cleanupOnDispose: false)) { builder2.ThrowOnBuildFailure = false; Assert.IsTrue (builder2.Build (library2), "Library should have built."); using (var b = CreateApkBuilder (Path.Combine (path, proj.ProjectName), cleanupAfterSuccessfulBuild: false, cleanupOnDispose: false)) { b.ThrowOnBuildFailure = false; Assert.IsTrue (b.Build (proj), "Project should have built."); string[] text = File.ReadAllLines (Path.Combine (Root, path, proj.ProjectName, "Resources", "Resource.designer.cs")); Assert.IsTrue (text.Count (x => x.Contains ("Library1.Resource.String.library_name")) == 2, "library_name resource should be present exactly once for each library"); Assert.IsTrue (text.Count (x => x == "extern alias Lib1A;" || x == "extern alias Lib1B;") <= 1, "No more than one extern alias should be present for each library."); } } } }

True, however I was trying to make this work in a way that if global is present, then we don't need to emit ANY aliases in order for the designer file to compile. This is what the customer was expecting #4542 (comment). And it's how it used to work when we completely ignored Aliases. global means we don't need anything.

Whoops. I think I might have obscured the key concern by including additional asserts. It's the step of building the app project that concerns me. That fails right now for this PR because Resource.designer.cs contains exactly the kind of C# compiler conflict that aliases solve:

Build FAILED. "C:\Source\xamarin-android\bin\TestDebug\temp\CheckMultipleLibraryProjectReferenceAliasTrue\UnnamedProject\UnnamedProject.csproj" (Build;SignAndroidPackage target) (1) -> (CoreCompile target) -> Resources\Resource.designer.cs(28,4,28,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' Resources\Resource.designer.cs(29,4,29,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' Resources\Resource.designer.cs(30,4,30,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' Resources\Resource.designer.cs(31,4,31,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' 0 Warning(s) 4 Error(s)

Due to the following generated lines:

global::Library1.Resource.Drawable.Icon = global::UnnamedProject.Resource.Drawable.Icon; global::Library1.Resource.String.library_name = global::UnnamedProject.Resource.String.library_name; global::Library1.Resource.Drawable.Icon = global::UnnamedProject.Resource.Drawable.Icon; global::Library1.Resource.String.library_name = global::UnnamedProject.Resource.String.library_name;

That is, even though both libraries have global in the list of aliases, Resource.designer.cs must not use global for both libraries or there will be a conflict. It seems to me the simplest way to handle an arbitrary number of conflicting libraries (admittedly unusual, but might as well handle it if we can) is probably to use the first non-global alias for every libraries, which is the nice behavior this PR gives after removing && !aliasMetaData.Contains ("global").

So with the proposed change we will ALWAYS use the alias even if global is present. I guess the downside of that is users will see a change in their Resource.designer.cs when they upgrade.

Shame we don't have any data on how often the RootNamespace is changed to match that of an identical library :(

Oh yeah, that is an interesting trade-off to consider.

I suppose one little reassurance is that anyone who would get a change in Resource.designer.cs with this PR would already have a change in behavior in Xamarin.Android 10.2 (d16-5) and Xamarin.Android 10.3 (d16-6) due to the initial feature addition from d18c824. So only users updating all at once from Xamarin.Android 10.1 (d16-4) or lower up to Xamarin.Android 10.4 (d16-7) or higher could benefit from changes in this PR. If there hasn't been much feedback about the change yet, then it's probably not too terribly common in user projects. (Though admittedly, there's a chance that projects that are slower to update Visual Studio versions might also be more likely to use %(Aliases) metadata.)

Maybe, like the reporter of #4542 was mentioning, an idea could be to provide an extra way to control how aliases are used in Resource.designer.cs. I wonder if any other .NET project type has a way to handle this. I think code-behind for XAML wouldn't have the issue since XAML files have to specify the CLR namespaces. I think Resources.Designer.cs files for .resx files wouldn't have to handle it either since there is only one assembly involved. Maybe ASP.NET or WCF has a scenario?

brendanzagaeski · 2020-04-27T18:24:09Z

Review update

To help make it easy to skim to the latest status of the discussion on this PR at a glance, I'll add the results from running the CheckMultipleLibraryProjectReferenceAlias() test with the latest updated commit. The build failure due to the conflicting types remains:

Build FAILED.

"C:\Source\xamarin-android\bin\TestDebug\temp\CheckMultipleLibraryProjectReferenceAliasTrue\UnnamedProject\UnnamedProject.csproj" (Build;SignAndroidPackage target) (1) ->
(CoreCompile target) -> 
  Resources\Resource.designer.cs(28,4,28,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'
  Resources\Resource.designer.cs(29,4,29,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'
  Resources\Resource.designer.cs(30,4,30,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'
  Resources\Resource.designer.cs(31,4,31,29): error CS0433: The type 'Resource' exists in both 'Library1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and 'Library2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'

    0 Warning(s)
    4 Error(s)

dellis1972 · 2020-04-29T11:21:16Z

@brendanzagaeski I reworked this and added your test. Can you re-review please.

brendanzagaeski

Looks good to me on functionality now. Thanks for bearing with me on assessing the behavior for this!

If you get a quick moment to add the release note into a file named Documentation/release-notes/4554.md as part of this PR, that would be perfect. Feel free to copy and paste the candidate note if it looks alright to you:

#### Application and library build and deployment

  * *error CS1681: You cannot redefine the global extern alias* prevented
    building app projects successfully if the *.csproj* file included `global`
    in the `<Aliases></Aliases>` metadata element of a referenced
    Xamarin.Android class library.

Thanks again!

src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesigner.cs

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs

brendanzagaeski

Looks good! Thanks once more!

jonpryor · 2020-05-05T12:16:52Z

In progress commit message:

Fixes: https://github.com/xamarin/xamarin-android/issues/4542

Our previous fix d18c824c did not take into account that users might
need to add `global` to their `%(ProjectReferece.Aliases)` metadata,
e.g.

	<ProjectReferece>
	  <Aliases>global,Lib1A,Lib1B</Aliases>
	</ProjectReferece>

This then causes the following error:

	Error CS1681: You cannot redefine the global extern alias (CS1681)

because our `Resource.designer.cs` generation emitted:

	extern alias global;

To fix this we need to ignore the `global` entry if one is present.

jonpryor · 2020-05-05T12:18:21Z

src/Xamarin.Android.Build.Tasks/Utilities/ResourceDesignerImportGenerator.cs

+								string [] aliases = aliasMetaData.Split (new [] {','}, StringSplitOptions.RemoveEmptyEntries);
+								foreach (var alias in aliases) {
+									string aliasName = alias.Trim ();
+									if (string.Compare ("global", aliasName, StringComparison.OrdinalIgnoreCase) == 0)


I'm not certain that OrdinalIgnoreCase is appropriate here, as C# is a case sensitive language. I'm not sure it's "unreasonable" for someone to use Global as an alias.

…unless MSBuild metadata is by design case insensitive, in which case this is appropriate. I don't think that's the case (?).

Fixes dotnet#4542 Our previous fix d18c824 did not take into account that users might need to add `global` to their list of aliases in the `ProjectReferece` MetaData. This then causes the following error Error CS1681: You cannot redefine the global extern alias (CS1681) So to fix this we need to ignore the `global` entry if one is present.

…e. (#4554) Fixes: #4542 Our previous fix in d18c824 did not take into account that users might need to add `global` to their list of aliases in the `%(ProjectReference.Aliases)` metadata. This then causes the following error: Error CS1681: You cannot redefine the global extern alias (CS1681) Fix this by ignoring the `global` entry if one is present.

dellis1972 requested a review from jonathanpeppers as a code owner April 14, 2020 09:35

dellis1972 force-pushed the Issue4542 branch from 20f2220 to 15174f9 Compare April 14, 2020 09:48

dellis1972 mentioned this pull request Apr 14, 2020

"extern alias" added to auto generated Resource.designer.cs causes syntax errors #4542

Closed

jonathanpeppers approved these changes Apr 14, 2020

View reviewed changes

brendanzagaeski suggested changes Apr 14, 2020

View reviewed changes

brendanzagaeski reviewed Apr 15, 2020

View reviewed changes

dellis1972 force-pushed the Issue4542 branch 2 times, most recently from 1c0c83e to 6416838 Compare April 27, 2020 11:10

dellis1972 force-pushed the Issue4542 branch from 6416838 to a9e5ae6 Compare April 28, 2020 13:09

dellis1972 requested a review from brendanzagaeski April 29, 2020 11:20

brendanzagaeski suggested changes Apr 30, 2020

View reviewed changes

brendanzagaeski reviewed Apr 30, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesigner.cs Outdated Show resolved Hide resolved

brendanzagaeski reviewed Apr 30, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesigner.cs Outdated Show resolved Hide resolved

brendanzagaeski reviewed Apr 30, 2020

View reviewed changes

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs Outdated Show resolved Hide resolved

dellis1972 force-pushed the Issue4542 branch from a9e5ae6 to 3ecb335 Compare April 30, 2020 09:17

dellis1972 requested a review from jonpryor as a code owner April 30, 2020 09:17

dellis1972 force-pushed the Issue4542 branch from 3ecb335 to 84d1d99 Compare April 30, 2020 10:33

brendanzagaeski approved these changes May 1, 2020

View reviewed changes

jonpryor reviewed May 5, 2020

View reviewed changes

dellis1972 force-pushed the Issue4542 branch from 84d1d99 to 8503368 Compare May 5, 2020 12:40

jonpryor merged commit 65b5c61 into dotnet:master May 20, 2020

dellis1972 deleted the Issue4542 branch May 20, 2020 08:47

brendanzagaeski added this to the In Preview: 10.4 (d16-7) milestone Jun 23, 2020

github-actions bot locked and limited conversation to collaborators Jan 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Xamarin.Android.Build.Tasks] Ignore `global` Alias when emitting code. #4554

[Xamarin.Android.Build.Tasks] Ignore `global` Alias when emitting code. #4554

dellis1972 commented Apr 14, 2020 •

edited

Loading

brendanzagaeski commented Apr 14, 2020

brendanzagaeski left a comment

brendanzagaeski Apr 15, 2020

brendanzagaeski Apr 15, 2020 •

edited

Loading

dellis1972 Apr 16, 2020

brendanzagaeski Apr 17, 2020 •

edited

Loading

dellis1972 Apr 17, 2020

brendanzagaeski Apr 17, 2020 •

edited

Loading

dellis1972 Apr 20, 2020

brendanzagaeski Apr 21, 2020

brendanzagaeski commented Apr 27, 2020

dellis1972 commented Apr 29, 2020

brendanzagaeski left a comment

brendanzagaeski left a comment

jonpryor commented May 5, 2020

jonpryor May 5, 2020

[Xamarin.Android.Build.Tasks] Ignore global Alias when emitting code. #4554

[Xamarin.Android.Build.Tasks] Ignore global Alias when emitting code. #4554

Conversation

dellis1972 commented Apr 14, 2020 • edited Loading

brendanzagaeski commented Apr 14, 2020

brendanzagaeski left a comment

Choose a reason for hiding this comment

brendanzagaeski Apr 15, 2020

Choose a reason for hiding this comment

brendanzagaeski Apr 15, 2020 • edited Loading

Choose a reason for hiding this comment

dellis1972 Apr 16, 2020

Choose a reason for hiding this comment

brendanzagaeski Apr 17, 2020 • edited Loading

Choose a reason for hiding this comment

dellis1972 Apr 17, 2020

Choose a reason for hiding this comment

brendanzagaeski Apr 17, 2020 • edited Loading

Choose a reason for hiding this comment

dellis1972 Apr 20, 2020

Choose a reason for hiding this comment

brendanzagaeski Apr 21, 2020

Choose a reason for hiding this comment

brendanzagaeski commented Apr 27, 2020

dellis1972 commented Apr 29, 2020

brendanzagaeski left a comment

Choose a reason for hiding this comment

brendanzagaeski left a comment

Choose a reason for hiding this comment

jonpryor commented May 5, 2020

jonpryor May 5, 2020

Choose a reason for hiding this comment

[Xamarin.Android.Build.Tasks] Ignore `global` Alias when emitting code. #4554

[Xamarin.Android.Build.Tasks] Ignore `global` Alias when emitting code. #4554

dellis1972 commented Apr 14, 2020 •

edited

Loading

brendanzagaeski Apr 15, 2020 •

edited

Loading

brendanzagaeski Apr 17, 2020 •

edited

Loading

brendanzagaeski Apr 17, 2020 •

edited

Loading