search

dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.21k stars 1.75k forks source link

[Bug] Resizetizer assumes all projects use the same TFM version #1935

Open mattleibow opened 3 years ago

mattleibow commented 3 years ago

Description

We had an issue in Resizetizer where it passes the TFM from the head project to the dependencies in order to try and locate assets. As a result the TFM that is specified in the dependency is never run and could potentially get missed. This was never noticed in the past as the previous collection target did not depend on anything and never ran anything but its own target. However, this just hid the underlying issue and could potentially just never returned anything.

After PR #1905, the MSBuild tasks changed a bit so that it expected the TFMs to be restored. But, since the TFM from the head project did not match the TFM in the dependency, it fails.

The correct fix is for the dependency project to somehow resolve the incoming TFM to match one of the one it has. Not sure how easy this is.

Workaround

A workaround would be for the library project to multi-target for bot hthe head project and the desired TFM.

The issue has a workaround where all the projects in the chain target the exact same TFM. However, this is not always viable since you may have a project that targets a lower TFM for compatibility reasons.

Logs

Microsoft.Maui.WinUI-Debug.binlog.zip

Eilon commented 3 years ago

Hmm not sure I fully grasp. The AppProj and SharedProj will frequently not have the same TFM, right? And the SharedProj can use only its declared TFMs for restore/resolving things. I don't think you can just pick another TFM to do the resolution because that could produce incompatible results (even if that TFM is a superset, e.g. using net6.0-android to resolve something in a net6.0 project).

cc @javiercn

mattleibow commented 3 years ago

Yeah, that is the issue. The shared project was say windows8 and the app was windows10. The bug is that we pass windows10 to the shared project, and that is bad.

What needs to happen is we pass windows10 to the shared project, and then we need that shared project to properly select a TFM to use.

I heard that @jsuarezruiz might have done something relating to this before? And then @jonathanpeppers just knows everything with MSBuild.

jonathanpeppers commented 3 years ago

@mattleibow can you see if we can remove this:

https://github.com/dotnet/maui/blob/a0f928892c2b25a20038e27e03d1c7b8d08036ad/.nuspec/Microsoft.Maui.Resizetizer.targets#L213

I vaguely remember there was some issue where it was needed, but maybe we don't.

javiercn commented 3 years ago

@jonathanpeppers Am I correct in thinking that the right logic when calling referenced projects is something along the lines of:

jonathanpeppers commented 3 years ago

@javiercn if it were me, I would try to remove TargetFramework completely if you can.

I've seen folks use something like:

<ProjectReference Include="..\foo\foo.csproj" AdditionalProperties="TargetFramework=netstandard2.0" />

Similar to: https://github.com/dotnet/sdk/issues/2280#issuecomment-605312434

Since this is another case, maybe it would be best to try to avoid TargetFramework?

javiercn commented 3 years ago

@jonathanpeppers we encountered something similar when we were doing the original version of static web assets.

The original recommendation we got was to pass in SetTargetFramework if it existed, so as to avoid multiple re-evaluations.

What we ended up doing for Maui was something along the lines of, undefine TargetFramework unless the reference has a SetTargetFramework metadata defined. The reason for this I believe is multi-targeting.

I'm not an expert in this area, but in general I believe the ProjectReference protocol does a call to GetTargetFrameworks when is preparing the project references to resolve what target framework to pass if needed base on the context of the current project.

That said, It's not my area of expertise.

The goal of passing SetTargetFramework, SetConfiguration and so on, I believe is to avoid re-evaluations of the project.

mattleibow commented 3 years ago

We don't want to remove this as we may have the shared project have different assets depending on the TFM referenced. Not sure if that is a thing we want to support.

We could have platform specific shared assets.