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

[wasm] Wasm.Build.Tests - build projects for net8.0 by default #79254

Merged
merged 15 commits into from
Dec 9, 2022

Conversation

radical
Copy link
Member

@radical radical commented Dec 5, 2022

  • Build Wasm.Build.Tests for tfm=net8.0 by default
  • Remove workarounds added earlier when we had 8.0 sdk with tfm=net7.0
  • Use a shared cache for installing workloads for tests, to make it faster

@radical radical added arch-wasm WebAssembly architecture area-Build-mono labels Dec 5, 2022
@ghost
Copy link

ghost commented Dec 5, 2022

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

Issue Details

null

Author: radical
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: -

@ghost ghost assigned radical Dec 5, 2022
@radical radical requested review from steveisok and lewing December 6, 2022 22:13
@radical radical marked this pull request as ready for review December 6, 2022 23:35
@radical radical requested a review from pavelsavara as a code owner December 6, 2022 23:35
@@ -19,29 +19,31 @@ internal sealed class PackageInstaller
private string _nugetConfigContents;
private TaskLoggingHelper _logger;
private string _packagesDir;
private bool _cleanPackagesdir;
Copy link
Member

Choose a reason for hiding this comment

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

should this be true by default ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is now set _cleanPackagesdir = packagesPath is null; in the constructor. So, if packagesPath is set, IOW, we are using a shared packages path then we don't want it to be cleaned.
Also, the main task now creates all the temp files under a top level temporary directory which gets removed at the end. And that would include these paths too.

Copy link
Member

Choose a reason for hiding this comment

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

Very cool. Would it make sense to not delete the directory when running on local machine && something failed ?

Copy link
Member

Choose a reason for hiding this comment

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

I guess SKIP_PROJECT_CLEANUP=1 still works ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I was disabling it in code when needed! I can add a similar env var for this. SKIP_PROJECT_CLEANUP is for WBT.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll include this in a follow up PR.

@radical
Copy link
Member Author

radical commented Dec 9, 2022

The llvmfullaot failures are #79255 .
The others are flagged as known issues by Build Analysis.

@radical radical merged commit 1857c91 into dotnet:main Dec 9, 2022
@radical radical deleted the wbt-8 branch December 9, 2022 16:17
@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2023
@lewing
Copy link
Member

lewing commented Jan 9, 2023

this caused the perf pipeline to switch back to testing against net7 runtimes again and we haven't had perf data on net8 since then dotnet/perf-autofiling-issues#10700

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.

3 participants