Source build patches by sfoslund · Pull Request #17417 · dotnet/sdk

sfoslund · 2021-05-04T17:47:29Z

This project restores a prebuilt Microsoft.Win32.Registry reference, but it doesn't end up in the SDK: the DLL from the net5.0 shared framework is already present. Remove this unnecessary PackageReference to remove the prebuilt.

ghost · 2021-05-04T17:47:32Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

MichaelSimons · 2021-05-04T20:51:24Z

@sfoslund, Can you please remove the source-build patches and the target that applies them as part of this PR as well? They are no longer needed with these changes.

sfoslund · 2021-05-04T20:55:07Z

/azp run

azure-pipelines · 2021-05-04T20:55:15Z

Azure Pipelines failed to run 1 pipeline(s).

wli3 · 2021-05-04T21:14:34Z


  <Target Name="PublishNetSdks"
          BeforeTargets="Build">
+    <ItemGroup>


why websdk is singled out

I don't know, @MichaelSimons do you?

It seems like it would be preferable to have project references to these from redist.csproj (with ReferenceOutputAssembly set to false).

sfoslund · 2021-05-04T23:24:14Z

error MSB4018: The "ResolvePackageAssets" task failed unexpectedly. [C:\code\sdk\artifacts\tmp\Debug\When_app_proj---CDDE0EAB\Test\XUnitTestProject.csproj]
error MSB4018: System.MissingMethodException: Method not found: 'NuGet.Versioning.NuGetVersion NuGet.ProjectModel.LockFileTargetLibrary.get_Version()'.

One of the dotnet tool tests is failing with an odd error that I haven't been able to debug, @wli3 do you know what this could be?

sfoslund · 2021-05-05T19:26:49Z

@MichaelSimons we're seeing test failures because of the dotent-hello test packages that are removed. Why do they need to be removed? Source build doesn't deal with test assets anyways, does it?

MichaelSimons · 2021-05-06T13:28:01Z

Why do they need to be removed? Source build doesn't deal with test assets anyways, does it?

They don't need to be removed, they just need to be excluded from source-build. I can't answer why they were removed because yes, source-build doesn't deal with tests. Maybe add them back we will see what prebuilts if any are added and we can address it at that time when we have the full context.

sfoslund · 2021-05-06T20:42:26Z

/__w/1/s/artifacts/source-build/self/src/source-build.slnf(1,1): error MSB4025: The project file could not be loaded. Data at the root level is invalid. Line 1, position 1.

@MichaelSimons all legs but the source build leg seem to be okay. Have you seen this before? We started getting this error with 45b7b2b but I'm not sure how that could be causing it.

dsplaisted · 2021-05-07T19:52:54Z


  <Target Name="PublishNetSdks"
          BeforeTargets="Build">
+    <ItemGroup>


It seems like it would be preferable to have project references to these from redist.csproj (with ReferenceOutputAssembly set to false).

sfoslund · 2021-05-12T16:19:08Z

/__w/1/s/artifacts/source-build/self/src/source-build.slnf(1,1): error MSB4025: The project file could not be loaded. Data at the root level is invalid. Line 1, position 1.

@MichaelSimons Michael Simons (.NET) FTE all legs but the source build leg seem to be okay. Have you seen this before? We started getting this error with 45b7b2b but I'm not sure how that could be causing it.

Ping @MichaelSimons do you have any thoughts here?

MichaelSimons · 2021-05-12T16:22:58Z

@sfoslund - Sorry I've been oof for a few days. I'm catching up and will take a look this afternoon.

MichaelSimons · 2021-05-12T23:13:44Z

/azp run

azure-pipelines · 2021-05-12T23:13:57Z

Azure Pipelines successfully started running 1 pipeline(s).

MichaelSimons · 2021-05-13T15:31:24Z

@sfoslund - Looks like this PR renamed a project which the source-build.slnf needs to respond to. Here are the changes which build locally for me after pulling the latest from main. I am going to log an issue for the build error as it is not helpful at all!

sfoslund · 2021-05-13T15:56:07Z

@MichaelSimons great, thank you!

sfoslund · 2021-05-13T16:56:54Z

@MichaelSimons we're green, does everything look okay to you to merge?

MichaelSimons · 2021-05-13T17:54:28Z

@MichaelSimons we're green, does everything look okay to you to merge?

@sfoslund, LGTM - thanks

dseefeld and others added 14 commits May 4, 2021 10:42

Conditionally include PrecompileScript

9666583

Add reference and update to use PVP

963ca39

Remove test projects

2e39d7b

Exclude System.Text.Json when calling PublishTestCli

5a67f9b

Patch out Windows Desktop targets

e2b1346

PlatformAbstractions is not produced in source-build

8b0288e

Use source-built MSBuild for F#.

d168358

Use source-built Nuget

6436f10

Remove unused Microsoft.Win32.Registry/4.6.0 ref

9dba9dc

This project restores a prebuilt Microsoft.Win32.Registry reference, but it doesn't end up in the SDK: the DLL from the net5.0 shared framework is already present. Remove this unnecessary PackageReference to remove the prebuilt.

Use PVP version of System.Text.Json

46912e2

vstest: build for .NET 6

90dc79f

Lift NETStandard.Library to SBRP version

100f212

Ensure WebSdk projects finish before PublishNetSdks

2ea3184

Remove dotnet-hello test projects

a62f449

sfoslund requested review from MichaelSimons, dsplaisted and wli3 May 4, 2021 17:48

MichaelSimons reviewed May 4, 2021

View reviewed changes

Comment thread .vsts-ci.yml Outdated

sfoslund force-pushed the SourceBuildPatches branch from 1563809 to 25533b7 Compare May 4, 2021 20:58

sfoslund added 2 commits May 4, 2021 14:01

Adding source build to CI

1db2fd8

Fix sourcebuild patches

80fdaf4

sfoslund force-pushed the SourceBuildPatches branch from 25533b7 to 80fdaf4 Compare May 4, 2021 21:02

wli3 reviewed May 4, 2021

View reviewed changes

Comment thread source-build.slnf

wli3 reviewed May 4, 2021

View reviewed changes

Fix source build test issues

45b7b2b

Adding dotnet-hello assets back

cba4b39