Skip to content

Conversation

vitek-karas
Copy link
Member

@vitek-karas vitek-karas commented Aug 23, 2024

E.g. Event source support is disabled by default by the iOS and Android SDKs, and ideally, we should match that behavior as closely as possible in general runtime functionality.

This PR matches the feature switch defaults to the ones in iOS/Android SDKs.

Event source support is disabled by default by the iOS and Android SDKs, and ideally the sample app should match that behavior as closely as possible in general runtime functionality.

The sample app is also used to measure app size, and so it should be representative of the defaults users will get when using SDK.
@vitek-karas vitek-karas added test-enhancement Improvements of test source code os-android os-ios Apple iOS labels Aug 23, 2024
@vitek-karas vitek-karas added this to the 10.0.0 milestone Aug 23, 2024
@vitek-karas vitek-karas self-assigned this Aug 23, 2024
@ghost ghost added the area-VM-meta-mono label Aug 23, 2024
Copy link
Contributor

Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

@akoeplinger
Copy link
Member

We already have this code which sets the ~same default for testing:

<!-- Reduce library test app size by trimming framework library features -->
<DebuggerSupport Condition="'$(DebuggerSupport)' == '' and '$(Configuration)' != 'Debug'">false</DebuggerSupport>
<EnableUnsafeUTF7Encoding Condition="'$(EnableUnsafeUTF7Encoding)' == ''">false</EnableUnsafeUTF7Encoding>
<EventSourceSupport Condition="'$(EventSourceSupport)' == ''">false</EventSourceSupport>
<HttpActivityPropagationSupport Condition="'$(HttpActivityPropagationSupport)' == ''">false</HttpActivityPropagationSupport>
<UseSystemResourceKeys Condition="'$(UseSystemResourceKeys)' == ''">false</UseSystemResourceKeys>
<UseNativeHttpHandler Condition="'$(UseNativeHttpHandler)' == ''">false</UseNativeHttpHandler>

Would be nice to move this to extract this into a common .targets that we can import into the samples apps.
Browser/wasm sample apps have the same issue.

@matouskozak
Copy link
Member

We already have this code which sets the ~same default for testing:

<!-- Reduce library test app size by trimming framework library features -->
<DebuggerSupport Condition="'$(DebuggerSupport)' == '' and '$(Configuration)' != 'Debug'">false</DebuggerSupport>
<EnableUnsafeUTF7Encoding Condition="'$(EnableUnsafeUTF7Encoding)' == ''">false</EnableUnsafeUTF7Encoding>
<EventSourceSupport Condition="'$(EventSourceSupport)' == ''">false</EventSourceSupport>
<HttpActivityPropagationSupport Condition="'$(HttpActivityPropagationSupport)' == ''">false</HttpActivityPropagationSupport>
<UseSystemResourceKeys Condition="'$(UseSystemResourceKeys)' == ''">false</UseSystemResourceKeys>
<UseNativeHttpHandler Condition="'$(UseNativeHttpHandler)' == ''">false</UseNativeHttpHandler>

Would be nice to move this to extract this into a common .targets that we can import into the samples apps. Browser/wasm sample apps have the same issue.

What kind of feature switches would Browser/wasm sample apps want to turn off/on?

For mobile sample apps, I think it would make sense to set an intersection of Android https://github.com/dotnet/android/blob/119d46c3a424ffeb3721718043616bfb10f5622f/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets#L109-L120 and macios https://github.com/xamarin/xamarin-macios/blob/92092f67fdc0979b9db95bdef55a9c8280d383e4/dotnet/targets/Xamarin.Shared.Sdk.targets#L121-L136 settings.

E.g.:

<EnableUnsafeBinaryFormatterSerialization Condition="'$(EnableUnsafeBinaryFormatterSerialization)' == ''">false</EnableUnsafeBinaryFormatterSerialization>
<EnableUnsafeUTF7Encoding Condition="'$(EnableUnsafeUTF7Encoding)' == ''">false</EnableUnsafeUTF7Encoding>
<EventSourceSupport Condition="'$(EventSourceSupport)' == ''">false</EventSourceSupport>
<HttpActivityPropagationSupport Condition="'$(HttpActivityPropagationSupport)' == ''">false</HttpActivityPropagationSupport>
<InvariantGlobalization Condition="'$(InvariantGlobalization)' == ''">false</InvariantGlobalization>
<StartupHookSupport Condition="'$(StartupHookSupport)' == ''">false</StartupHookSupport>
<UseNativeHttpHandler Condition="'$(UseNativeHttpHandler)' == ''">true</UseNativeHttpHandler>

@ivanpovazan
Copy link
Member

@stephentoub
Copy link
Member

@vitek-karas, should this PR be merged or closed?

@davidnguyen-tech davidnguyen-tech self-assigned this Aug 15, 2025
@Copilot Copilot AI review requested due to automatic review settings August 15, 2025 12:41
Copilot

This comment was marked as outdated.

@davidnguyen-tech davidnguyen-tech changed the title Disable event source in mobile sample apps [mobile] Match feature switch defaults with SDK Aug 15, 2025
Copilot

This comment was marked as outdated.

@davidnguyen-tech
Copy link
Member

davidnguyen-tech commented Aug 15, 2025

From what I understood, the goal here was to match feature switch defaults to the ones in iOS/Android SDKs.

For that reason, I chose to copy-paste the SDKs defaults into AppleBuild.props and AndroidBuild.props, as it should be easier to maintain than using CommonMobileBuilds.props that was proposed earlier.

EDIT: CommonMobileBuilds.props is actually Mono-specific https://github.com/dotnet/runtime/pull/111545/files#r1933751217

@davidnguyen-tech
Copy link
Member

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidnguyen-tech
Copy link
Member

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidnguyen-tech
Copy link
Member

davidnguyen-tech commented Sep 24, 2025

Observation from the runtime pipeline: Some mobile tests start to fail because of some FileNotFound exceptions.

  • android-x64 Release AllSubsets_CoreCLR: `System.IO.FileNotFoundException:
...
[FAIL] System.Net.WebSockets.Client.Tests.CloseTest_Invoker_Http2Loopback.CloseOutputAsync_ServerInitiated_CanSend(useSsl: False)
                 09-24 10:17:12.231  4138  7982 I DOTNET  : System.IO.FileNotFoundException : Could not load file or assembly 'Mono.Android, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
                 09-24 10:17:12.231  4138  7982 I DOTNET  :    at System.Reflection.RuntimeAssembly.<InternalLoad>g____PInvoke|48_0(NativeAssemblyNameParts* __pAssemblyNameParts_native, ObjectHandleOnStack __requestingAssembly_native, StackCrawlMarkHandle __stackMark_native, Int32 __throwOnFileNotFound_native, ObjectHandleOnStack __assemblyLoadContext_native, ObjectHandleOnStack __retAssembly_native)
...
  • maccatalyst-x64 Release AllSubsets_Mono:
...
[FAIL] System.IO.Tests.FileNotFoundExceptionTests.ToStringTest   Test name: System.IO.Tests.FileNotFoundExceptionTests.ToStringTest
[05:14:39] dbug: Assembly:  [System.Runtime.Tests, Version=10.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51]
[05:14:39] dbug: 2025-09-24 05:14:22.052 Df System.Runtime.Tests[11650:30122]    Exception messages: Assert.Contains() Failure: Sub-string not found
[05:14:39] dbug: String:    "System.IO.FileNotFoundException: this is "···
[05:14:39] dbug: Not found: ": 'file.txt'"   Exception stack traces:    at System.IO.Tests.FileNotFoundExceptionTests.ToStringTest()
...
  • maccatalyst-arm64 Release AllSubsets_Mono: similar as x64

@kotlarmilos
Copy link
Member

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-VM-meta-mono os-android os-ios Apple iOS test-enhancement Improvements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants