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

[browser] Enable webcil in Wasm SDK #84977

Merged
merged 3 commits into from
Apr 21, 2023
Merged

[browser] Enable webcil in Wasm SDK #84977

merged 3 commits into from
Apr 21, 2023

Conversation

maraf
Copy link
Member

@maraf maraf commented Apr 18, 2023

  • Support webcil format in Wasm SDK, which is shared with blazor
  • Enable webcil by setting msbuild property WasmEnableWebcil=true
  • Tested only locally with manually constructed SDK

@maraf maraf added arch-wasm WebAssembly architecture area-Build-mono labels Apr 18, 2023
@maraf maraf added this to the 8.0.0 milestone Apr 18, 2023
@maraf maraf requested a review from lambdageek April 18, 2023 13:04
@maraf maraf self-assigned this Apr 18, 2023
@ghost
Copy link

ghost commented Apr 18, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
  • TBD

TODO

  • Fix publish
Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-Build-mono

Milestone: 8.0.0

@maraf maraf marked this pull request as ready for review April 19, 2023 07:34
@maraf maraf requested review from lewing and radical as code owners April 19, 2023 07:34
@maraf
Copy link
Member Author

maraf commented Apr 19, 2023

Failures are not related

@radical
Copy link
Member

radical commented Apr 19, 2023

This needs a test in WBT.

@radical
Copy link
Member

radical commented Apr 19, 2023

This needs a test in WBT.

.. that does build/publish, and runs the app. There are existing tests that run blazor apps.

@maraf
Copy link
Member Author

maraf commented Apr 19, 2023

I haven't done hooking Wasm SDK in WBT yet. It would use the pack from official build, which doesn't have the required changes. I haven't even investigated how difficult it would be.

We can either test it manually or postpone it to preview5 probably.

@radical
Copy link
Member

radical commented Apr 19, 2023

I haven't done hooking Wasm SDK in WBT yet. It would use the pack from official build, which doesn't have the required changes. I haven't even investigated how difficult it would be.

Is this usable with the sdk currently, or are there other PRs to make this usable?

@maraf
Copy link
Member Author

maraf commented Apr 20, 2023

Is this usable with the sdk currently, or are there other PRs to make this usable?

This makes it usable in Blazor without any other change.
Yes, it depends on the SDK that needs manual tweaking at the moment. Also it requires aspnetcore from PR with unified boot

EDIT: steps to reproduce https://gist.github.com/maraf/a33651a9151fd129f0457e597cd54ee7

@lewing
Copy link
Member

lewing commented Apr 20, 2023

unified boot is open on sdk now, best case it will have made it through installer in 4-6 hrs dotnet/sdk#31912

@lambdageek
Copy link
Member

I think after this installer codeflow PR dotnet/installer#16179 , the aspnetcore bits are in. so WBT should work now if I try this PR, right?

@maraf
Copy link
Member Author

maraf commented Apr 21, 2023

It requires SDK from dotnet/sdk#31519. Anyway I can show you what needs to be patched. It's much easier that few days before.

@lambdageek
Copy link
Member

It requires SDK from dotnet/sdk#31519. Anyway I can show you what needs to be patched. It's much easier that few days before.

yea ok - if it's simpler to patch now, I'd like to try it. do we need that SDK PR to flow before we can review and merge this runtime PR? Would anything break if we merged this one early?

@maraf
Copy link
Member Author

maraf commented Apr 21, 2023

It's safe to merge anytime

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

LGTM
(I primarily looked at the C# code for the task and the .targets stuff. I didn't try a blazor build, but I'll try it later)

@lewing
Copy link
Member

lewing commented Apr 21, 2023

dotnet/installer#16197

@lewing lewing merged commit 92a74d2 into dotnet:main Apr 21, 2023
@lambdageek
Copy link
Member

lambdageek commented Apr 22, 2023

dotnet/installer#16197

After this. I can confirm webcil is working with blazor.

I did the usual WBT setup, but also had to edit Microsoft.NETCoreSdk.BundledVersions.props in artifacts/bin/dotnet-latest/... to change to the 8.0.0-dev version of the pack (thanks to @maraf for the hint):

<KnownWebAssemblySdkPack Include="Microsoft.NET.Sdk.WebAssembly.Pack"
                             TargetFramework="net8.0"
                             WebAssemblySdkPackVersion="8.0.0-dev" />`

and setup a nuget.config to point to my artifacts/packages/Release/Shipping dir.

but after all that, dotnet new blazorwasm and then dotnet run and dotnet publish make versions of the blazor template that serve .webcil files and not .dlls.

@radical is it possible to make it so WBT sets up dotnet-latest so it can just use the Sdk Pack from artifacts instead of needing to edit the BundledVersions.props by hand?

@maraf
Copy link
Member Author

maraf commented Apr 22, 2023

I think he already has the change stacked up and was just waiting for everything to flow.

@maraf maraf deleted the WasmSdkWebCil branch April 24, 2023 07:24
@ghost ghost locked as resolved and limited conversation to collaborators May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants