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

Update to .NET 9 #2674

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Update to .NET 9 #2674

wants to merge 6 commits into from

Conversation

ErikEJ
Copy link
Contributor

@ErikEJ ErikEJ commented Jul 15, 2024

fixes #2669

NuGet.config Outdated Show resolved Hide resolved
- name: Database
value: Northwind
- name: platform
value: AnyCPU
- name: TargetNetFxVersion
value: net481
- name: TargetNetCoreVersion
value: net8.0
value: net9.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the exact purpose of this value is

@@ -152,7 +152,11 @@ internal sealed class BinaryOrderedUdtNormalizer : Normalizer
if (nullByte == 0)
{
result = _nullInstance;
s.Read(_padBuffer, 0, _padBuffer.Length);
#if NET8_0_OR_GREATER
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes the new CA2022

@@ -12,6 +12,7 @@
<Configurations>Debug;Release;</Configurations>
<Platforms>AnyCPU;x86;x64</Platforms>
<ReferenceType Condition="'$(ReferenceType)'==''">Project</ReferenceType>
<NuGetAudit>false</NuGetAudit>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent NuGet audit on test projects giving warnings as errors on old packages

@DavoudEshtehari
Copy link
Contributor

FYI, the pipeline is expected to fail because lack of .NET 9 on running agents.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 16, 2024

@DavoudEshtehari Yes, that was expected, but it did not even fail ??

@JRahnama
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 16, 2024

@JRahnama Thanks, much better error this time 😄

The current .NET SDK does not support targeting .NET 9.0.

@saurabh500
Copy link
Contributor

@cheenamalhotra @DavoudEshtehari @JRahnama @David-Engel What do we need to do to move this along?
Our build agents require .Net runtime 9 installed. Is the runtime supposed to be baked into the image itself? Or is there a script which downloads and installs the runtime?

@JRahnama
Copy link
Contributor

JRahnama commented Jul 22, 2024

@cheenamalhotra @DavoudEshtehari @JRahnama @David-Engel What do we need to do to move this along? Our build agents require .Net runtime 9 installed. Is the runtime supposed to be baked into the image itself? Or is there a script which downloads and installs the runtime?

1ES images do not have Net9.0 added. You can see included SDK in 1ES at here. What we can do is trying to install them manually, but then comes VS issues. Net9.0 needs another version of VS

@DavoudEshtehari
Copy link
Contributor

@cheenamalhotra @DavoudEshtehari @JRahnama @David-Engel What do we need to do to move this along? Our build agents require .Net runtime 9 installed. Is the runtime supposed to be baked into the image itself? Or is there a script which downloads and installs the runtime?

To streamline the process and follow the pattern, a new artifact should be included in the images to reduce installation time for each run. But the compatible VS would be required though.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 22, 2024

Not sure I understand the concerns about VS. I you just install the 9.0 preview SDK via a builtin Azure DevOps task and use dotnet build - but as usual I am probably overlooking something

@JRahnama
Copy link
Contributor

Not sure I understand the concerns about VS. I you just install the 9.0 preview SDK via a builtin Azure DevOps task and use dotnet build - but as usual I am probably overlooking something

This is because .NET 9.0 is typically still in development or early release stages, and the corresponding tooling support for the latest .NET version is often provided in the preview releases of Visual Studio. For testing you can remove VS preview from your local machine and try running your application in Net9

@JRahnama JRahnama changed the title Update to .NET 9 preview 6 Update to .NET 9 in v6.0-preview1 Jul 22, 2024
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 23, 2024

@JRahnama I just use this setting in VS Options:

image

@JRahnama
Copy link
Contributor

@JRahnama I just use this setting in VS Options:

image

but if you look into VS Installer you probably will see something similar as below.
However, I will try running them with a test pipeline. We may end up getting good results with C# 13 and not having VS 2022-preview.
image

@JRahnama
Copy link
Contributor

@ErikEJ , hope you don't mind me pushing the yaml changes into your PR.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 23, 2024

@JRahnama no, not at all! I would love to see this build

@David-Engel
Copy link
Contributor

I think this will need a change in ci-prebuild-step.yml like:

- task: UseDotNet@2
  inputs:
    packageType: 'sdk'
    version: '9.x'
    includePreviewVersions: true

@DavoudEshtehari DavoudEshtehari added this to the 6.0-preview2 milestone Sep 5, 2024
@DavoudEshtehari DavoudEshtehari added 🔨 Breaking Change Issues/PRs that are related with breaking API changes in the driver. and removed 🔨 Breaking Change Issues/PRs that are related with breaking API changes in the driver. labels Sep 5, 2024
@DavoudEshtehari DavoudEshtehari changed the title Update to .NET 9 in v6.0-preview1 Update to .NET 9 Sep 5, 2024
@arellegue
Copy link
Contributor

When tested on a local machine having .Net9.0 preview, Code SYSLIB0057 was thrown by the compiler for the following files:

  • SNICommon,cs
  • VirtualSecureModeEnclaveProviderBase.cs

@JRahnama
Copy link
Contributor

When tested on a local machine having .Net9.0 preview, Code SYSLIB0057 was thrown by the compiler for the following files:

  • SNICommon,cs
  • VirtualSecureModeEnclaveProviderBase.cs

Basically you need to change the code to something similar to below:
in SNICommon:

#if !NET9_0_OR_GREATER
                        validationCertificate = new X509Certificate(validationCertFileName);
#else
                        validationCertificate = X509CertificateLoader.LoadCertificateFromFile(validationCertFileName);
#endif

in VirtualSecureModeEnclaveProviderBase.cs:

#if !NET9_0_OR_GREATER
                    certificateCollection.Import(data);
#else
                   _ =  X509CertificateLoader.LoadCertificate(data);
#endif

and in VirtualSecureModeEnclaveProvider.cs:

#if !NET9_0_OR_GREATER
            Certificate = new X509Certificate2(payload);
#else
            _ = X509CertificateLoader.LoadCertificate(payload);
#endif

After that you will get 5 more errors such as below:

SqlClient\artifacts\Project\obj\Debug.AnyCPU\Microsoft.Data.SqlClient\netcore\net9.0\Microsoft.Data.SqlClie
nt.notsupported.cs(798,37): error IL2093: 'DynamicallyAccessedMemberTypes' in 'DynamicallyAccessedMembersAttribute' on
the return value of method 'Microsoft.Data.SqlClient.SqlDataReader.GetFieldType(Int32)' don't match overridden return v
alue of method 'System.Data.IDataRecord.GetFieldType(Int32)'. All overridden members must have the same 'DynamicallyAcc
essedMembersAttribute'

Now we have to address those.

@arellegue
Copy link
Contributor

For the new CA2022 error, in addition to #if NET8_0_OR_GREATER for ReadExactly, since the compiler is throwing error for TargetFramework=net6.0, a #pragma warning disable/restore CA2022 was added for the #else block.

@arellegue
Copy link
Contributor

@ErikEJ, can I push minor fixes into this branch for issues found when compiling MDS locally?

@JRahnama
Copy link
Contributor

JRahnama commented Sep 12, 2024

For the new CA2022 error, in addition to #if NET8_0_OR_GREATER for ReadExactly, since the compiler is throwing error for TargetFramework=net6.0, a #pragma warning disable/restore CA2022 was added for the #else block.

Preferably, add them to the .editorconfig file inside netcore folder.

Per documentation

When to suppress warnings
It is safe to suppress the warning if the locked object is this or Me and the visibility of the self object type is private or internal, and the instance is not accessible using any public reference.
Otherwise, do not suppress a warning from this rule.

Please make sure these do not apply to these cases.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Sep 12, 2024

@arellegue go ahead...

@arellegue
Copy link
Contributor

A "dotnet_diagnostic.CA2022.severity = error" was added to .editorconfig instead since it is the preferred method.

@cheenamalhotra cheenamalhotra modified the milestones: 6.0-preview2, 6.0.0 Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to .Net 9
7 participants