Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.47k stars 4.8k forks source link

[BUG] Azure sdk references packages that live in the .NET 6+ ref and runtime packs. #28477

Closed AraHaan closed 6 months ago

AraHaan commented 2 years ago

Library name and version

Azure.Core 1.24.0

Describe the bug

The package references packages that are outdated and are a part of the ref and runtime packs for .NET 6+. As such I think they can be removed for those who target .NET 6 or newer TFMs. It also gets annoying when they get copied to our output directories as well. Azure.Identity is an dependency of SqlClient which is an dependency of efcore that is used on my projects.

Why remove the packages highlighted below? Well they could be/are outdated, also they use an unneeded amount of network usage to download them (replacing them with the copies from the ref packs for building should reduce network usage for metered networks, yes some programmers use metered networks).

What can be removed:

With this cleanup it would make applications using efcore being able to have smaller build output directories which would make the typical dev using efcore more happy 😄. Also keep up the good work on the azure sdk.

Expected behavior

For these packages to not be in the reference to Azure.Identity and Azure.Core (or the entire SDK even) as they are either could be/are outdated (the ref packs are a much better way to get the references at least for .NET 6+) to also reduce the amount of files copied to the consumer's build output directories (for those who do FDD deployments and not SCD deployments).

Actual behavior

The packages highlighted above are referenced needlessly which then gets copied into consumer project's build output directories when they are not self contained.

Reproduction Steps

Create a new project that references EFCore for sql server and make an db context to an test database, then build the program and look at it's build output directory (do not deploy as self contained).

Environment

.NET SDK:
 Version:   7.0.100-preview.5.22227.1
 Commit:    ebd9c5c2b2

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22000
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\7.0.100-preview.5.22227.1\

global.json file:
  Not found

Host:
  Version:      7.0.0-preview.5.22225.6
  Architecture: x64
  Commit:       6387a2dc3d

.NET SDKs installed:
  3.1.418 [C:\Program Files\dotnet\sdk] (Visual Studio installed this because I also been working on the winforms repo...)
  5.0.407 [C:\Program Files\dotnet\sdk] (Visual Studio installed this because I also been working on the winforms repo...)
  6.0.104 [C:\Program Files\dotnet\sdk] (I installed this)
  6.0.202 [C:\Program Files\dotnet\sdk] (I installed this)
  6.0.300-preview.22204.3 [C:\Program Files\dotnet\sdk] (Visual Studio installed this as well)
  7.0.100-preview.5.22227.1 [C:\Program Files\dotnet\sdk] (I installed this)

.NET runtimes installed:
  Elskom.Sdk.App 6.0.0-preview.58.1 [C:\Program Files\dotnet\shared\Elskom.Sdk.App] (an runtime I made myself)
  Microsoft.AspNetCore.App 3.1.24 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.0-preview.5.22226.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.24 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0-preview.5.22225.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.24 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.16 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.0-preview.5.22226.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Download .NET:
  https://aka.ms/dotnet-download

Learn about .NET Runtimes and SDKs:
  https://aka.ms/dotnet/runtimes-sdk-info

Visual Studio 2022 17.2 Preview 5.

azure-sdk commented 2 years ago

Label prediction was below confidence level 0.6 for Model:ServiceLabels: 'Storage:0.25744912,Extensions:0.10864456,Service Bus:0.072350934'

AraHaan commented 2 years ago

Other things that might need to be changed:

jsquire commented 2 years ago

//cc: @tg-msft, @KrzysztofCwalina

jsquire commented 2 years ago

Thank you for your feedback. Tagging and routing to the team members best able to assist.

weshaggard commented 2 years ago

@AraHaan your screen shots are for older versions of Azure.Core, you are showing version 1.6.0. We have added a net5.0 target in newer versions (https://www.nuget.org/packages/Azure.Core/1.24.0), however some of those might also be able to be pruned further.

@ericstj is there any recommendations for packages like ours here? Should we add net6.0 specific target to eliminate some of these dependencies?

ericstj commented 2 years ago

cc @viktorhofer

I do think it's good to target newer frameworks and reduce your dependency set. For any packages which have been absorbed into the framework this is safe and compatible. You can also benefit by using the new API and functionality available in those new frameworks.

PS: The only place to be careful about this is with packages like Microsoft.BCL.AsyncInterfaces -- those were not absorbed by the framework and need to remain exposed -- or even better if you ever use them try to avoid exposing them by using PrivateAssets="Compile"

AraHaan commented 2 years ago

Ah I see that 1.6.0 is old, should I update my PR to SqlClient to also update the Azure SDK packages it uses?

weshaggard commented 2 years ago

@AraHaan I don't have any context of your SqlClient PR but I would definitely recommend referencing the latest versions if possible.

Thanks @ericstj. We will definitely consider adding the net6.0 dependency just to remove package dependencies.

AraHaan commented 2 years ago

Alright for now I reference it like so:

image

At least until I get something merged and published for SqlClient to update it directly there to where I could simply just reference the latest SqlClient directly.

mhamri commented 1 year ago

Is there any update on this? It's high priority as it's breaking and blocking us from building our app

image

We are using efcore, and efcore is dependent on Azure.Identity and azure.identity is dependent on azure.core. But that's still using 1.24

We are facing a version mismatch and we can't build our app anymore

System.IO.FileLoadException: Could not load file or assembly 'Azure.Core, Version=1.33.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8'. Could not find or load a specific file. (0x80131621)
File name: 'Azure.Core, Version=1.33.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8'
 ---> System.IO.FileLoadException: Could not load file or assembly 'Azure.Core, Version=1.33.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8'.
   at System.Runtime.Loader.AssemblyLoadContext.<LoadFromPath>g____PInvoke|5_0(IntPtr ptrNativeAssemblyBinder, UInt16* ilPath, UInt16* niPath, ObjectHandleOnStack retAssembly)
   at System.Runtime.Loader.AssemblyLoadContext.LoadFromAssemblyPath(String assemblyPath)
   at System.Reflection.Assembly.LoadFrom(String assemblyFile)
   at System.Reflection.Assembly.LoadFromResolveHandler(Object sender, ResolveEventArgs args)
   at System.Runtime.Loader.AssemblyLoadContext.InvokeResolveEvent(ResolveEventHandler eventHandler, RuntimeAssembly assembly, String name)
   --- End of inner exception stack trace ---
   at System.Reflection.ConstructorInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
   at System.Activator.CreateInstance(Type type, Object[] args)
   at companyName.AxlTool.Commands.SeedCommand.Execute() in C:\repo\AppName\Common\src\companyName.AxlTool\Commands\SeedCommand.cs:line 40
   at Microsoft.EntityFrameworkCore.Tools.Commands.CommandBase.<Configure>b__0_0() in C:\repo\AppName\Common\src\companyName.AxlTool\ef\Commands\CommandBase.cs:line 17
   at Microsoft.DotNet.Cli.CommandLine.CommandLineApplication.Execute(String[] args) in C:\repo\AppName\Common\src\companyName.AxlTool\ef\CommandLineUtils\CommandLineApplication.cs:line 151
   at companyName.AxlTool.Program.Main(String[] args) in C:\repo\AppName\Common\src\companyName.AxlTool\Program.cs:line 35

the interesting part is that Microsoft.EntityFramework.SqlServer is dependent on both 1.24 and 1.33.

AraHaan commented 1 year ago

This is still work in progress due to the PR having some conflicts.

ericstj commented 8 months ago

In addition to these being unnecessary - many of the packages referenced by Azure.Core are now out of date and no longer being serviced at the major version referenced. This will make it a significant change should any of these packages require critical updates. I'd recommend that Azure.Core update the latest available LTS for all of these packages. That should be the latest major version.

github-actions[bot] commented 6 months ago

Hi @AraHaan, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.