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
[One .NET] fix $(ApplicationId) and //manifest@package at the same time #6304
[One .NET] fix $(ApplicationId) and //manifest@package at the same time #6304
Conversation
using var stream = File.OpenRead (ManifestFile); | ||
using var reader = XmlReader.Create (stream); | ||
if (reader.MoveToContent () == XmlNodeType.Element) { | ||
var package = reader.GetAttribute ("package"); | ||
if (!string.IsNullOrEmpty (package)) { | ||
package = ManifestDocument.ReplacePlaceholders (ManifestPlaceholders, package); | ||
PackageName = AndroidAppManifest.CanonicalizePackageName (package); | ||
} else if (!string.IsNullOrEmpty (PackageName)) { |
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.
Instead of this else if
here, and the "duplicate" PackageName = …
statement between the two blocks, why not remove this else if
, and turn the else if
on line 64 into an if
?
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.
I moved it around, should be better now. I removed a CanonicalizePackageName()
call as well, so it isn't called twice.
Fixes: xamarin#6302 If you try to use both `$(ApplicationId)` and `@package` in `AndroidManifest.xml` at the same time, you hit errors during deployment: Xamarin.Android.Common.Debugging.targets(611,5): error XA0132: The package was not installed. Please check you do not have it installed under any other user. If the package does show up on the device, try manually uninstalling it then try again. You should be able to uninstall the app via the Settings app on the device. From the spec, the `AndroidManifest.xml` is *supposed* to be preferred. I found the `<GetAndroidPackageName/>` MSBuild task is accidentally preferring the `$(ApplicationId)` MSBuild property. I reordered the logic and only use the property when no value is found in the `AndroidManifest.xml`. I could reproduce the issue in a test, which now passes.
32a7f5c
to
1095a3e
Compare
Fixes: #6302 If you try to use both the `$(ApplicationId)` MSBuild property and the `/manifest/@package` attribute within `AndroidManifest.xml` at the same time, you hit errors during deployment: Xamarin.Android.Common.Debugging.targets(611,5): error XA0132: The package was not installed. Please check you do not have it installed under any other user. If the package does show up on the device, try manually uninstalling it then try again. You should be able to uninstall the app via the Settings app on the device. From the spec, the `AndroidManifest.xml` value is *supposed* to be preferred. I found that the `<GetAndroidPackageName/>` MSBuild task is accidentally preferring the `$(ApplicationId)` MSBuild property. I reordered the logic and only use the property when no value is found in the `AndroidManifest.xml`. I could reproduce the issue in a test, which now passes.
Fixes: #6302 If you try to use both the `$(ApplicationId)` MSBuild property and the `/manifest/@package` attribute within `AndroidManifest.xml` at the same time, you hit errors during deployment: Xamarin.Android.Common.Debugging.targets(611,5): error XA0132: The package was not installed. Please check you do not have it installed under any other user. If the package does show up on the device, try manually uninstalling it then try again. You should be able to uninstall the app via the Settings app on the device. From the spec, the `AndroidManifest.xml` value is *supposed* to be preferred. I found that the `<GetAndroidPackageName/>` MSBuild task is accidentally preferring the `$(ApplicationId)` MSBuild property. I reordered the logic and only use the property when no value is found in the `AndroidManifest.xml`. I could reproduce the issue in a test, which now passes.
Fixes: #6302
If you try to use both
$(ApplicationId)
and@package
inAndroidManifest.xml
at the same time, you hit errors duringdeployment:
From the spec, the
AndroidManifest.xml
is supposed to be preferred.I found the
<GetAndroidPackageName/>
MSBuild task is accidentallypreferring the
$(ApplicationId)
MSBuild property. I reordered thelogic and only use the property when no value is found in the
AndroidManifest.xml
.I could reproduce the issue in a test, which now passes.