- 
                Notifications
    You must be signed in to change notification settings 
- Fork 316
Adding support for .NET 10 #3686
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
f9ef389
              dc62764
              6b3eb2c
              2f0ba41
              6430e14
              c862760
              4dfe206
              d8ba636
              08ac1e7
              6ce6e73
              63a8371
              0c82041
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -14,10 +14,10 @@ | |
| <PackageReference Include="Microsoft.Extensions.Caching.Memory" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" /> | ||
| <PackageReference Include="System.Buffers" /> | ||
| <!--<PackageReference Include="System.Buffers" />--> | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tidy up these commented-out lines. | ||
| <PackageReference Include="System.Configuration.ConfigurationManager" /> | ||
| <PackageReference Include="System.Memory" /> | ||
| <PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" /> | ||
| <!--<PackageReference Include="System.Memory" />--> | ||
| <!--<PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" />--> | ||
| <PackageReference Include="System.Security.Cryptography.Pkcs" /> | ||
| </ItemGroup> | ||
| <ItemGroup Condition="'$(TargetFramework)' == 'net462'"> | ||
|  | @@ -27,4 +27,10 @@ | |
| <PackageReference Include="System.Data.Common" /> | ||
| <PackageReference Include="System.ValueTuple" /> | ||
| </ItemGroup> | ||
|         
                  priyankatiwari08 marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
|  | ||
| <ItemGroup Condition="$([MSBuild]::VersionLessThan('$(NETCoreSdkVersion)', '10.0.100'))"> | ||
| <PackageReference Include="System.Buffers" /> | ||
| <PackageReference Include="System.Memory" /> | ||
| <PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" /> | ||
| </ItemGroup> | ||
| </Project> | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -2,7 +2,7 @@ | |
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <AssemblyName>PerformanceTests</AssemblyName> | ||
| <TargetFrameworks>net8.0;net9.0</TargetFrameworks> | ||
| <TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks> | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, making this change will require that we start using the .NET 10 SDK to build the tests everywhere (locally as devs and during all pipelines). For devs, that's fine - we just need to install the .NET 10 SDK wherever we're developing. We will need to update the BUILDGUIDE.md to reflect this new requiremet. For pipelines, we will need to find everywhere that we perform a build of the tests and add a step that installs the .NET 10 SDK. If you do this, then you can use the existing CI pipelines to verify .NET 10 compatibility rather than trying to get the tests to run for you locally (which is close to impossible currently). | ||
| <Configurations>Debug;Release;</Configurations> | ||
| <IntermediateOutputPath>$(ObjFolder)$(Configuration).$(Platform).$(AssemblyName)</IntermediateOutputPath> | ||
| <OutputPath>$(BinFolder)$(Configuration).$(Platform).$(AssemblyName)</OutputPath> | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -2,7 +2,7 @@ | |
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <StartupObject>Microsoft.Data.SqlClient.ExtUtilities.Runner</StartupObject> | ||
| <TargetFrameworks>net9.0</TargetFrameworks> | ||
| <TargetFrameworks>net9.0;net10.0</TargetFrameworks> | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can remove  | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.SqlServer.SqlManagementObjects" /> | ||
|  | @@ -11,7 +11,6 @@ | |
| Transitive dependencies with vulnerabilities, so we explicitly ask for | ||
| non-vulnerable versions. | ||
| --> | ||
| <PackageReference Include="System.Formats.Asn1" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <ProjectReference Include="../Microsoft.Data.SqlClient.TestUtilities/Microsoft.Data.SqlClient.TestUtilities.csproj" /> | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use
$([MSBuild]::VersionGreaterThanOrEquals(...))here and below.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this change and tried building the driver code but it resulted in below error:
error MSB4184: The express
ion "[MSBuild]::VersionGreaterThanOrEquals('', net9.0)" cannot be evaluated. Version string was not in a correct format.
On checking further could understand that since we have multi-targetting with TargetFrameworks, MSBuild sets $(TargetFramework) for each target during the build, but during initial evaluation or restore, it may be unset leading to $TargetFramework being empty which caused above error.
Hence, I believe its rather safe to use below:
• It directly checks for the exact target frameworks you want (net9.0 or net10.0).
• It avoids MSBuild errors if $(TargetFramework) is empty or not a valid version string.
• It is more readable and less error-prone than using VersionGreaterThanOrEquals, which can fail if the property is not set.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at the docs for
VersionGreaterThanOrEqualshere:https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?view=vs-2022#msbuild-version-comparison-functions
You could also try using
IsTargetFrameworkCompatible:https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?view=vs-2022#msbuild-targetframework-and-targetplatform-functions
$([MSBuild]::IsTargetFrameworkCompatible($(TargetFramework), 'net9.0'))That should be false when
$(TargetFramework)isnet8.0, and true fornet9.0andnet10.0.I think that is actually better than using
VersionGreaterThanOrEquals.