search

xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.46k stars 512 forks source link

System.Drawing.Common Gets Added DLL & To NuGet on Pack #7249

Closed jamesmontemagno closed 2 years ago

jamesmontemagno commented 5 years ago

Steps to Reproduce

  1. Take the very simple library attached and run pack on it with msbuild

Open the dll that is generated or the nuget that is created and System.Drawing.Common.dll will be added as a ref.

This only happens in VS 2019 16.3/16.4 (maybe 16.2 as well).

If you pack with VS 2017 it works just fine.

This is happening with many library creators:

This causes install issues for some users since ref assembly isn't added on previous verisons and it isn't even needed.

Expected Behavior

No framework ref should be added

Actual Behavior

We get a ref in VS 2019 but not 2017. This does not happen with Android at all.

Environment

VS 2019 Xamarin.iOS 13.4.0.2

Example Project (If Possible)

Attached: Feature2.zip

NuGet Generated: Plugin.Feature2.1.0.0.zip

chamons commented 5 years ago

Results of https://github.com/xamarin/xamarin-macios/pull/6011

chamons commented 5 years ago

We believe the same fix should impact XA as well. @jamesmontemagno - Can you please verify that the same XA and XI SDK versions have different behavior?

chamons commented 5 years ago

From that PR:


Xamarin.iOS/WatchOS/TVOS.dll to System.Drawing.Common.dll user projects
would fail to compile. We need to add some msbuild logic to add a
reference to the assembly automatically.```
chamons commented 5 years ago

Also try this in your project:

<DisableExtraReferences>true</DisableExtraReferences>

jamesmontemagno commented 5 years ago

So, it weirdly didn't impact XA, which is strange.

I have tested it with <DisableExtraReferences> and it works now perfect!

jamesmontemagno commented 5 years ago

I still find it odd that only this one was added though on pack.... maybe it should be flipped to true by default when packing? @Redth

clairernovotny commented 5 years ago

The Extras has treated the "default" ones as implicit and thus updates them to not have them pack: https://github.com/onovotny/MSBuildSdkExtras/blob/master/Source/MSBuild.Sdk.Extras/Build/Platforms/Xamarin.targets#L16-L22

That sets the PrivateAssets=true ultimately. Should the Extras always add System.Drawing.Common for projects? Better yet, if there's a place where the defaults are captured, I can keep them updated -- or someone can submit a PR ;)

Right now you can see that it adds a few common references and the platform reference.

jamesmontemagno commented 5 years ago

I don't think it should be adding them. My thought is that probably SDK Extras could set 

<DisableExtraReferences>true</DisableExtraReferences>

Oh, or I see we could add a PR so we add in SDC.dll in your list and it then wouldn't be added?

clairernovotny commented 5 years ago

Oh, or I see we could add a PR so we add in SDC.dll in your list and it then wouldn't be added?

Right -- it'll be added as an implicit reference in VS but not added to the pack list.

Redth commented 5 years ago

I think the disconnect here is that the ref for System.Drawing.Common is blanket added in vs2019 regardless of if it's being used or not. So in our case, we have a library which doesn't use it at all, but when you compile in vs2019, it gets added regardless of its usage, which flows all the way to /t:pack and ends up in the nupkg. Since this thing didn't exist in vs2017, if our nupkg gets installed there, we have an issue (or it fails to install entirely?). This add it by default makes sense for apps, but not really for libraries.

clairernovotny commented 5 years ago

If it's added using the Extras as an implicit reference, then it won't be exposed to 2017 -- at least not directly. If there's a type in it (that's not forwarded out), it'll have an assm ref in the dll that'll have to be resolved somehow.

rhouweling commented 5 years ago

Hi, experiencing this issue currently when updating the akavache libary for iOS for our project. I want to try the "workaround" true but I can't seem to find any documenatation or examples on this and I'm not sure where exactly to place the element. Or am I misunderstanding and should this be done in the package? If so, is there any workaround when experiencing this issue as a client?

chamons commented 5 years ago

@rhouweling To apply the work around, you'd need to open your csproj in a text editor and add:

<DisableExtraReferences>true</DisableExtraReferences>

between the first <PropertyGroup> and </PropertyGroup>, like the other properties.

rhouweling commented 5 years ago

@chamons Thanks for the response. Sad to say that when adding the DisableExtraReferences in the csproj doesn't fix the error below. This occurs when upgrading the akavache package from 6.2.3 to 6.9.1 in our current iOS project. Any other ideas on what I can do to resolve this?

Error: Failed to add reference. The package 'SQLitePCLRaw.lib.e_sqlite3.ios' tried to add a framework reference to 'System.Drawing.Common.dll' which was not found in the GAC. This is possibly a bug in the package. Please contact the package owners for assistance. Error HRESULT E_FAIL has been returned from a call to a COM component.

NehaGawri commented 5 years ago

After upgrading Acr.UserDialogs to version 7.0.35 , Azure pipeline build is failing with error "Failed to resolve "System.Drawing.Color" reference from "System.Drawing.Common, Version=4.0.0.0,." Though it is building fine locally .

I have tried adding "true" in iOS. csproj for , but it did not help .

Will it be fixed in the next upgrade of this nuget ? Or is it something that needs to be fixed in csproj .

jamesmontemagno commented 5 years ago

The NuGet package maintainer needs to add the flag, not your app.

coldacid commented 4 years ago

Unfortunately it seems that adding the flag isn't a possibility for some projects (i.e. Acr.UserDialogs) because of CI constraints, and that package's developer's advice of upgrade to VS2019 doesn't work because I'm already running the latest version of 2019. Our project now has to use two different versions of the package between Android and iOS and I'm quite worried and annoyed at the potential support cost this will cause us.

coldacid commented 4 years ago

It would be nice if installing the System.Drawing.Common package would work around this, but it doesn't. Just putting it out there for anyone who hasn't tried it yet.

aritchie commented 4 years ago

@jamesmontemagno This flag breaks builds for me. Also, if I don't ref System.Drawing.Common in iOS, I don't get System.Drawing.Color which I use. I did use to pull in OpenTK-1.0 but that doesn't work anymore.

If there is something I need to change in my package for the latest updates - I'm happy change to it. The Nuget maintainer in this case, has tried that flag in failure.

rhouweling commented 4 years ago

Any update on this? This is preventing me from updating some packages in which bugs are solved for quite a while now...

jamesmontemagno commented 4 years ago

Another one here: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/1493 I think we need to try to diagnose this a bit more.

@rhouweling are you using an old packages.config file?

rhouweling commented 4 years ago

@jamesmontemagno Yes, we are using packages.config

rhouweling commented 4 years ago

@jamesmontemagno We didn't really notice the PackageRefence method was supported in Xamarin platform projects to be honest. Converting now.

mattleibow commented 4 years ago

I created a PR for a NuGet that you can install to make sure that your assemblies are good. It basically redirects the type from System.Drawing.Common to either OpenTK or Xamarin.Xxx depending on where the type was moved from.

https://github.com/xamarin/XamarinComponents/pull/764

Technically, you can use it to remap references, but I have not tested that in any way.

@aritchie this may help with your issue with OpenTK and all that.

It hasn't been merged yet, but seems to fix things in my tests. I hope to get a preview out ASAP so folks can give it a go.

rhouweling commented 4 years ago

Thanks @mattleibow . My issue was solved after I converted to Package References. However this (and the update to VS2019 16.4) resulted in an issue which causes my Android builds to fail (local as well as AppCenter): https://developercommunity.visualstudio.com/content/problem/845345/cant-build-xamarin-for-android-project.html Not sure if the issues are related, I doubt it, but wanted to mention it just in case... Thanks for the help and the suggestions @jamesmontemagno and @mattleibow and the others!

fabioaraujoo commented 4 years ago

I updated by MAC and ... Eureca!

rolfbjarne commented 2 years ago

This does not apply to .NET.