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

!fixup! Remove more WebAssembly template mentions #33497

Merged
merged 2 commits into from
Jun 16, 2021

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Jun 12, 2021

  • follow up to part of b6c411f
    • more dangling references to a non-existent project

- follow up to part of b6c411f
  - more dangling references to a non-existent project
@dougbu dougbu requested a review from ryanbrandenburg as a code owner June 12, 2021 03:01
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 12, 2021
@dougbu dougbu requested a review from a team June 12, 2021 03:01
@dougbu
Copy link
Member Author

dougbu commented Jun 12, 2021

Would expect to hit problems continuously since 6c16632 went in in February (v3.1.13). But, change didn't reliably cause

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(1947,5): warning : The referenced project '../ComponentsWebAssembly.ProjectTemplates/Microsoft.AspNetCore.Components.WebAssembly.Templates.csproj' does not exist. [D:\workspace\_work\1\s\src\ProjectTemplates\test\ProjectTemplates.Tests.csproj]

all the time. Has occurred in official builds e.g.

@dougbu
Copy link
Member Author

dougbu commented Jun 12, 2021

@mmitche @wtgodbe (or 3.1.17 CQB if neither of you) should we get this into v3.1.17❔

@dougbu dougbu added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jun 12, 2021
@dougbu
Copy link
Member Author

dougbu commented Jun 12, 2021

@Pilchie I'm thinking tell-mode for either v3.1.17 (if we have time) or v3.1.18. Your thoughts❔

@Pilchie
Copy link
Member

Pilchie commented Jun 13, 2021

👍🏻 for tell-mode

@dougbu dougbu added the tell-mode Indicates a PR which is being merged during tell-mode label Jun 13, 2021
@dougbu
Copy link
Member Author

dougbu commented Jun 14, 2021

@dotnet/aspnet-build I'm hesitant to merge this due to the failures in https://dev.azure.com/dnceng/public/_build/results?buildId=1184332&view=results I think I've seen similar problems in other release/3.1 builds but am not positive. The binary log is also 429 MB and is still opening on my machine. Anyone seen this before❔ Any objections to retrying the failed job❔

@rainersigwald any suggestions you have would of course be much appreciated❕

Oddly, the following seemed to occur while desktop msbuild was shutting down after a successful build:

Microsoft.Build.Shared.InternalErrorException: MSB0001: Internal MSBuild Error: We shouldn't be accessing the ProjectInstance when the configuration is cached.

@dougbu
Copy link
Member Author

dougbu commented Jun 14, 2021

/btw ping reviewers 😺

@dougbu
Copy link
Member Author

dougbu commented Jun 14, 2021

Binary log finished opening as I submitted the previous comment (of course). No indication of any problems beyond our usual "double writes" of a few blah.sourcelink.json files and two copies of xunit.abstractions.dll. (Unrelated question: Are the double writes anything we should be working to fix❔ If so, what's the recommended fix❔)

@rainersigwald
Copy link
Member

That error is dotnet/msbuild#6436 which is fixed in 16.9.7 and 16.10.0. Unfortunately, Arcade updates obliterated the workaround 75b7808. You should probably pick that back up until you can update the toolset.

Copy link
Contributor

@ryanbrandenburg ryanbrandenburg left a comment

Choose a reason for hiding this comment

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

Seems OK to me, though I'm not plugged into this work anymore.

@dougbu
Copy link
Member Author

dougbu commented Jun 14, 2021

Unfortunately, Arcade updates obliterated the workaround 75b7808. You should probably pick that back up until you can update the toolset.

Either we must get the workaround into the Arcade SDK or we should update the toolset -- soon. Which toolset did you mean @rainersigwald

In the meantime, I'll retry the failing job…

@rainersigwald
Copy link
Member

Which toolset did you mean @rainersigwald Rainer Sigwald FTE❔

The Visual Studio version installed on your worker machines, I believe.

@dougbu
Copy link
Member Author

dougbu commented Jun 15, 2021

The Visual Studio version installed on your worker machines, I believe.

According to the VS2019 Upgrade Schedule (see Core-Eng rollout emails), the agents are at 16.9.5 and will stay that way for at least a week. I doubt switching to the .Pre build agents would be enough (they're at 16.10.0 Preview 3) and we'd only be able to use "scouting" agents (at 16.10.0) in public builds. Scouting pool for public builds won't be enough because we run about the same stuff (or more) in our internal builds of 'release/3.1'. Ah well…

I'm not thrilled about updating eng/common/ files outside an Arcade update but will do that if the next retry fails (too).

@markwilkie @ilyas1974 @MattGal the workaround we had basically switched to use the 64-bit msbuild when using desktop msbuild. Would Core-Eng object to a switch enabling that going forward in your release/3.x branch❔ A "yes" would make me happier about re-enabling our local hack for now.

@ilyas1974
Copy link

I don't see a problem with adding a switch to use the 64bit version of msbuild, but I will defer to the others on my team on that question. Another option is to bring the VS 16.10.0 image "forward" into the release queue early. If that is something that is needed, we can contact Tactics and get their by-off.
cc: @tkapin

@rainersigwald
Copy link
Member

Another option is to bring the VS 16.10.0 image "forward" into the release queue early.

Note that there's a few bugs that have affected external customers in MSBuild 16.10.0 -- ideally we'd roll out directly to 16.10.2 to avoid hitting those for dotnet teams. Arcade keeps folks from hitting the biggest bug fixed in 16.10.1, but some of the .2 content might be relevant.

I don't see a problem with adding a switch to use the 64bit version of msbuild

Adding a switch should be fine; doing it by default is riskier since some (incorrectly authored) tasks don't work in a 64-bit MSBuild environment. I'd be fine changing the default to 64-bit in Arcade 6 but in servicing it's not my favorite idea.

@wtgodbe
Copy link
Member

wtgodbe commented Jun 15, 2021

If this fixes any transient build failures & it's green today then we can take it for July, otherwise let's hold until August

@dougbu
Copy link
Member Author

dougbu commented Jun 15, 2021

I hacked around the problem by disabling the Windows x86 binary log. That may be enough between now and when we move to a recent-enough VS installation or I add the Arcade switch.

I see dotnet/aspnetcore's later branches don't create binary logs for Windows x86. So maybe this hack is enough❔

I'd like to make the Arcade change everywhere anyway because it seems likely to improve build performance, even though binary logs are smaller in 6.0. (Is that valid @rainersigwald❔) At least dotnet/aspnetcore's other branches are less pressing because they do less in a single msbuild invocation e.g. doing more with dotnet msbuild (separating the native builds) and not building test projects in many build steps (including the one that's fairly consistently failing here). Thoughts❔

@rainersigwald
Copy link
Member

I hacked around the problem by disabling the Windows x86 binary log

What makes you think the binlog is related here? I wouldn't expect it to be a major factor.

I'd like to make the Arcade change everywhere anyway because it seems likely to improve build performance, even though binary logs are smaller in 6.0. (Is that valid @rainersigwald Rainer Sigwald FTE❔)

Binary logs got smaller in 16.10 but haven't significantly changed (yet) for 17.0. Performance-wise, 32- versus 64-bit MSBuild might make a difference if the "paging" approach we take is actually costing you significant time--but it often doesn't, and there are risks of switching to 64-bit in terms of functionality, which is why I don't recommend switching to it everywhere in servicing. Probably a great idea to do so for Arcade 6 though.

@dougbu
Copy link
Member Author

dougbu commented Jun 15, 2021

I hacked around the problem by disabling the Windows x86 binary log

What makes you think the binlog is related here? I wouldn't expect it to be a major factor.

Well, my hack failed in any case. I thought it would help because dotnet/msbuild#6436 mentions memory pressure and binary logs are certainly not free.

which is why I don't recommend switching to it everywhere in servicing

Agreed. I wasn't proposing anything other than an off-by-default switch we could try using in this repo.

- will be mostly overwritten in next 3.1 Arcade update
@dougbu dougbu force-pushed the dougbu/build.break/3.1 branch from c3fb8e8 to 85280fb Compare June 16, 2021 02:25
@dougbu
Copy link
Member Author

dougbu commented Jun 16, 2021

https://dev.azure.com/dnceng/public/_build/results?buildId=1189546&view=results built successfully though that's not reflected here. Getting this in…

@dougbu dougbu merged commit 946e916 into release/3.1 Jun 16, 2021
@dougbu dougbu deleted the dougbu/build.break/3.1 branch June 16, 2021 04:01
@wtgodbe wtgodbe added this to the 3.1.17 milestone Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants