-
Notifications
You must be signed in to change notification settings - Fork 533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
.net8 Android not building because Resources from resx not found. #9057
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
...amarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Resource.Designer.targets
Show resolved
Hide resolved
dellis1972
force-pushed
the
Issue_maui_19117
branch
2 times, most recently
from
July 2, 2024 15:27
690f387
to
aad2d2c
Compare
jonpryor
reviewed
Jul 2, 2024
jonpryor
reviewed
Jul 2, 2024
jonpryor
reviewed
Jul 2, 2024
jonpryor
reviewed
Jul 2, 2024
jonpryor
reviewed
Jul 2, 2024
...amarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Resource.Designer.targets
Outdated
Show resolved
Hide resolved
jonpryor
reviewed
Jul 2, 2024
...amarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Resource.Designer.targets
Outdated
Show resolved
Hide resolved
dellis1972
force-pushed
the
Issue_maui_19117
branch
from
July 3, 2024 07:53
aad2d2c
to
a09e672
Compare
jonathanpeppers
approved these changes
Jul 8, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise, LGTM.
dellis1972
force-pushed
the
Issue_maui_19117
branch
from
July 9, 2024 11:44
3d58890
to
093a145
Compare
dellis1972
force-pushed
the
Issue_maui_19117
branch
from
July 16, 2024 12:33
093a145
to
756176b
Compare
jonathanpeppers
approved these changes
Jul 18, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM. 👍 I reran some test lanes to check if they will be green.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/InlineData.cs
Show resolved
Hide resolved
dellis1972
force-pushed
the
Issue_maui_19117
branch
2 times, most recently
from
August 6, 2024 15:06
eb3b038
to
cf55b7b
Compare
dellis1972
force-pushed
the
Issue_maui_19117
branch
3 times, most recently
from
September 5, 2024 15:31
2d0b75c
to
4050b08
Compare
jonathanpeppers
approved these changes
Sep 6, 2024
jonpryor
reviewed
Sep 10, 2024
...amarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Resource.Designer.targets
Outdated
Show resolved
Hide resolved
jonpryor
reviewed
Sep 10, 2024
src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesignerIntermediateClass.cs
Outdated
Show resolved
Hide resolved
dellis1972
force-pushed
the
Issue_maui_19117
branch
from
September 11, 2024 15:45
4050b08
to
e9c2803
Compare
dellis1972
force-pushed
the
Issue_maui_19117
branch
from
October 8, 2024 12:08
e9c2803
to
b22c3a5
Compare
Fixes https://github.com/dotnet/maui/issues/19117 With the release of the new Resource Designer assembly we purposely removed the existing legacy `Resource.designer.cs` file from the `@(Compile)` ItemGroup. This is so that we did not get any clashes with the new system. Unfortunately the name `Resource.designer.cs` is also used by developers as the name for the generated class when using `.resx` files. As a result we got the above bug report because the class was disappearing. ``` error CS0234: The type or namespace name 'Resources' does not exist in the namespace 'StringResourcesBug' (are you missing an assembly reference?) ``` It turns out that the `_RemoveLegacyDesigner` was being a bit too aggressive with its removal of files. Because it was matching on filename only it was removing files which had nothing to do with Android Resources. This would cause the above error. The fix in this case is to check for additional metadata. In the case of `.resx` file designers they typically have a `DependentUpon` metadata to signal that they are generated from the `.resx`. So we should check this metadata to make sure it is NOT set before removing a file. A new unit test was added to test this fix and it worked. But it did show up one other issue. In certain cases we would end up with two `Resource` classes in the same namespace. This would then cause the following build error. ``` error CS0260: Missing partial modifier on declaration of type 'Resource'; another partial declaration of this type exists ``` This is because the two classes had different modifiers. We could also get an error if the access modifiers are different (public vs internal). So lets add a new MSBuild property `AndroidResourceDesignerClassModifier`. This property defaults to `public`, but can be overridden by the user to control the access modifier of the generated class.
It causes ``` __Microsoft.Android.Resource.Designer.cs(15,3,15,16): error CS0579: Duplicate 'GeneratedCode' attribute ```
dellis1972
force-pushed
the
Issue_maui_19117
branch
from
October 9, 2024 08:03
6b02e92
to
b48b3aa
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #9163
With the release of the new Resource Designer assembly we purposely removed the existing legacy
Resource.designer.cs
file from the@(Compile)
ItemGroup. This is so that we did not get any clashes with the new system.Unfortunately the name
Resource.designer.cs
is also used by developers as the name for the generated class when using.resx
files. As a result we got the above bug report because the class was disappearing.It turns out that the
_RemoveLegacyDesigner
was being a bit too aggressive with its removal of files. Because it was matching on filename only it was removing files which had nothing to do with Android Resources. This would cause the above error.The fix in this case is to check for additional metadata. In the case of
.resx
file designers they typically have aDependentUpon
metadata to signal that they are generated from the.resx
. So we should check this metadata to make sure it is NOT set before removing a file.A new unit test was added to test this fix and it worked. But it did show up one other issue. In certain cases we would end up with two
Resource
classes in the same namespace. This would then cause the following build error.This is because the two classes had different modifiers. We could also get an error if the access modifiers are different (public vs internal). So lets add a new MSBuild property
AndroidResourceDesignerClassModifier
. This property defaults topublic
, but can be overridden by the user to control the access modifier of the generated class.Also we have to remove the
GeneratedCode
Attribute usage on the generatedResource
class. Since it can conflict with the one that is auto added to theResource.designer.cs
generated by theresx
code generator. If we leave this in place we get the following error.Unfortunately the
GeneratedCode
Attribute [1] is marked asAllowMultiple=false
and in this case there is no way to know if it exists on the class already as its apartial
class.[1] https://learn.microsoft.com/en-us/dotnet/api/system.codedom.compiler.generatedcodeattribute?view=net-8.0