You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "paulirwin (via GitHub)" <gi...@apache.org> on 2025/11/22 04:49:04 UTC
[PR] Add .NET 10 target, #1219 [lucenenet]
paulirwin opened a new pull request, #1222:
URL: https://github.com/apache/lucenenet/pull/1222
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/master/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html).
- [X] You've included unit or integration tests for your change, where applicable.
- [X] You've included inline docs for your change, where applicable.
- [X] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue.
Add .NET 10 target
Fixes #1219
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2554470265
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,11 +24,10 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net8.')) ">Major</RollForward>
+ <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
+ <RollForward>Major</RollForward>
Review Comment:
Agreed re: net9.0, but the condition doesn't make sense to me. The condition only applies at compile time, and with setting `TargetFrameworks` to `net10.0;net8.0` it will only ever be net10.0 or net8.0. So setting the RollForward to apply only to net10.0 and net8.0 is the same as not having the condition at all and having it always apply.
That said, I'll make that change for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#issuecomment-3741210840
The directory issue wasn't a single test, that was just an example on how it can be reproduced. I saw at least 2 tests fail. I suspect this is a systemic concurrency problem that will frequently fail on the `net10.0` target, so ideally, we wouldn't merge this until after it is addressed.
Since this will get in the way of normal development, I suggest do one of the following on the `master` branch:
1. Set the default target framework for tests to `net9.0`
2. Continually run the tests, record all of the ones that fail, and mark them with the `[AwaitsFix]` attribute
3. Revert this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#issuecomment-3706136304
I have made all of the updates to support Visual Studio 2022 (limited support for contributors, excluding `net10.0`, `netstandard2.0`, and all .NET Framework versions from the build when building inside VS 2022).
I also fixed an issue with how the scripts use the `PrintTargetFrameworks` target. Note that this target is what requires projects to identify themselves with `<IsTestProject>true</IsTestProject>` and why it is conditional in some cases. This target tells the scripts which target frameworks are supported for a specific project file after all MSBuild conditions are run. However, it was failing because the output of MSBuild changed to include more information. The fix was to add a key identifier for the "field" and use a Regex to parse it rather than relying on a position in the output.
> NOTE: This fix will need to be copied to all of our other projects because they all use the same approach to determine which target frameworks to run tests on.
I am currently testing the local build with those changes and will report back if there are any issues with it, but it is limited to a function that has already been tested in the `Generate-TestWorkflows.ps1` script, so it is not likely that it will fail.
The two test failures are real issues. One of them is similar to the issue I reported earlier - something having to do with `CompoundFileDiretory` thread safety on .NET 10 (I saw it happen on macOS, also). The other failure is the ongoing issue we are having with Antlr4BuildTasks, which should be fixed after the new version is released and we can drop the dependency on Antlr4.CodeGenerator as well as the following probing for the `.jar` file within that package.
```xml
<PropertyGroup>
<Antlr4CodeGeneratorVersion>$([System.IO.Path]::GetFileName('$(PkgAntlr4_CodeGenerator)'))</Antlr4CodeGeneratorVersion>
<!-- Required for Antlr4BuildTasks to find the antlr4 .jar file. When missing,
it causes an error on the SonarCloud workflow. -->
<Antlr4ToolPath>$(PkgAntlr4_CodeGenerator)\tools\antlr4-csharp-$(Antlr4CodeGeneratorVersion)-complete.jar</Antlr4ToolPath>
</PropertyGroup>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2554968314
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,11 +24,10 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net8.')) ">Major</RollForward>
+ <TargetFrameworks>net10.0;net8.0</TargetFrameworks>
+ <RollForward Condition=" $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net10.')) ">Major</RollForward>
- <IsPublishable>false</IsPublishable>
- <IsPublishable Condition="'$(TargetFramework)' == 'net8.0'">true</IsPublishable>
+ <IsPublishable Condition="' $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net10.')) '">true</IsPublishable>
Review Comment:
The tests are failing due to the extra single quotes around this condition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2554942706
##########
.build/azure-templates/run-tests-on-os.yml:
##########
@@ -107,31 +107,6 @@ steps:
displayName: 'Use .NET SDK ${{ parameters.dotNetSdkVersion }} (x86)'
condition: and(succeeded(), contains('${{ parameters.framework }}', 'net10.'), eq('${{ parameters.vsTestPlatform }}', 'x86'))
-- task: UseDotNet@2
Review Comment:
No, I didn't mean to remove .NET 9 entirely. We still need to cover it in Azure DevOps and debugging to verify it works, just not for PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2553856055
##########
Directory.Build.props:
##########
@@ -26,7 +26,7 @@
</PropertyGroup>
<PropertyGroup>
- <LangVersion>11.0</LangVersion>
+ <LangVersion>14.0</LangVersion>
Review Comment:
Typically, we only bump `LangVersion` if we need features from the language due to the knock-on effects that it could create. And we wouldn't need to rename all of the `field` fields in this PR.
Let's split this out into a separate issue if we need to do it at all.
One issue with this is that it immediately means that the latest tooling is required that supports .NET 10 to work on Lucene.NET. IMO, it would be better for contributors if we supported VS2022/.NET8/.NET9 so we don't leave contributors that may not have the time or space to install new tools behind. We can do this by staying on an older `LangVersion` and making the `net10.0` target conditional depending on the version of Visual Studio installed. This should be fine because issues on a specific TFM are rare and any issues that arise would be caught by CI.
An example of this approach can be found in `ICU4N`:
```xml
<TargetFrameworks Condition="$(VisualStudioVersion) >= 16.10">net9.0;net8.0</TargetFrameworks>
<TargetFrameworks>$(TargetFrameworks);netstandard2.0;net462;net451;net40</TargetFrameworks>
```
At one point, I think we had `net6.0` requiring `$(VisualStudioVersion) >= 16.10` and `net5.0` always enabled. We can do something similar using the condition `$(VisualStudioVersion) >= 18.0` for `net10.0` now.
Of course, there is another level to this in the tests, since we only compile the .NET Framework tests on Windows.
```xml
<TargetFrameworks Condition=" '$(TestFrameworks)' == 'true' AND $([MSBuild]::IsOsPlatform('Windows')) ">$(TargetFrameworks);net48;net472</TargetFrameworks>
```
And in the case of Lucene.NET, we only have it defaulted to run tests for a single target framework, so that would need to be conditional depending on the tooling.
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,11 +24,10 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net8.')) ">Major</RollForward>
+ <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
+ <RollForward>Major</RollForward>
Review Comment:
We need this to be conditional, since we should only roll forward from the highest version we support to any future version of .NET.
Technically, we could do it this way, but this is the largest package by far and packing up `net9.0` (with only 6 months of support left) seems like a non-starter. So, IMO, we should remove `net9.0` and make both `net8.0` and `net10.0` roll forward to future versions, which would include `net9.0` rolled forward from `net8.0`.
```xml
<TargetFrameworks>net10.0;net8.0</TargetFrameworks>
<RollForward Condition=" $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net10.')) ">Major</RollForward>
```
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,11 +24,10 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net8.')) ">Major</RollForward>
+ <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
+ <RollForward>Major</RollForward>
- <IsPublishable>false</IsPublishable>
- <IsPublishable Condition="'$(TargetFramework)' == 'net8.0'">true</IsPublishable>
+ <IsPublishable>true</IsPublishable>
Review Comment:
Since we publish on .NET Framework for the solution, this *must* be conditional, since .NET Tools only work on .NET Core.
```xml
<IsPublishable Condition="' $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net10.')) '">true</IsPublishable>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2553874167
##########
.github/workflows/Generate-TestWorkflows.ps1:
##########
@@ -284,6 +287,12 @@ jobs:
uses: actions/setup-dotnet@v5
with:
dotnet-version: '$DotNet9SDKVersion'
+ if: `${{ startswith(matrix.framework, 'net9.') }}
+
+ - name: Setup .NET 10 SDK
+ uses: actions/setup-dotnet@v5
+ with:
+ dotnet-version: '$DotNet10SDKVersion'
Review Comment:
Note that we got some feedback about how the `setup-dotnet` task works, which was different from the assumptions made when this task was written: https://github.com/actions/setup-dotnet/issues/679#issuecomment-3550932760
As pointed out, the latest SDK is always installed when specifying a lower one.
While this configuration will work, it would be more explicit (and faster) if this were also conditional. But keep in mind, we also need to ensure it is installed for building .NET Framework or .NET Standard. In this case, we are building test projects, though, so the matrix only includes executable target frameworks. Therefore, we can do:
```yml
- name: Setup .NET 10 SDK
uses: actions/setup-dotnet@v5
with:
dotnet-version: '$DotNet10SDKVersion'
if: `${{ startswith(matrix.framework, 'net4') || startswith(matrix.framework, 'net10.') }}
```
##########
.github/workflows/Generate-TestWorkflows.ps1:
##########
@@ -65,14 +65,16 @@ param(
[string]$RepoRoot = (Split-Path (Split-Path $PSScriptRoot)),
- [string[]]$TestFrameworks = @('net9.0','net8.0','net472','net48'), # targets under test: net8.0, net8.0, netstandard2.0, net462
+ [string[]]$TestFrameworks = @('net10.0', 'net9.0','net8.0','net472','net48'), # targets under test: net10.0, net8.0, net8.0, netstandard2.0, net462
Review Comment:
It is unlikely anything will slip through if we eliminate `net9.0` here, since these tests are just to gatekeep the PRs in a fast and efficient way. We don't bother with macOS or slow tests here, either.
We always run Azure DevOps tests before a release, which include all of the test frameworks, operating systems, and run the full set of tests.
But we need `net10.0` because we should have some coverage on that target before PRs are merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2684220587
##########
src/dotnet/Lucene.Net.Tests.CodeAnalysis/Lucene.Net.Tests.CodeAnalysis.csproj:
##########
@@ -22,11 +22,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
- <TargetFramework>net9.0</TargetFramework>
+ <!-- We don't really care which target framework we test with - this is just to emulate Visual Studio 2017+ all which can load netstandard1.3 analyzers -->
+ <!-- Given that VS 2022 doesn't support net10.0 we will use net8.0 (which will outlive net9.0). We will need to adjust this when we stop testing on net8.0. -->
Review Comment:
This comment is incorrect. .NET 8 and 9 will go out of support on the same day. The .NET STS support policy changed in September. https://devblogs.microsoft.com/dotnet/dotnet-sts-releases-supported-for-24-months/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2591332249
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,11 +24,10 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net8.')) ">Major</RollForward>
+ <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
+ <RollForward>Major</RollForward>
Review Comment:
> packing up net9.0 (with only 6 months of support left) seems like a non-starter
Note that Microsoft extended STS support to match the end date of the previous LTS release, .NET 8, so now there is 11-ish months of support left for .NET 9. But, I don't think we need it. Just FYI.
https://devblogs.microsoft.com/dotnet/dotnet-sts-releases-supported-for-24-months/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#issuecomment-3744880697
The PR does not need to be reverted. If this is an issue in .NET 10, it would surface to users anyways, so better to get it merged so more people can test it.
I am not convinced this is a systemic concurrency problem. I ran the test locally tens of thousands of times on macOS and Ubuntu under .NET 10 and could not reproduce, even tweaking the parameters like the number of threads and wait time. If it were systemic, it would be reproducible at least once every few thousand tries.
My current working theory is that .NET 10 is _so fast_ that we're now dealing with microsecond race conditions that we weren't before, and as noted in #1233, the test does not test real-world conditions. It also might be true that my machine is too fast for this to surface, and it only occurs in the significantly slower Actions containers. Even if someone were trying to create and destroy IndexWriters without reusing them that fast under extremely heavy small-write concurrent load with a RAMDirectory in the real-world, they probably would have their system resilient enough to retry on such an exception, and the chance of it happening twice is very small.
But, I am in favor of #2 above. We should keep testing for this and mark awaits fix if needed. I created PR #1234 to catch the file not found exception, but if someone can demonstrate that there is an actual real-world concurrency issue here with a different solution, I can abandon that PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#issuecomment-3740990289
There already was a separate issue for the Antlr flaky tests, #1228. I've logged one for the directory one, #1233. Those should not hold up this PR. Thanks for contributing the VS2022 support. I hope all contributors can get on VS2026 or Rider making this change obsolete as soon as possible. It will be important to test locally against .NET 10.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2653224168
##########
Directory.Build.props:
##########
@@ -31,6 +31,19 @@
<GitHubProject>lucenenet</GitHubProject>
</PropertyGroup>
+ <PropertyGroup Label="Centralized Target Frameworks">
Review Comment:
Let's centralize the legacy Visual Studio logic here. This will likely be something we need to deal with again when future versions of Visual Studio are released. Here is how this was done in J2N:
```xml
<PropertyGroup Label="Legacy Visual Studio Support">
<VisualStudioMajorVersion>0</VisualStudioMajorVersion>
<!-- Extract major version (works for preview builds too) -->
<VisualStudioMajorVersion Condition="'$(VisualStudioVersion)' != ''">$([System.Version]::Parse($([System.Text.RegularExpressions.Regex]::Match('$(VisualStudioVersion)', '^\d+(\.\d+)*').Value)).Major)</VisualStudioMajorVersion>
<IsLegacyVisualStudioVersion>false</IsLegacyVisualStudioVersion>
<!-- Visual Studio 2026 == Version 18.x - anything below that major version is legacy -->
<IsLegacyVisualStudioVersion Condition="'$(BuildingInsideVisualStudio)' == 'true' And '$(VisualStudioMajorVersion)' < '18'">true</IsLegacyVisualStudioVersion>
</PropertyGroup>
```
This gives us a simple boolean to be used elsewhere instead of repeating this logic in other places (like `Lucene.Net.Analysis.OpenNLP`).
With that in place, the below can be simplified to:
```xml
<PropertyGroup Label="Centralized Target Frameworks">
<!-- Add net10.0 conditionally based on VS version (VS2024+ = 18.0) -->
<LibraryTargetFrameworks>net10.0;net8.0;netstandard2.0;net462</LibraryTargetFrameworks>
<LibraryTargetFrameworks Condition=" '$(IsLegacyVisualStudioVersion)' == 'true' ">net8.0</LibraryTargetFrameworks>
<NetCoreOnlyTargetFrameworks>net10.0;net8.0</NetCoreOnlyTargetFrameworks>
<NetCoreOnlyTargetFrameworks Condition=" '$(IsLegacyVisualStudioVersion)' == 'true' ">net8.0</NetCoreOnlyTargetFrameworks>
</PropertyGroup>
```
While C# 14.0 isn't added in this PR, we will need to eliminate targets on `netstandard2.0` and .NET Framework due to invalid warnings generated by Visual Studio 2022 when using `string` > `ReadOnlySpan<char>` intrinsic conversions. It is probably simpler just to add that logic now so we don't have to revisit it again later. Note that it only applies to the IDE itself, not the command line or CI.
##########
TestTargetFramework.props:
##########
@@ -29,21 +29,25 @@
<!--<TargetFramework>net472</TargetFramework>-->
<!--<TargetFramework>net48</TargetFramework>-->
<!--<TargetFramework>net8.0</TargetFramework>-->
- <TargetFramework>net9.0</TargetFramework>
+ <!-- Default for local development - conditional based on VS version (VS2024+ = 18.0) -->
+ <TargetFramework Condition=" '$(VisualStudioVersion)' >= '18.0' ">net10.0</TargetFramework>
+ <TargetFramework Condition=" '$(VisualStudioVersion)' < '18.0' Or '$(VisualStudioVersion)' == '' ">net8.0</TargetFramework>
<!-- Allow the build script to pass in the test frameworks to build for.
This overrides the above TargetFramework setting.
LUCENENET TODO: Due to a parsing bug, we cannot pass a string with a ; to dotnet msbuild, so passing true as a workaround -->
<!-- Test Client to DLL target works as follows:
Test Client | Target Under Test
+ net10.0 | net10.0
net9.0 | net8.0
net8.0 | net8.0
net48 | net462
net472 | netstandard2.0
-->
- <TargetFrameworks Condition=" '$(TestFrameworks)' == 'true' ">net9.0;net8.0</TargetFrameworks>
+ <!-- CI builds - always build all test frameworks (CI installs required SDKs) -->
+ <TargetFrameworks Condition=" '$(TestFrameworks)' == 'true' ">net10.0;net9.0;net8.0</TargetFrameworks>
<TargetFrameworks Condition=" '$(TestFrameworks)' == 'true' AND $([MSBuild]::IsOsPlatform('Windows')) ">$(TargetFrameworks);net48;net472</TargetFrameworks>
Review Comment:
We will need to add an exclusion for VS 2022 here (this is for C# 14.0 support, which we can add later).
```xml
<TargetFrameworks Condition=" '$(TestFrameworks)' == 'true' AND $([MSBuild]::IsOsPlatform('Windows')) And '$(IsLegacyVisualStudioVersion)' != 'true' ">$(TargetFrameworks);net48;net472</TargetFrameworks>
```
##########
TestTargetFramework.props:
##########
@@ -29,21 +29,25 @@
<!--<TargetFramework>net472</TargetFramework>-->
<!--<TargetFramework>net48</TargetFramework>-->
<!--<TargetFramework>net8.0</TargetFramework>-->
- <TargetFramework>net9.0</TargetFramework>
+ <!-- Default for local development - conditional based on VS version (VS2024+ = 18.0) -->
+ <TargetFramework Condition=" '$(VisualStudioVersion)' >= '18.0' ">net10.0</TargetFramework>
Review Comment:
The way this is currently specified, it will not default to `net10.0` on any IDE that is not Visual Studio 2026 or higher, including Rider or Visual Studio Code. It would be more sensible to downgrade automatically iif it is a legacy VS version.
```xml
<TargetFrameworks>net10.0</TargetFrameworks>
<!-- For Visual Studio 2022, there is no support for net10.0, so we test on net8.0 -->
<TargetFrameworks Condition=" '$(IsLegacyVisualStudioVersion)' == 'true' ">net8.0</TargetFrameworks>
```
See the comment in the root `Directory.Build.targets` for the definition of `IsLegacyVisualStudioVersion`.
##########
src/dotnet/Lucene.Net.Tests.CodeAnalysis/Lucene.Net.Tests.CodeAnalysis.csproj:
##########
@@ -22,11 +22,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
- <TargetFramework>net9.0</TargetFramework>
+ <TargetFramework>net10.0</TargetFramework>
Review Comment:
We will need to add an exception for Visual Studio 2022 here or simply leave this on `net9.0` or `net8.0`.
```xml
<TargetFrameworks>net10.0</TargetFrameworks>
<!-- For Visual Studio 2022, there is no support for net10.0, so we test on net8.0 -->
<TargetFrameworks Condition=" '$(IsLegacyVisualStudioVersion)' == 'true' ">net8.0</TargetFrameworks>
```
See the comment in the root `Directory.Build.targets` for the definition of `IsLegacyVisualStudioVersion`.
##########
src/dotnet/Lucene.Net.Tests.CodeAnalysis/Lucene.Net.Tests.CodeAnalysis.csproj:
##########
@@ -22,11 +22,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
- <TargetFramework>net9.0</TargetFramework>
+ <TargetFramework>net10.0</TargetFramework>
<RootNamespace>Lucene.Net.CodeAnalysis</RootNamespace>
<IsPublishable>false</IsPublishable>
- <IsPublishable Condition=" '$(TargetFramework)' == 'net9.0' ">true</IsPublishable>
+ <IsPublishable Condition=" '$(TargetFramework)' == 'net10.0' ">true</IsPublishable>
Review Comment:
Like the above, we will need this to be publishable on the same platform we use for testing and don't publish any other platform. Note that publishing is not currently executed in the IDE, but this is just in case it is attempted.
```xml
<IsPublishable Condition=" '$(TargetFramework)' == 'net10.0' And '$(IsLegacyVisualStudioVersion)' != 'true' ">true</IsPublishable>
<IsPublishable Condition=" '$(TargetFramework)' == 'net8.0' And '$(IsLegacyVisualStudioVersion)' == 'true' ">true</IsPublishable>
```
##########
src/Lucene.Net.Tests.Analysis.OpenNLP/Lucene.Net.Tests.Analysis.OpenNLP.csproj:
##########
@@ -26,7 +26,7 @@
<PropertyGroup>
<!-- Allow specific target framework to flow in from TestTargetFrameworks.props -->
<!--suppress MsbuildTargetFrameworkTagInspection - even though this only has one target right now, we need to use the plural version for the line below -->
- <TargetFrameworks Condition=" '$(TargetFramework)' == '' ">net9.0;net8.0</TargetFrameworks>
+ <TargetFrameworks Condition=" '$(TargetFramework)' == '' ">net10.0;net9.0;net8.0</TargetFrameworks>
Review Comment:
We will need to conditionally exclude Visual Studio < 2026 here.
```xml
<TargetFrameworks Condition=" '$(TargetFramework)' == '' And '$(IsLegacyVisualStudioVersion)' != 'true' ">net10.0</TargetFrameworks>
<TargetFrameworks Condition=" '$(TargetFramework)' == '' ">$(TargetFrameworks);net9.0;net8.0</TargetFrameworks>
<TargetFrameworks Condition=" $([MSBuild]::IsOsPlatform('Windows')) And '$(IsLegacyVisualStudioVersion)' != 'true' ">$(TargetFrameworks);net48</TargetFrameworks>
```
See the comment in the root `Directory.Build.targets` for the definition of `IsLegacyVisualStudioVersion`.
##########
src/Lucene.Net.Analysis.OpenNLP/Lucene.Net.Analysis.OpenNLP.csproj:
##########
@@ -32,8 +32,9 @@
<PropertyGroup>
<!-- Currently, IKVM doesn't officially support building NetFX on anything but Windows, so we skip it for contributors who may be on various platforms.
We can remove the condition once that has been addressed. See: https://github.com/ikvmnet/ikvm-maven/issues/49 -->
- <!--suppress MsbuildTargetFrameworkTagInspection - even though this only has one target right now, we need to use the plural version for the line below -->
- <TargetFrameworks>net8.0</TargetFrameworks>
+ <!-- net10.0 is conditional on VS version (VS2024+ = 18.0) -->
+ <TargetFrameworks Condition=" '$(VisualStudioVersion)' >= '18.0' ">net10.0;net8.0</TargetFrameworks>
Review Comment:
The way this is currently specified, it will not compile `net10.0` on any IDE that is not Visual Studio 2026 or higher, including Rider or Visual Studio Code. It would be more sensible to reject iif it is a legacy VS version.
```xml
<TargetFrameworks Condition=" '$(IsLegacyVisualStudioVersion)' != 'true' ">net10.0</TargetFrameworks>
<TargetFrameworks>$(TargetFrameworks);net8.0</TargetFrameworks>
<TargetFrameworks Condition=" $([MSBuild]::IsOsPlatform('Windows')) And '$(IsLegacyVisualStudioVersion)' != 'true' ">$(TargetFrameworks);net472</TargetFrameworks>
```
See the comment in the root `Directory.Build.targets` for the definition of `IsLegacyVisualStudioVersion`.
##########
src/dotnet/Lucene.Net.ICU/Lucene.Net.ICU.csproj:
##########
@@ -30,7 +30,7 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0;netstandard2.0;net462</TargetFrameworks>
+ <TargetFrameworks>net10.0;net8.0;netstandard2.0;net462</TargetFrameworks>
Review Comment:
This library is our replacement for what would have been `Lucene.Net.Analysis.ICU`, except that it also contains some of the things from `Lucene.Net.Analysis.Common` that require a `BreakIterator` and other support from ICU to function. So, technically, it can use `$(LibraryTargetFrameworks)` just like the other libraries.
##########
src/dotnet/tools/Lucene.Net.Tests.Cli/Lucene.Net.Tests.Cli.csproj:
##########
@@ -25,7 +25,7 @@
<PropertyGroup>
<!-- Allow specific target framework to flow in from TestTargetFrameworks.props -->
- <TargetFrameworks Condition=" '$(TargetFramework)' == '' ">net9.0;net8.0</TargetFrameworks>
+ <TargetFrameworks Condition=" '$(TargetFramework)' == '' ">net10.0;net9.0;net8.0</TargetFrameworks>
Review Comment:
We will need to add an exception for Visual Studio 2022 here.
```xml
<TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
<!-- For Visual Studio 2022, there is no support for net10.0, so we test on net9.0 and net8.0 -->
<TargetFrameworks Condition=" '$(IsLegacyVisualStudioVersion)' == 'true' ">net9.0;net8.0</TargetFrameworks>
```
See the comment in the root `Directory.Build.targets` for the definition of `IsLegacyVisualStudioVersion`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2554475356
##########
.github/workflows/Generate-TestWorkflows.ps1:
##########
@@ -284,6 +287,12 @@ jobs:
uses: actions/setup-dotnet@v5
with:
dotnet-version: '$DotNet9SDKVersion'
+ if: `${{ startswith(matrix.framework, 'net9.') }}
+
+ - name: Setup .NET 10 SDK
+ uses: actions/setup-dotnet@v5
+ with:
+ dotnet-version: '$DotNet10SDKVersion'
Review Comment:
> As pointed out, the latest SDK is always installed when specifying a lower one.
That is not quite how I read the discussion. It was that the latest _host and runtime_ is installed, then it installs your specified _SDK_ version. This is shown in the logs you shared on that comment thread (note the Runtime vs Sdk difference in the URLs):
```
un actions/setup-dotnet@v5
(node:10216) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
"C:\Program Files\PowerShell\7\pwsh.exe" -NoLogo -Sta -NoProfile -NonInteractive -ExecutionPolicy Unrestricted -Command & 'D:\a\_actions\actions\setup-dotnet\v5\externals\install-dotnet.ps1' -SkipNonVersionedFiles -Runtime dotnet -Channel LTS
dotnet-install: Downloaded file https://builds.dotnet.microsoft.com/dotnet/Runtime/10.0.0/dotnet-runtime-10.0.0-win-x64.zip size is 37111780 bytes.
dotnet-install: Either downloaded or local package size can not be measured. One of them may be corrupted.
dotnet-install: Extracting the archive.
dotnet-install: Note that the script does not ensure your Windows version is supported during the installation.
dotnet-install: To check the list of supported versions, go to https://learn.microsoft.com/dotnet/core/install/windows#supported-versions
dotnet-install: Installed version is 10.0.0
dotnet-install: Installation finished
"C:\Program Files\PowerShell\7\pwsh.exe" -NoLogo -Sta -NoProfile -NonInteractive -ExecutionPolicy Unrestricted -Command & 'D:\a\_actions\actions\setup-dotnet\v5\externals\install-dotnet.ps1' -SkipNonVersionedFiles -Channel 8.0
dotnet-install: Downloaded file https://builds.dotnet.microsoft.com/dotnet/Sdk/8.0.416/dotnet-sdk-8.0.416-win-x64.zip size is 285854895 bytes.
dotnet-install: Either downloaded or local package size can not be measured. One of them may be corrupted.
dotnet-install: Extracting the archive.
dotnet-install: Note that the script does not ensure your Windows version is supported during the installation.
dotnet-install: To check the list of supported versions, go to https://learn.microsoft.com/dotnet/core/install/windows#supported-versions
dotnet-install: Installed version is 8.0.416
dotnet-install: Installation finished
```
So I think we have to keep this so that we have the .NET 10 SDK installed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add .NET 10 target, #1219 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#issuecomment-3565656887
Note that this PR reverts the hardcoded .NET 9 SDK since a new version is out that resolves that issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org