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

Set main to run net7.0 until sdk+runtime are supported #2695

Conversation

LoopedBard3
Copy link
Member

Set main to run net7.0 until sdk+runtime are supported per: dotnet/runtime#77913 (comment).

@radical is this the fix you had in mind from the above comment?

@LoopedBard3 LoopedBard3 self-assigned this Nov 4, 2022
@radical
Copy link
Member

radical commented Nov 4, 2022

Honestly, I'm not sure what the correct fix would be, and there might not be one for now. AFAIU, we are not quite there with net8.0 project support, w.r.t the SDK, and runtime.
So, for now, reverting the earlier state would make sense.

What would be the impact of this change? Wouldn't we also need to change the tfm being passed to the command line?
It would be a good idea to also confirm that the correct assemblies, and runtime pack is being used from the workload.

cc @ericstj @carlossanlop

@radical
Copy link
Member

radical commented Nov 4, 2022

Also, before merging this one do make sure to run a dotnet-runtime-perf+wasm build with this change.

@LoopedBard3
Copy link
Member Author

@LoopedBard3
Copy link
Member Author

Changing the main branch runs back to net7.0 and 7.0 branch will cause things to build with net7.0.

@LoopedBard3 LoopedBard3 marked this pull request as ready for review November 4, 2022 22:09
@LoopedBard3
Copy link
Member Author

It looks like this fixed the wasm runs. Waiting for the rest of the test runs to finish and a review.

@LoopedBard3
Copy link
Member Author

One wasm run succeeded and one had a handful of failures.

@LoopedBard3
Copy link
Member Author

@radical Do you know the ETA for having net8.0 packages for wasm?

@radical
Copy link
Member

radical commented Nov 5, 2022

@radical Do you know the ETA for having net8.0 packages for wasm?

AFAIK, we are waiting for other things to be in place, like a PR that would change the default TFM in dotnet/runtime to net8.0. And we can fix anything that breaks in wasm.

Copy link
Contributor

@caaavik-msft caaavik-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving in the case that the alternative solution to use net7.0 for just wasm is not suitable

@@ -3,8 +3,8 @@
class ChannelMap():
channel_map = {
'main': {
'tfm': 'net8.0',
'branch': '8.0',
'tfm': 'net7.0',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used only to decide which tfm to use on dotnet/runtime main branch?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that is correct

@LoopedBard3
Copy link
Member Author

Closing to merge #2696 as a fix instead. This will allow wasm to build targeting net7 while the rest of the builds target net8.0.

@LoopedBard3 LoopedBard3 closed this Nov 5, 2022
@LoopedBard3 LoopedBard3 deleted the SetMainToUseNet7.0Untilsdk+runtimebothwork branch November 5, 2022 08:07
@carlossanlop
Copy link
Member

It seems this issue is related to dotnet/runtime#77899 . FYI, I merged it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants