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

Add .NET 6 and .NET Framework 4.8 targets #3000

Merged
merged 10 commits into from
Oct 28, 2022
Merged

Conversation

bahusoid
Copy link
Member

@bahusoid bahusoid commented Jan 19, 2022

No description provided.

@bahusoid
Copy link
Member Author

bahusoid commented Jan 19, 2022

Looks like ANTLR generator has some concurrency issue as github build now throw from time to time exceptions like:

Error: /home/runner/.nuget/packages/antlr3.codegenerator/3.5.2-rc1/build/Antlr3.CodeGenerator.targets(146,5): error AC1000: 
Unknown build error:  : error 10 : internal error: 
System.IO.FileNotFoundException: Could not find file '/home/runner/work/nhibernate-core/nhibernate-core/src/NHibernate/obj/Release/netcoreapp2.0/Hql__.g'. File name: '/home/runner/work/nhibernate-core/nhibernate-core/src/NHibernate/obj/Release/netcoreapp2.0/Hql__.g'    
at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter)    
at Interop.CheckIo(Error error, String path, Boolean isDirectory, Func`2 errorRewriter)    
at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String path, OpenFlags flags, Int32 mode)    
at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)    at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)    
at Antlr3.Tool.GrammarSpelunker.Parse()   
...

@hazzik
Copy link
Member

hazzik commented Jan 19, 2022

Why do we need net48 target in the library? In test I understand, and wanted to implement it, but in the library?

@bahusoid
Copy link
Member Author

Why do we need net48?

Why not? Since it's already used in tests, I thought it wouldn't hurt to have it for lib too. And some optimizations are still possible on this target (like here)

@bahusoid bahusoid changed the title Add .net 6 and net48 targets WIP Add .net 6 and net48 targets Jan 22, 2022
@bahusoid bahusoid changed the title WIP Add .net 6 and net48 targets Add .net 6 and net48 targets Aug 29, 2022
@hazzik hazzik enabled auto-merge (squash) August 30, 2022 00:17
@bahusoid
Copy link
Member Author

@fredericDelaporte Seems hazzik made it up to you (enabled auto-merge). Any objections?

@hazzik
Copy link
Member

hazzik commented Sep 13, 2022

I wanted to merge it after some other PR and after 5.3.x merge, so enabled auto merge.

Also, I’m a little hesitant on adding .NET4.8 target.

@fredericDelaporte
Copy link
Member

I think it makes sense to add that 4.8 target, provided we consider the NuGet size is not an issue. .Net Framework is far from its end of life: enabling some more optimizations already done for other targets should be beneficial to lots of users.

(I have seen a 4.8.1 has been released in August, but we should not target it in my opinion. It adds arm support, which users can still get by targeting their application on 4.8.1, without needing NHibernate targeting it too. Moreover that version is available only on most recent versions of Windows OSes.)

@hazzik
Copy link
Member

hazzik commented Sep 14, 2022

enabling some more optimizations already done for other targets should be beneficial to lots of users.

What exactly do you mean, @fredericDelaporte? Optimizations in the NHibernate, or in the Framework? If later then it is all runtime dependent and does not depend on the target.

@fredericDelaporte
Copy link
Member

I mean the former, like mentioned by Roman.

Why not? Since it's already used in tests, I thought it wouldn't hurt to have it for lib too. And some optimizations are still possible on this target (like here)

@hazzik
Copy link
Member

hazzik commented Oct 27, 2022

@fredericDelaporte please approve and merge if you're happy with the changes

@fredericDelaporte fredericDelaporte added this to the 5.4 milestone Oct 28, 2022
@hazzik hazzik changed the title Add .net 6 and net48 targets Add .NET 6 and .NET Framework 4.8 targets Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants