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.

Sign up for GitHub

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

Remove custom types marking for types which XA does not own for netcore #5354

Merged
merged 13 commits into from
Dec 9, 2020
Merged

Remove custom types marking for types which XA does not own for netcore #5354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fcf5a47
Remove custom types marking for types which XA does not own for netcore
marek-safar Dec 2, 2020
6f2502c
Updated expected list of files
marek-safar Dec 2, 2020
848648e
PR review feedback
marek-safar Dec 3, 2020
e131184
More test updates
marek-safar Dec 3, 2020
13abcaa
More tests fixes
marek-safar Dec 3, 2020
e9be842
Move DataContractSerializer linker test to disabled category for netcore
marek-safar Dec 3, 2020
ee6c8df
Fix typo
radekdoulik Dec 4, 2020
932db4c
Undo accidentally pushed change
marek-safar Dec 4, 2020
894ea49
Update ILLink.LinkAttributes.xml
marek-safar Dec 4, 2020
e2898e4
Rename the source file to reflect class name
radekdoulik Dec 7, 2020
43169d7
Fix test after rebase
radekdoulik Dec 7, 2020
d30b2f0
Update comment
radekdoulik Dec 7, 2020
8383257
Fix build of test
radekdoulik Dec 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 7 additions & 26 deletions ....Sdk.ILLink/ApplyPreserveAttributeBase.cs → ...roid.Sdk.ILLink/ApplyPreserveAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
namespace Microsoft.Android.Sdk.ILLink
{

public abstract class ApplyPreserveAttributeBase : BaseSubStep {

// set 'removeAttribute' to true if you want the preserved attribute to be removed from the final assembly
protected abstract bool IsPreservedAttribute (ICustomAttributeProvider provider, CustomAttribute attribute, out bool removeAttribute);
public class ApplyPreserveAttribute : BaseSubStep {

public override SubStepTargets Targets {
get {
Expand Down Expand Up @@ -66,7 +63,7 @@ public override void ProcessEvent (EventDefinition @event)

void MarkMethodIfPreserved (MethodDefinition method)
{
foreach (var attribute in GetPreserveAttributes (method))
foreach (var attribute in GetPreserveAttributes (method))
MarkMethod (method, attribute);
}

Expand Down Expand Up @@ -126,6 +123,9 @@ void PreserveUnconditional (IMetadataTokenProvider provider)

void TryApplyPreserveAttribute (TypeDefinition type)
{
if (!type.HasCustomAttributes)
return;

foreach (var attribute in GetPreserveAttributes (type)) {
Annotations.Mark (type);

Expand All @@ -138,28 +138,9 @@ void TryApplyPreserveAttribute (TypeDefinition type)
}
}

List<CustomAttribute> GetPreserveAttributes (ICustomAttributeProvider provider)
static IEnumerable<CustomAttribute> GetPreserveAttributes (ICustomAttributeProvider provider)
{
List<CustomAttribute> attrs = new List<CustomAttribute> ();

if (!provider.HasCustomAttributes)
return attrs;

var attributes = provider.CustomAttributes;

for (int i = attributes.Count - 1; i >= 0; i--) {
var attribute = attributes [i];

bool remote_attribute;
if (!IsPreservedAttribute (provider, attribute, out remote_attribute))
continue;

attrs.Add (attribute);
if (remote_attribute)
attributes.RemoveAt (i);
}

return attrs;
return provider.CustomAttributes.Where (a => a.Constructor.DeclaringType.Name == "PreserveAttribute");
}
}
}
2 changes: 2 additions & 0 deletions src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\LinkerOptions.cs" />

<!--Steps that are upstreamed, these are OK to remove-->
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\ApplyPreserveAttribute.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\OutputStepWithTimestamps.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveCode.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveDynamicTypes.cs" />
Expand All @@ -36,6 +37,7 @@
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveRuntimeSerialization.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveTlsProvider.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\PreserveTypeConverters.cs" />
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\RemoveAttributes.cs" />

<!--TODO: Subclasses MarkStep-->
<Compile Remove="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\MonoDroidMarkStep.cs" />
Expand Down
1 change: 0 additions & 1 deletion src/Microsoft.Android.Sdk.ILLink/SetupStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ protected override void Process ()
subSteps2.Add (new PreserveApplications ());
subSteps2.Add (new PreserveRegistrations (cache));
subSteps2.Add (new PreserveJavaInterfaces ());
subSteps2.Add (new RemoveAttributes ());

InsertAfter (new FixAbstractMethodsStep (cache), "RemoveUnreachableBlocksStep");
InsertAfter (subSteps2, "RemoveUnreachableBlocksStep");
Expand Down
10 changes: 10 additions & 0 deletions src/Mono.Android/ILLink/ILLink.LinkAttributes.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<linker>
<assembly fullname="Mono.Android">
<type fullname="Android.Runtime.PreserveAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="Android.Runtime.IntDefinitionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
</linker>
2 changes: 2 additions & 0 deletions src/Mono.Android/Mono.Android.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@

<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' ">
<ProjectReference Include="..\..\external\Java.Interop\src\Java.Interop\Java.Interop.csproj" />

<EmbeddedResource Include="ILLink/ILLink.LinkAttributes.xml" />
</ItemGroup>

<ItemGroup>
Expand Down
24 changes: 0 additions & 24 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,38 +85,14 @@ public void CheckIncludedAssemblies ()
var expectedFiles = Builder.UseDotNet ?
new [] {
"Java.Interop.dll",
"Microsoft.Win32.Primitives.dll",
"Mono.Android.dll",
"System.Collections.NonGeneric.dll",
"System.ComponentModel.Primitives.dll",
"System.Console.dll",
"System.Formats.Asn1.dll",
"System.IO.Compression.Brotli.dll",
"System.IO.Compression.dll",
"System.IO.FileSystem.dll",
"System.Linq.Expressions.dll",
"System.Net.Http.dll",
"System.Net.NameResolution.dll",
"System.Net.NetworkInformation.dll",
"System.Net.Primitives.dll",
"System.Net.Quic.dll",
"System.Net.Security.dll",
"System.Net.Sockets.dll",
"System.ObjectModel.dll",
"System.Private.Uri.dll",
"System.Private.Xml.dll",
"System.Private.Xml.Linq.dll",
"System.Runtime.CompilerServices.Unsafe.dll",
"System.Runtime.InteropServices.RuntimeInformation.dll",
"System.Runtime.Numerics.dll",
"System.Security.Cryptography.Encoding.dll",
"System.Security.Cryptography.OpenSsl.dll",
"System.Security.Cryptography.X509Certificates.dll",
"System.Runtime.Serialization.Primitives.dll",
"System.Security.Cryptography.Algorithms.dll",
"System.Security.Cryptography.Primitives.dll",
"System.Text.RegularExpressions.dll",
"System.Threading.Channels.dll",
"System.Private.CoreLib.dll",
"System.Collections.Concurrent.dll",
"System.Collections.dll",
Expand Down
3 changes: 2 additions & 1 deletion tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MainActivityReplacement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,9 @@ protected override void OnCreate(Bundle bundle)
Android.Util.Log.Info(TAG, LinkTestLib.Bug21578.MulticastOption_ShouldNotBeStripped());
Android.Util.Log.Info(TAG, LinkTestLib.Bug21578.MulticastOption_ShouldNotBeStripped2());
Android.Util.Log.Info(TAG, LinkTestLib.Bug35195.AttemptCreateTable());
#if !NET
Android.Util.Log.Info(TAG, LinkTestLib.Bug36250.SerializeSearchRequestWithDictionary());

#endif
Android.Util.Log.Info(TAG, "All regression tests completed.");
}
}
Expand Down
17 changes: 11 additions & 6 deletions tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,6 @@ public class LinkModeFullClass {
return sr.ReadToEnd ();
},
},
new BuildItem.Source ("Bug36250.cs") {
TextContent = () => {
using (var sr = new StreamReader (typeof (InstallAndRunTests).Assembly.GetManifestResourceStream ("Xamarin.Android.Build.Tests.Resources.LinkDescTest.Bug36250.cs")))
return sr.ReadToEnd ();
},
},
new BuildItem.Source ("Bug35195.cs") {
TextContent = () => {
using (var sr = new StreamReader (typeof (InstallAndRunTests).Assembly.GetManifestResourceStream ("Xamarin.Android.Build.Tests.Resources.LinkDescTest.Bug35195.cs")))
Expand All @@ -345,6 +339,17 @@ public class LinkModeFullClass {
},
};

if (!Builder.UseDotNet) {
// DataContractSerializer is not trimming safe
// https://github.com/dotnet/runtime/issues/45559
lib2.Sources.Add (new BuildItem.Source ("Bug36250.cs") {
TextContent = () => {
using (var sr = new StreamReader (typeof (InstallAndRunTests).Assembly.GetManifestResourceStream ("Xamarin.Android.Build.Tests.Resources.LinkDescTest.Bug36250.cs")))
return sr.ReadToEnd ();
},
});
}

proj = new XamarinFormsAndroidApplicationProject () {
IsRelease = true,
AndroidLinkModeRelease = linkMode,
Expand Down
6 changes: 5 additions & 1 deletion tests/MSBuildDeviceIntegration/Tests/InstallTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,11 @@ public void InstallWithoutSharedRuntime ()
// "The Shared Runtime should not have been installed.");
var directorylist = GetContentFromAllOverrideDirectories (proj.PackageName);
StringAssert.Contains ($"{proj.ProjectName}.dll", directorylist, $"{proj.ProjectName}.dll should exist in the .__override__ directory.");
StringAssert.Contains ($"System.dll", directorylist, $"System.dll should exist in the .__override__ directory.");
if (Builder.UseDotNet)
StringAssert.Contains ($"System.Private.CoreLib.dll", directorylist, $"System.Private.CoreLib.dll should exist in the .__override__ directory.");
Copy link
Member

Choose a reason for hiding this comment

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

If this test fails, you might check for a different assembly like System.Linq.dll. I think System.Private.CoreLib.dll can be in a sub-directory like armeabi-v7a or x86.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

System.Private.CoreLib.dll is probably only reliable marker. I hope System.Linq.dll will be gone too next week ;-)

else
StringAssert.Contains ($"System.dll", directorylist, $"System.dll should exist in the .__override__ directory.");

StringAssert.Contains ($"Mono.Android.dll", directorylist, $"Mono.Android.dll should exist in the .__override__ directory.");
Assert.IsTrue (builder.Uninstall (proj), "unnstall should have succeeded.");
}
Expand Down