Avoid spurious references to Microsoft.Bcl.AsyncInterfaces

[WeihanLi]

Update System.Linq.Async dependency condition for Microsoft.Bcl.AsyncInterfaces

[WeihanLi]

Is there someone having a look at this wrong dependency condition 👀

[danielcweber]

@idg10 May I kindly ask to give this PR a quick and final review, it fixes a unnecessary dependency on Microsoft.Bcl.AsyncInterfaces on platforms where it's not needed due to a wrong condition on the dependency. Thanks a lot.

[idg10]

There doesn't seem to be a clear discussion of what problem this sets out to solve, so I'm going to attempt to reverse engineer the intention from the PR.

The existing code was supposed to embody this idea:

Modern targets have IAsyncEnumerable<T> and related interfaces built in, so they don't need a reference to Microsoft.Bcl.AsyncInterfaces.

Unfortunately, this was expressed by equating "modern targets" with ".NET Core 3.1 and .NET Standard 2.1". At the time that code was written, those were indeed the most modern targets.

However, those targets are now, respectively, "old, and out of support" and "still current, but of decreasing relevance because the 'one .NET' initiative means that .NET Standard 2.1 is no longer very interesting."

Because those definitions of "modern targets" were not updated in this particular file, the problem now is that actually modern targets (and in fact anything "modern" enough to be a currently supported version of .NET) get misidentified as "not modern" and so they get this spurious reference.

This PR resolves this by flipping the logic, so that it works this way:

Old targets don't have IAsyncEnumerable<T> and related interfaces built in, so they need a reference to Microsoft.Bcl.AsyncInterfaces.

The set of "old targets" here is small, and known, and it won't change. It's just: .NET Framework 4.8 and .NET Standard 2.0. Those are the only targets we'll ever support that will have this requirement, so the advantage of flipping the logic this way is that this logic will continue to be correct as future versions add newer targets. So whereas the existing code has demonstrably gone stale, with the effect that we get spurious references to Microsoft.Bcl.AsyncInterfaces in projects that don't need it, the change will not require any further updates as we add new targets in the future.

[idg10]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[danielcweber]

Thanks a lot. There was a bit of discussion in #1941 which I then closed in favor of this earlier PR.

[masonwheeler]

Now that this is merged, can we expect an updated NuGet release anytime soon?

[wessleym]

@danielcweber, @idg10, and @WeihanLi, sorry to bother you, but I'm also interested in @masonwheeler's question. I would like to see a NuGet package release so I can avoid the Microsoft.Bcl.AsyncInterfaces dependency in my .NET 8 projects. Thank you.

[danielcweber]

Kindly pinging @idg10 for a 6.0.2 release of System.Linq.Async containing this specific fix.

[idg10]

OK, I'm sorry this took so long. I've had very little time to work on Rx in the last 6 months, and this turned out to be more involved than simply pressing the button to make a new release, so it has got stuck behind some other things we needed to do to keep the build process aligned with current tools.

The specific problem, if you're interested, is that the build agents have all changed to Ubuntu 24, which not only broke the Ix.NET build (which is the one that creates System.Linq.Async), it did so in a way that required us to change how we build it. Also, the C# compiler appears recently to have fixed a bug that it turns out our tests were inadvertently relying on, so I've had to fix those too.

However, that pipeline is now working again. I've built a provisional 6.0.2 but haven't yet released it to NuGet. If you want to test that it meets your requirements, it's available now from the Rx.NET preview NuGet feed:

https://pkgs.dev.azure.com/dotnet/Rx.NET/_packaging/RxNet/nuget/v3/index.json

It looks to me to have fixed the problem. Here's the problem in 6.0.1:

And here's the 6.0.2 package up on the preview feed:

As you can see, the net6.0 target no longer has the dependency on Microsoft.Bcl.AsyncInterfaces.

So I believe this is done, but I'll give it a few days before pushing to NuGet in case anyone on this thread wants to verify that this
works for them.

[wessleym]

@idg10, thank you!

[idg10]

Due to some wrangling with the release process that turned out not to be an Rx repo issue after all (the .NET foundation needed to change their config) we ended up bumping the version number. Version 6.0.3 on NuGet has this fix.