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

[release/7.0][wasm] Fix Wasm.Build.Tests with stable 7.0.0 #76939

Merged
merged 12 commits into from
Nov 2, 2022
Merged

[release/7.0][wasm] Fix Wasm.Build.Tests with stable 7.0.0 #76939

merged 12 commits into from
Nov 2, 2022

Conversation

radical
Copy link
Member

@radical radical commented Oct 12, 2022 •
edited
Loading

Wasm.Build.Tests started to fail when installing the workload manifests with #76849 which moved to Stable branding for GA. This caused the workload installation to fail because the targets were using the incorrect manifest version - 7.0.0-ci instead of 7.0.0. It is fixed here by setting that early, and using that in all the relevant places.
This revealed a second issue - dotnet/sdk#28607 - which caused tests to break, and will be fixed by dotnet/sdk#28627 .

Fixes #76908 .

Customer Impact

No impact, since this is a tests-only change.

Testing

Running Wasm.Build.Tests.

Risk

No risk, since this is a tests-only change.

@ghost ghost assigned radical Oct 12, 2022
@ghost
Copy link

ghost commented Oct 12, 2022

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

Issue Details

Set the manifest package version to use (7.0.0 instead of 7.0.0-ci) early.

Fixes #76908 .

Author: radical
Assignees: radical
Labels:

area-Infrastructure-mono
Milestone: -

@radical radical added arch-wasm WebAssembly architecture test-failure and removed area-Infrastructure-mono labels Oct 12, 2022
@ghost
Copy link

ghost commented Oct 12, 2022

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

Issue Details

Set the manifest package version to use (7.0.0 instead of 7.0.0-ci) early.

Fixes #76908 .

Author: radical
Assignees: radical
Labels:

arch-wasm, test-failure
Milestone: -

@radical radical marked this pull request as ready for review October 12, 2022 17:38
@radical radical added this to the 7.0.x milestone Oct 12, 2022
@carlossanlop
Copy link
Member

Whenever you get a chance, can you please take a look at the CI failures? LMK when this is good to merge.

@radical
Copy link
Member Author

radical commented Oct 12, 2022

Working on that, I'll ping you when this is ready.

This was referenced Oct 12, 2022
Closed
Closed
@radical radical added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 14, 2022
@radical
Copy link
Member Author

radical commented Oct 19, 2022

The Wasm.Build.Tests failures will be fixed by dotnet/sdk#28627 . Once, an installer with that change is available, this should be green, and can be merged.

@radical
[wasm] Fix Wasm.Build.Tests with stable 7.0.0
63f1516 
Set the manifest package version to use (`7.0.0` instead of `7.0.0-ci`)
early.

Fixes dotnet#76908 .
@radical
workloads-testing.targets: ignore symbols.nupkgs when looking for run…
3686f61 
…time packs
@radical
Fix WBT to have the correct version for WORKLOAD_PACKS_VER
be2c112
@radical
[wasm] Emit an error if PackageVersionForWorkloadManifests property i…
e56993a 
…s not set
@radical
workloads-testing: Get the PackageVersion at build time
3df9edc 
.. because it is not set at evaluation time. So, get it in a target, and
move the WorkloadIds to a target too.
@radical
Ensure the version is set before generating the runscript for WBT
e2a0cad
@radical
[wams] Update Mono.Toolchain's WorkloadManifest.targets to not depend on
5f20a22 
.. the import order.
@radical
revert workloadManifest.target changes
69caec4
@radical radical mentioned this pull request Oct 31, 2022
Merged
@radical radical mentioned this pull request Oct 31, 2022
Merged
@carlossanlop
Copy link
Member

There are failures showing up in the wasm legs. Are any related to this change?

@carlossanlop carlossanlop added the blocking Marks issues that we want to fast track in order to unblock other important work label Oct 31, 2022
@carlossanlop
Copy link
Member

This is blocking the runtime branding PR as described here: #77549 (comment)

@build-analysis build-analysis bot mentioned this pull request Oct 31, 2022
Closed
@LoopedBard3 LoopedBard3 mentioned this pull request Nov 1, 2022
Merged
@radical radical mentioned this pull request Nov 1, 2022
Closed
@radical
Copy link
Member Author

radical commented Nov 2, 2022

@carlossanlop this is good to merge. The WasmBuildTests seem to be stuck on SIMD tests, but those are not affected by this at all.

@radical
Copy link
Member Author

radical commented Nov 2, 2022

And once this is merged, then we can merge upstream into #77549 .

@radical radical removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 2, 2022
@carlossanlop carlossanlop merged commit 78cf32a into dotnet:release/7.0 Nov 2, 2022
@radical radical deleted the fix-wbt-7 branch November 2, 2022 05:56
This was referenced Nov 2, 2022
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lewing lewing lewing approved these changes

@steveisok steveisok steveisok approved these changes

@akoeplinger akoeplinger Awaiting requested review from akoeplinger

Assignees

@radical radical

Labels
arch-wasm WebAssembly architecture blocking Marks issues that we want to fast track in order to unblock other important work test-failure
Projects
None yet
Milestone
7.0.x
Development

Successfully merging this pull request may close these issues.
4 participants
@radical @carlossanlop @lewing @steveisok