Update Ix.NET for .NET 8.0 sdk #2135
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The code generating the IQ-space versions of interfaces and operators in both Rx and Ix were written before nullability was added to C#. It appears that the workaround for this was to manually modify the generated files and then avoid ever running the generators again. Unfortunately, the .NET 8.0 SDK was detecting errors in the generated code. Rather than repeat the cycle of patching this up manually again, I decided to fix the generator to be nullable-aware. This is a bit of a hack because we don't do a thorough job of processing nullability attributes (which is surprisingly difficult as a result of nullability not really being part of .NET's type system), but it works for the types we actually need to process. This has also detected a couple of mistakes in the manual edits to the generator code.
Removed old NO_ARRAY_EMPTY conditional sections as no build targets now set that. Remove use of MSBuild.Sdk.Extras
Reinstate netcoreapp3.1 target so that we test the netstandard2.1 build.
As far as it was possible to tell, the only thing we were using MSBuild.Sdk.Extras for was to associate reference assembly projects with their corresponding main projects. This was what was breaking the build, and it doesn't seem to be necessary. The .NET Runtime Library source code just has a "ref" folder each functionality area (and Ix would effectively be a whole area in itself) and for each real csproj, there's another in this folder with the same name set up to build the references. The main thing MSBuild.Sdk.Extras seemed to be trying to do was automatically build the reference assemblies for us (which is what broke. Since the main CI build seems to go out of its way to avoid publishing the reference assemblies, it seems that nothing is using them anyway. Also: * Replace erroneous net4.8 TFM with net48 * Move
HowardvanRooijen
previously approved these changes
Jul 2, 2024
Necessary now that we can't rely on MSBuild.Extras.SDK
Apparently the configuration setting works only for things like pack, not build!
Ix.NET/Source/System.Linq.Async.Queryable.Tests/System.Linq.Async.Queryable.Tests.csproj
Show resolved
Hide resolved
Ix.NET/Source/refs/System.Interactive.Providers/System.Interactive.Providers.csproj
Show resolved
Hide resolved
HowardvanRooijen
approved these changes
Jul 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ix.NET had not been updated since the .NET 8.0 SDK shipped, and it turns out that we have:
MSBuild.Sdk.Extrashas not been updated since .NET 5.0, andThis PR removes the use of
MSBuild.Sdk.Extras. Nobody is maintaining it any more, so it's no longer viable to depend on it.This also makes numerous small changes to comply with the majority of SDK diagnostics.
It leaves a small number in because they seem to indicate some genuine potential problems around cancellation. (They coincide with some "REVIEW" comments in the code indicating that possible problems here were already known about.)