Uh oh!
There was an error while loading. Please reload this page.
Reverting change in source generators that require Roslyn 4.4 until we have a VS version with the compiler fix to consume them. - #75490
Conversation
…e have a VS version with the compiler fix to consume them.
ghost
commented
Sep 12, 2022
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
joperezr
commented
Sep 12, 2022
@danmoseley we would appreciate a sign off so we can merge and unblock the ingestion from runtime to installer. As the description says, we will do this once RC1 and 17.4 P2 ships. |
danmoseley
commented
Sep 12, 2022
approved (of course) Do I understand correctly that with this reverted, we are using the polyfill. So we don't expect a perf issue, it's clumsy but shippable if we must -- is that right? |
joperezr
commented
Sep 12, 2022
Yes, this is a direct revert, and this is now falling back to use polyfill again. We weren't expecting a perf issue, on the other hand, we were expecting a perf gain without the polyfill, but given that late roslyn bug fix and the fact that it didn't make it to 17.4 preview 1 we will need to hold off on this.
Yes, this is shippable, but we would rather try again after RC1 ships for the same reasons we wanted to do it in the first place: we expect to see a significant perf increase, we want to build against newer compilers (which we have to do anyway), we want to use Roslyn APIs which are better tested than our polyfill approach. |
carlossanlop
commented
Sep 13, 2022
Approved, signed off, CI failure is unrelated (System.Net.Http.Functional.Tests, known). |
When #74822 got merged we realized that the compiler that is now required for using the source generators is not yet present in any VS preview version yet (or a functioning SDK) so we are instead going to wait till after RC1 and 17.4 Preview 2 ships to make this change. Otherwise, we would need to disable all of the SDK tests that run against VS MSBuild, which is not worth the risk of doing.
cc: @jkoritzinsky@danmoseley@jaredpar@CyrusNajmabadi