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.

Sign up for GitHub

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

Jump to bottom

Improve resolving runtime pack in WasmApp targets #55258

Merged
merged 2 commits into from
Jul 13, 2021
Merged

Improve resolving runtime pack in WasmApp targets #55258

merged 2 commits into from
Jul 13, 2021

Conversation

akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Jul 7, 2021 •
edited
Loading

Use similar logic to eng/targetingpacks.targets in WasmApp.InTree.targets and WasmApp.LocalBuild.targets.

Also set UseMonoRuntime=true to make sure we get the Mono-based runtime pack.

@akoeplinger akoeplinger requested a review from ViktorHofer July 7, 2021 11:43
@akoeplinger akoeplinger requested a review from marek-safar as a code owner July 7, 2021 11:43
@ghost
Copy link

ghost commented Jul 7, 2021

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

Import targetingpacks.targets in WasmApp.InTree.targets and use similar logic in WasmApp.LocalBuild.targets.

Also set EnableRuntimePackDownload=false in targetingpacks.targets instead of manually messing with the PackageDownload items since this property can be used now.

Author: akoeplinger
Assignees: -
Labels:

area-Infrastructure-mono
Milestone: -

@SamMonoRT SamMonoRT requested review from lewing and radical July 7, 2021 13:16
ViktorHofer
ViktorHofer approved these changes Jul 7, 2021
View reviewed changes
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

The targetingpack.targets changes LGTM. Seems like there are some other build failures though.

@akoeplinger akoeplinger force-pushed the wasm-runtimepack branch 2 times, most recently from a725f4d to 7c31a55 Compare July 8, 2021 10:59
@akoeplinger
Copy link
Member Author

Had to take out the EnableRuntimePackDownload=false change since it caused ResolvedRuntimePack to not be populated so overriding it didn't work anymore.

@akoeplinger
Improve resolving runtime pack in WasmApp targets
38bf9b4 
Import targetingpacks.targets in WasmApp.InTree.targets and use similar logic in WasmApp.LocalBuild.targets.
@naricc naricc mentioned this pull request Jul 12, 2021
Closed
@akoeplinger akoeplinger merged commit b2a670b into dotnet:main Jul 13, 2021
@akoeplinger akoeplinger deleted the wasm-runtimepack branch July 13, 2021 15:12
@akoeplinger akoeplinger mentioned this pull request Jul 13, 2021
Merged
@runfoapp runfoapp bot mentioned this pull request Jul 15, 2021
Closed
@ManickaP ManickaP mentioned this pull request Jul 20, 2021
Merged
@karelz karelz mentioned this pull request Jul 28, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ViktorHofer ViktorHofer ViktorHofer approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar

@lewing lewing Awaiting requested review from lewing

@radical radical Awaiting requested review from radical

Assignees
No one assigned
Labels
area-Infrastructure-mono
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@akoeplinger @ViktorHofer