Skip to content
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

Bump xamarin-android-tools and StrongName Xamarin.Android.Build.Tasks #6116

Merged
merged 11 commits into from
Aug 11, 2021

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Jul 23, 2021

We need to StrongName our MSBuild assemblies. This is so we can have different versions side by side (Legacy and .net 6). If we do not StrongName them, Visual Studio ends up trying to use the first loaded assembly for both Legacy and .net 6. This will result
in errors about missing properties for example.

insert sample here

StrongNaming is the best way to get around this issue. A few notes though.

  1. pdb2mdb.exe is now ILRemaped into Xamarin.Android.Build.Tasks since it is not StrongNamed. This would require changes upstream.
  2. The Xamarin.Android.Build.Tasks project itself is NOT StrongNamed. This is because pdb2mdb.exe is not either. We handle the StrongNaming via the ILRemap process later in the build. The end result is a StrongNamed Xamarin.Android.Build.Tasks.dll.

@dellis1972
Copy link
Contributor Author

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should change this to bump things without adding the strong naming yet?

There are a few more that would need to be strong named:

1) Invalid : C:\a\_work\1\s\bin\TestRelease\net472\Xamarin.Android.Build.Tests.dll
Unable to load one or more of the requested types. Retrieve the LoaderExceptions property for more information.
  ----> Could not load file or assembly 'Xamarin.ProjectTools, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. A strongly-named assembly is required. (Exception from HRESULT: 0x80131044)
  ----> Could not load file or assembly 'Xamarin.ProjectTools, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. A strongly-named assembly is required. (Exception from HRESULT: 0x80131044)
  ----> Could not load file or assembly 'Xamarin.ProjectTools, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. A strongly-named assembly is required. (Exception from HRESULT: 0x80131044)
  ----> Could not load file or assembly 'Xamarin.Android.Build.Tasks, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. A strongly-named assembly is required. (Exception from HRESULT: 0x80131044)

@jonpryor
Copy link
Member

Context: #6116

@dellis1972
Copy link
Contributor Author

Ok Strong naming is needs to be added to

Java.Interop.Tools.Cecil
pdb2mdb

@dellis1972 dellis1972 added the do-not-merge PR should not be merged. label Jul 28, 2021
@dellis1972 dellis1972 marked this pull request as draft July 28, 2021 10:51
@dellis1972
Copy link
Contributor Author

Needs https://github.com/xamarin/monodroid/pull/1214 to pass some of the Task based Unit tests.

Comment on lines 414 to 418
<PropertyGroup>
<AssemblyVersion>$(ProductVersion).$(PackVersionCommitCount)</AssemblyVersion>
<FileVersion>$(AssemblyVersion)</FileVersion>
<InformationalVersion>$(AssemblyVersion)</InformationalVersion>
</PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recently found you can just set $(Version) and it sets all three of these.

Do we also need to setup the version on our other assemblies?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doh. Now we are gonna have to go through all the dependencies and make sure they all have versions set. Including Java.Interop/androidtools/xamarin-android-tools etc 🤦

…rgets

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>
@dellis1972 dellis1972 marked this pull request as ready for review August 6, 2021 21:03
@dellis1972 dellis1972 removed the do-not-merge PR should not be merged. label Aug 7, 2021
@dellis1972
Copy link
Contributor Author

@jonathanpeppers I think this is good to go. We should deal with the version numbering issue in a different set of PR's I think. Since it will require some thought on how to match the versions in say Java.Interop with XA (or if we even should). But we need to put in place something which auto increments them I think, otherwise it's just a pain to maintain.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assembly attributes look good now:

[assembly: AssemblyFileVersion("11.4.99.134")]
[assembly: AssemblyInformationalVersion("11.4.99.134")]
[assembly: AssemblyVersion("11.4.99.134")]

@dellis1972 dellis1972 changed the title Bump xamarin-android-tools Bump xamarin-android-tools and StrongName Xamarin.Android.Build.Tasks Aug 10, 2021
@jonpryor
Copy link
Member

Commit message for review:

Bump to xamarin/monodroid/main@fb0d5021; StrongName Xamarin.Android.Build.Tasks (#6116)

Context: https://discord.com/channels/732297728826277939/732297837953679412/865316625950572554
Context: https://discord.com/channels/732297728826277939/732297837953679412/869307441601470484

Changes: http://github.com/xamarin/monodroid/compare/b359d3b43774ff031add3d7a4949483052957db3...fb0d502139bc222d4e9ad82f912b1c6f1d4ca34e

  * xamarin/monodroid@fb0d50213: StrongName Xamarin.Android.Build.Debugging.Tasks (#1214)
  * xamarin/monodroid@71ac9a81b: Bump to xamarin/xamarin-android/main@4ea37883 (#1220)

Occasionally when:

 1. Building on Windows within Visual Studio, with
 2. "Legacy" Xamarin.Android installed, and
 3. A preview .NET for Android (net6.0 + Android) workload installed

the build may fail:

	C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\30.0.100-preview.6.62\tools\Xamarin.Android.Common.targets(512,7): error MSB4064: The "ManifestPlaceholders" parameter is not supported by the "GetAndroidPackageName" task loaded from assembly: Xamarin.Android.Build.Tasks, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null from the path: D:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Xamarin\Android\Xamarin.Android.Build.Tasks.dll. Verify that the parameter exists on the task, the <UsingTask> points to the correct assembly, and it is a settable public instance property.
	C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\30.0.100-preview.6.62\tools\Xamarin.Android.Common.targets(509,3): error MSB4063: The "GetAndroidPackageName" task could not be initialized with its input parameters.

The root cause of the problem is that *both* "Legacy" Xamarin.Android
and .NET SDK for Android contain a non-strong-named
`Xamarin.Android.Build.Tasks.dll`, and if a Project Solution `.sln`
contains Projects `.csproj` which reference both SDKs, then
`Xamarin.Android.Build.Tasks.dll` will only be loaded *once*, which
will be incompatible with the other.  In the above error message,
the legacy `Xamarin.Android.Build.Tasks.dll` was loaded first,
causing the build to fail when trying to build the .NET SDK for Android
project.

There are three ways to solve this:

 1. Maintain API compatibility between Legacy & .NET 6+
 2. Use different assembly names between "Legacy" and .NET 6+, or
 3. [Strong-name][0] all MSBuild-related assemblies.

(1) is a non-starter; new or different functionality is introduced as
part of .NET SDK for Android.  The assemblies can't be API compatible.

We could solve via (2) by renaming the .NET SDK for Android assembly
to e.g. `Microsoft.Android.Build.Tasks.dll`, but this only delays the
problem; come .NET 7, we'll still have the same assembly name for two
different installation locations (.NET 6, .NET 7), and they won't be
compatible with each other.

Which leaves (3): we strong-name our MSBuild assemblies.
Strong-naming allows us to have different versions installed
"side by side" (Legacy and .net 6) in separate directories.
Strong-naming is the best solution to this issue.

A few notes though.

 1. `pdb2mdb.exe` is now ILRepacked into `Xamarin.Android.Build.Tasks`
    via [`ILRepack`][1], as it is not Strong-named.  See also 610ade7f.

 2. The `Xamarin.Android.Build.Tasks.csproj` project itself is *NOT*
    StrongNamed. This is because `pdb2mdb.exe` is not either.
    We handle the StrongNaming via the `ILRepacker` target later in
    the build.  The end result is a Strong-named 
    `Xamarin.Android.Build.Tasks.dll`.

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>

[0]: https://docs.microsoft.com/en-us/dotnet/standard/assembly/strong-named
[1]: https://www.nuget.org/packages/ILRepack/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants