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

Consider setting NullabilityInfoContextSupport=true in EF's NuGet package #27474

Closed
eerhardt opened this issue Feb 19, 2022 · 20 comments · Fixed by #27916
Closed

Consider setting NullabilityInfoContextSupport=true in EF's NuGet package #27474

eerhardt opened this issue Feb 19, 2022 · 20 comments · Fixed by #27916
Assignees
Labels
area-global closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@eerhardt
Copy link
Member

When using EF in a .NET Maui Android/iOS application, developers will get a runtime error because .NET MAUI sets NullabilityInfoContextSupport=false here:

https://github.com/xamarin/xamarin-android/blob/c80dfff7a3183e21d356d2c5835aa0821fd9bd90/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets#L95
and
https://github.com/xamarin/xamarin-macios/blob/e25163f573d31b28fa60f000ce084b8cdb0ca697/dotnet/targets/Xamarin.Shared.Sdk.targets#L136

Since this setting is set to false, when EF tries to create a NullabilityInfo object, an exception is thrown here:

PropertyInfo propertyInfo => nullabilityInfoContext.Create(propertyInfo),
FieldInfo fieldInfo => nullabilityInfoContext.Create(fieldInfo),

02-18 17:55:25.483 14402 14402 E AndroidRuntime: Process: com.companyname.mauioptimisertest, PID: 14402
02-18 17:55:25.483 14402 14402 E AndroidRuntime: android.runtime.JavaProxyThrowable: System.InvalidOperationException: NullabilityInfoContext is not supported in the current application because 'System.Reflection.NullabilityInfoContext.IsSupported' is set to false. Set the MSBuild Property 'NullabilityInfoContextSupport' to true in order to enable it.
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at System.Reflection.NullabilityInfoContext.EnsureIsSupported()
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at System.Reflection.NullabilityInfoContext.Create(PropertyInfo propertyInfo)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.NonNullableConventionBase.IsNonNullableReferenceType(IConventionModelBuilder modelBuilder, MemberInfo memberInfo)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.NonNullableReferencePropertyConvention.Process(IConventionPropertyBuilder propertyBuilder)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.NonNullableReferencePropertyConvention.ProcessPropertyAdded(IConventionPropertyBuilder propertyBuilder, IConventionContext`1 context)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ImmediateConventionScope.OnPropertyAdded(IConventionPropertyBuilder propertyBuilder)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.OnPropertyAddedNode.Run(ConventionDispatcher dispatcher)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.DelayedConventionScope.Run(ConventionDispatcher dispatcher)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ConventionBatch.Run()
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ConventionBatch.Dispose()
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ImmediateConventionScope.OnModelInitialized(IConventionModelBuilder modelBuilder)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.OnModelInitialized(IConventionModelBuilder modelBuilder)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Metadata.Internal.Model..ctor(ConventionSet conventions, ModelDependencies modelDependencies, ModelConfiguration modelConfiguration)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.ModelBuilder..ctor(ConventionSet conventions, ModelDependencies modelDependencies, ModelConfiguration modelConfiguration)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.ModelConfigurationBuilder.CreateModelBuilder(ModelDependencies modelDependencies)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, ModelDependencies modelDependencies)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, ModelCreationDependencies modelCreationDependencies, Boolean designTime)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel(Boolean designTime)
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
02-18 17:55:25.483 14402 14402 E AndroidRuntime:    at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__8_4(IServiceProvider p)

We should consider defaulting NullabilityInfoContextSupport=true in EF's NuGet package, so when a .NET Maui app starts using EF, they automatically get this support turned on and their app works out of the box.

Repro steps

  1. dotnet new maui
  2. Add <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="7.0.0-preview.1.22076.6" />
  3. Change MainPage.xaml.cs to:
using Microsoft.EntityFrameworkCore;

namespace MauiOptimiserTest;

public partial class MainPage : ContentPage
{

    public MainPage()
    {
        InitializeComponent();

        var dbpath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "test.db");
        using (var context = new TestContext(dbpath))
        {
            context.Database.EnsureCreated();
            var records = context.MyRecords.ToList();
            BindingContext = records;
        }
    }

}

public class MyRecord
{
    public int Id { get; set; }
    public string Name { get; set; }
}

public class TestContext : DbContext
{
    public DbSet<MyRecord> MyRecords { get; set; }

    public TestContext() : this(":memory:")
    {
    }

    public TestContext(string path)
    {
        sqliteConnectionString.DataSource = path;
    }

    private readonly Microsoft.Data.Sqlite.SqliteConnectionStringBuilder sqliteConnectionString = new();
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        base.OnConfiguring(optionsBuilder);

        optionsBuilder.UseSqlite(sqliteConnectionString.ToString());
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);

        modelBuilder.Entity<MyRecord>().HasData(new MyRecord { Id = 1, Name = "Apple" });
        modelBuilder.Entity<MyRecord>().HasData(new MyRecord { Id = 2, Name = "Banana" });
        modelBuilder.Entity<MyRecord>().HasData(new MyRecord { Id = 3, Name = "Coconut" });
    }
}
  1. dotnet build -f net6.0-android -r android-arm64 -t:Run

The app will crash with the above exception when it is loaded.

@roji
Copy link
Member

roji commented Feb 19, 2022

@eerhardt thanks for this... Can you provide a bit more context on this? Is this simply a property we'd add to our csproj to get rid of the problem? Does it have any downsides/potential issues? Any reason why this standard .NET feature isn't just available by default and needs this opt-in?

@eerhardt
Copy link
Member Author

Can you provide a bit more context on this?

Sure. Check out dotnet/runtime#55860 and dotnet/runtime#56475.

Some app models (like Blazor WASM and Maui) want to produce as small of applications as possible. One of the ways to make it smaller is to trim out unnecessary attributes, like the Nulalble attributes: AllowNull, MaybeNullWhen, Nullable, etc. In order to support that we added a "feature switch" in .NET 6 that allows apps to trim these nullable attributes. When that switch is set to NullabilityInfoContextSupport=false, calling NullabilityInfoContext with throw the exception listed above in the OP.

By default, Maui iOS and Android applications have this setting NullabilityInfoContextSupport=false. So by default, if you create a Maui iOS or Android app and use EF, you get this exception.

To workaround this, the developer can explicitly set NullabilityInfoContextSupport=true in their .csproj.

My proposal here is that just by referencing the EntityFramework NuGet packages, we should consider setting NullabilityInfoContextSupport=true, which will override the default value set in Maui and Blazor WASM projects.

Is this simply a property we'd add to our csproj to get rid of the problem?

It is a property that would need to be set in a .props/.targets file in your NuGet package. This would then set the property in consumer's projects.

Does it have any downsides/potential issues?

The only downside I can see is that when you aren't in a Maui or Blazor WASM app, and reference the EF package, you would get an extra property that is explicitly setting the value to true when the default is already true in server/desktop apps.

Any reason why this standard .NET feature isn't just available by default and needs this opt-in?

It is available by default in a lot of app models. It is just turned off in app models that are concerned with app size.

@roji
Copy link
Member

roji commented Feb 23, 2022

Thanks for the info @eerhardt, makes sense.

However, if the intent here is to allow WASM/MAUI apps to reduce size, wouldn't it be better to allow EF Core to function properly when NullabilityInfoContextSupport is set to false? In other words, instead of forcing NullabilityInfoContextSupport=true on user applications (and thereby increasing their size), wouldn't it be better for EF Core to not invoke NullabilityInfoContext in the first place (since it doesn't have to in order to work)?

I guess we could do a manual check at runtime to see whether the nullability attributes exist or have been trimmed, and based on that avoid calling into NullabilityInfoContext. However, would it be better to expose a static (cached) bool check on NullabilityInfoContext itself, which libraries such as EF Core could use to know whether NullabilityInfoContext is usable?

@ajcvickers
Copy link
Contributor

@roji

since it doesn't have to in order to work

Model building needs the attributes, right?

@roji
Copy link
Member

roji commented Feb 28, 2022

@ajcvickers oh sure - I was thinking about when the user hasn't enabled NRTs in their project.

But thinking about it again, I think you're right: in order to support scenarios where the user does want NRTs, the attributes would have to be conserved, and so we're have to have NullabilityInfoContextSupport=true in any case. I'll do that.

@roji roji added the type-bug label Feb 28, 2022
@roji roji self-assigned this Feb 28, 2022
@ajcvickers
Copy link
Contributor

@eerhardt Is this something we should do for 6.0 a patch release, given that it impacts the Maui experience?

/cc @Pilchie

@eerhardt
Copy link
Member Author

eerhardt commented Mar 1, 2022

Is this something we should do for 6.0 a patch release, given that it impacts the Maui experience?

I'm not 100% sure.

The bigger issue that I ran into was fixed with #27099. I think changes like that would be much more impactful to backport to 6.0 if we were open to fixing the Maui experience. The reason being that #27099 is much harder to diagnose and workaround for the developer. This issue gives the developer a good error message and tells them exactly what to do to fix it:

Set the MSBuild Property 'NullabilityInfoContextSupport' to true in order to enable it.

Without #27099 you get an error $"Could not find method '{name}' on type '{type}'" deep in the EF code:

image

@roji
Copy link
Member

roji commented Mar 1, 2022

@eerhardt we did patch #27097, which is a much smaller/lower-risk "version" of #27099, in order to unblock people trimming on 6.0 - but that only took care of SQL Server. #27311 was filed for this exact problem with SQLite, we could do the same there. Reopening that issue to discuss.

@eerhardt
Copy link
Member Author

eerhardt commented Mar 1, 2022

Thanks! The customer I was working with was using SQLite. I just used 7.0-preview1 of EF to get those changes.

@ajcvickers ajcvickers added this to the 6.0.x milestone Mar 7, 2022
@ajcvickers
Copy link
Contributor

Note from triage: consider patching because it is low risk and will improve MAUI experience.

roji added a commit to roji/efcore that referenced this issue Apr 29, 2022
To make NRT-based model building work correctly in aggressively
trimmed applications (e.g. MAUI).

Fixes dotnet#27474
roji added a commit to roji/efcore that referenced this issue Apr 29, 2022
To make NRT-based model building work correctly in aggressively
trimmed applications (e.g. MAUI).

Fixes dotnet#27474
roji added a commit to roji/efcore that referenced this issue Apr 29, 2022
To make NRT-based model building work correctly in aggressively
trimmed applications (e.g. MAUI).

Fixes dotnet#27474
roji added a commit to roji/efcore that referenced this issue Apr 29, 2022
To make NRT-based model building work correctly in aggressively
trimmed applications (e.g. MAUI).

Fixes dotnet#27474
roji added a commit to roji/efcore that referenced this issue Apr 29, 2022
To make NRT-based model building work correctly in aggressively
trimmed applications (e.g. MAUI).

Fixes dotnet#27474
roji added a commit to roji/efcore that referenced this issue Apr 29, 2022
To make NRT-based model building work correctly in aggressively
trimmed applications (e.g. MAUI).

Fixes dotnet#27474
@ghost ghost closed this as completed in 813322f May 16, 2022
@AndriySvyryd AndriySvyryd modified the milestones: 6.0.x, 6.0.6 May 16, 2022
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 16, 2022
@RobertoGFilho
Copy link

i've been facing the same issue on IOS, butwindows and Android it's ok

@roji
Copy link
Member

roji commented Aug 17, 2022

@RobertoGFilho which error message are you seeing exactly on iOS? If it's Set the MSBuild Property 'NullabilityInfoContextSupport' to true in order to enable it., can you try doing that in your csproj and see if the error goes away?

@RobertoGFilho
Copy link

hi @roji this error shows up when i try to run Database.Migrate(); or Database.EnsureCreated(); but just iOS. on Aondroid and Windows it's ok.

Yes I added NullabilityInfoContextSupport on project file, see:
<NullabilityInfoContextSupport>true</NullabilityInfoContextSupport>

image

Error message:

NullabilityInfoContext is not supported in the current application because 'System.Reflection.NullabilityInfoContext.IsSupported' is set to false.
Set the MSBuild Property 'NullabilityInfoContextSupport' to true in order to enable it.

@roji
Copy link
Member

roji commented Aug 17, 2022

@eerhardt any ideas on the above?

@RobertoGFilho
Copy link

On Xamarin works OK, but MAUI not.

@RobertoGFilho
Copy link

i will try to create a demo to reproduce this erro.. and i will share here

@roji
Copy link
Member

roji commented Aug 17, 2022

@RobertoGFilho please open a new issue with your repro so we can track that separately.

@eerhardt
Copy link
Member Author

@RobertoGFilho - if you have an isolated app that reproduces the error, I can take a look. Please ping me on the new issue.

@RobertoGFilho
Copy link

Hi @eerhardt ! please you can reproduce this erro on this demo : https://github.com/RobertoGFilho/EFCore

  • Windows ok
  • Android ok
  • iOS crash

I'm using these packages:

image

@RobertoGFilho
Copy link

I have openned a new issue #28773

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-global closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants