vstest: build for .NET 5 #1834
Merged
vstest: build for .NET 5 #1834
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
CI now fails with: |
|
When you get new prebuilts, you either fix them, or add them to the baseline to defer fixing them by copying the generated baseline over the corresponding checked-in baseline. But there's another pressing problem here, smoke-test is failing (in the tarball builds, the prod build doesn't fail on baseline failure, so you can see both issues): Binlogs: testing-smoke-binlogs-1834.zip Failed task: |
|
I'm taking a look at this now. DanS suggested we might have better luck adding the new Microsoft.NETCore.App version to source-build-reference-packages. |
|
I think this is the key error (in order to continue switching over to Maybe the place to look is how exactly |
|
CI fail when building installer: |
|
Hmm. The node from the binlog (Build (31).zip): Doing a general binlog search for that I guess the wrong one is being found? I'm not sure what exactly is the diff from this to a working build. (E.g. does the I see this interesting looking bit in Maybe adding |
|
It looks like this reference assembly is built by |
|
I think these files may be the result of https://github.com/microsoft/vstest/blob/e3c7e106fd7fe3671768e1998718fffd3f1a920b/scripts/build.sh#L344. I'll look into it further tomorrow. |
Great! Unexpected, needs more investigation: |
|
CI jobs aren't starting for my latest commit. |
This comment has been minimized.
This comment has been minimized.
|
Not sure why that happened. 😕 ( |
This is getting started with dotnet/source-build-reference-packages#171, so it might be better to stick with that given the additional prebuilts this seems to have brought in. (If they can be fixed, that's good, but at that point the shift to |
I'm surprised by these prebuilts, I have no clue how I'm introducing them. |
|
If you go to the In that file, It also tells you where the project is: (The I don't see any obvious reason for this to happen, would probably need to build locally to poke around efficiently. |
It's due to some manual copying that happens in the build.sh script here: |
|
Looking better: |
|
@dagood do you have some thoughts about the new packages? What does the Maybe testhost.deps.zip tells you something interesting? It's testhost.deps.json from |
|
That I think how it works is that I'm able to repro the large amount of assets with this simple proj: <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net5.0</TargetFramework>
<RuntimeIdentifier>win7-x86</RuntimeIdentifier>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Newtonsoft.Json" Version="9.0.1" />
</ItemGroup>
</Project>If we compiled |
This is just what we replace |
|
Ah, noticed that there was a patch previously that removed |
|
CI is happy now. @dagood thank you for your help! |
dagood
approved these changes
Nov 5, 2020
dagood
reviewed
Nov 5, 2020
repos/vstest.proj
Outdated
| @@ -17,7 +17,6 @@ | |||
| <PackagesOutput>$(ProjectDirectory)/artifacts/$(Configuration)/packages</PackagesOutput> | |||
| <RepoApiImplemented>false</RepoApiImplemented> | |||
| <DeterministicBuildOptOut>true</DeterministicBuildOptOut> | |||
| <GlobalJsonFile>$(ProjectDirectory)global.json</GlobalJsonFile> | |||
There was a problem hiding this comment.
Not sure why I didn't notice this earlier... this seems strange, it should disable some of our override logic. Why is it removed?
There was a problem hiding this comment.
I'll add it back. I was not sure if I could keep it since the global.json file specified a 3.1 sdk. What gets done to the file specified here?
There was a problem hiding this comment.
It has a 5.0 SDK now: https://github.com/microsoft/vstest/blob/b195e2589980861425b331e73a859252c3f2b71a/global.json. But even with a 3.1 SDK, we need this--source-build overrides it with the 5.0 SDK that we can actually use at the time. (3.1 would be a prebuilt that the repo would try to download, or if we block the download, the repo may fail to build with the "mismatched" 5.0 SDK we actually have.) The override would be the prebuilt tooling version for prod builds, but also an N-1 for servicing builds, or an N version for bootstrapping.
|
Thanks! Merging this should remove the need to put together source-build-reference-packages versions of the 2.1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
attempt to eliminate .NET 2.1 host packages used by vstest.
cc @dagood @crummel @dseefeld @omajid