Remove buildtools by sbomer · Pull Request #26108 · dotnet/coreclr

sbomer · 2019-08-09T23:30:02Z

I think this is finally ready to go. @dotnet/coreclr-infra this removes the dependency on buildtools. Please let me know if you think there's any additional testing I need to do.

Makes a number of common test projects SDK-style, and replaces the buildtools prerelease restore target with the restore logic built in to the SDK. For some projects, we currently restore 5.0 assets that are used for netcoreapp3.0 - this required a workaround to switch the TFM in a few cases.
Replaces the corefx testhost deps file creation logic with that from the SDK's publish logic.
Uses a new IL sdk (as the old one used to make many redundant file copies), and uses an .ilproj to restore ilasm/ildasm up-front.
Uses the UpdateVersions logic from arcade instead of buildtools.

This opens up the possibility of further simplifying some of our msbuild imports, but I don't want to do so in this PR as it's already a sizeable change.

hoyosjs · 2019-08-12T16:46:56Z

@@ -17,7 +17,6 @@
  <!-- Source of truth for dependency tooling: the commit hash of the dotnet/versions master branch as of the last auto-upgrade. -->
  <PropertyGroup>
    <CoreClrCurrentRef>5d3c9a7c54c1c59b764de0e2dfb6bbb4ce29476c</CoreClrCurrentRef>


This was mostly used to update ILAsmVersion.txt. I see you are removing that file, should we just remove the MicrosoftNETCoreRuntimeCoreCLRPackageVersion update and make a DarcMaestro subscription for the CoreCLR packages? Also, if that happens, should we delete this file and move anything interesting left to Directory.Build.props? Are there scenarios that depend on this as it currently is?

Eventual goal is to get rid of this file - I'll look into it and may work on that in this change if it makes sense.

Instead we use the built-in SDK targets to restore packages. The new logic will not copy pdbs from package dependencies to CORE_ROOT.

This prevents issues with the old SDK trying to copy ilasm to the Tools directory for each build of an ilproj.

Use netcoreapp3.0 to ensure that package dependencies show up in the build output (bin/TargetingPack). This fixes a missing reference for CscBench, which depends on Microsoft.CodeAnalysis.CSharp. Also clean up some unnecessary properties.

This library needs to be restored for netcoreapp5.0, and built for netcoreapp3.0 to match the tests.

There were problems when trying to target 5.0 using the current SDK, which only supports targeting 3.0 out of the box. This required using a more recent SDK, as p6 restored framework references differently. p7 is able to restore a framework reference with the MicrosoftNETCoreApp version that we desire.

sbomer · 2019-09-13T15:53:02Z

/azp run coreclr-outerloop

azure-pipelines · 2019-09-13T15:53:16Z

Azure Pipelines successfully started running 1 pipeline(s).

sbomer · 2019-09-16T18:10:35Z

/azp run coreclr-outerloop

azure-pipelines · 2019-09-16T18:10:50Z

Azure Pipelines successfully started running 1 pipeline(s).

jashook

lgtm thank you for this change!

hoyosjs · 2019-09-17T00:27:29Z

+
+  <Import Project="$([MSBuild]::GetPathOfFileAbove('Directory.Build.props', '$(MSBuildThisFileDirectory)../'))" />
+
+  <Import Project="$(NuGetPackageRoot)\microsoft.dotnet.versiontools.tasks\$(MicrosoftDotNetVersionToolsTasksPackageVersion)\build\Microsoft.DotNet.VersionTools.Tasks.targets" />


Technically this is the only thing that we still have from buildtools?

This now comes from arcade: https://github.com/dotnet/arcade/tree/master/src/Microsoft.DotNet.VersionTools :)

echesakov

Looks Great! Thank you!

echesakov · 2019-09-17T20:29:21Z

@@ -59,14 +31,6 @@
    <AltJitArch>$(__AltJitArch)</AltJitArch>


We can git of this in the future.

echesakov · 2019-09-17T20:34:01Z

+        <File>%(Identity)</File>
+      </AllResolvedRuntimeDependencies>
+      <RunTimeDependecyCopyLocalFile Include="@(AllResolvedRuntimeDependencies)"  Exclude="@(RunTimeDependecyExcludeFiles)"/>
+      <RunTimeDependecyCopyLocal Include="@(RunTimeDependecyCopyLocalFile -> '%(File)')"  />


We can fix the typo in the future.

echesakov · 2019-09-17T20:34:11Z

+    </ItemGroup>
+
+    <Copy
+      SourceFiles="@(RunTimeDependecyCopyLocal)"


After dotnet#26108, this was broken because the Tools.proj was never restored on the agent running the finalize-publish job. This fixes the script to do a restore, and adds a missing import.

* Fix update versions logic After #26108, this was broken because the Tools.proj was never restored on the agent running the finalize-publish job. This fixes the script to do a restore, and adds a missing import. * Remove UpdatePublishedVersions.ps1 and add darc dependency

sbomer added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 9, 2019

sbomer force-pushed the removeBuildTools branch from 85bcebd to 13878d7 Compare August 10, 2019 00:13

hoyosjs reviewed Aug 12, 2019

View reviewed changes

sbomer force-pushed the removeBuildTools branch 2 times, most recently from a0d55f6 to 285f5de Compare August 15, 2019 17:49

sbomer force-pushed the removeBuildTools branch 2 times, most recently from 03f4d2c to f069b2d Compare September 5, 2019 05:31

sbomer added 23 commits September 10, 2019 17:45

Start removing buildtools

cc4f82e

Clean up project files and remove dependency on prerelease restore

a9b916b

Instead we use the built-in SDK targets to restore packages. The new logic will not copy pdbs from package dependencies to CORE_ROOT.

Don't generate deps.json in TargetingPack, and remove depproj targets

74abc54

Remove some more references to buildtools

93da987

Remove references to binclashlogger

e0fb42d

Avoid using GenerateDepsJson task for corefx testhost

d816184

Fix order of core_root projects

dd35466

Don't define Build target in dir.targets

c521595

Make CoreCLRTestLibrary SDK-style

d37560b

Fix test targeting pack build

a19959e

Remove duplicate imports in testwrapper

e1ae4cf

Filter external.depproj to prevent conflicts

08a92a3

Avoid using dotnet on path for batch restore

c18c70e

Restore CoreCLRTestLibrary

70a296d

Restore ilasm/ildasm for .ilproj tests

be4af8a

Use recent IL SDK

dbf55b6

This prevents issues with the old SDK trying to copy ilasm to the Tools directory for each build of an ilproj.

Fix targeting pack build

f848771

Use netcoreapp3.0 to ensure that package dependencies show up in the build output (bin/TargetingPack). This fixes a missing reference for CscBench, which depends on Microsoft.CodeAnalysis.CSharp. Also clean up some unnecessary properties.

Remove some unused properties

fbe4ba4

Update performance projects

1655966

Don't error on package downgrade

fdaee58

Rename CoreFX.depproj and clean up

fc10bee

Fix coreclrtestlibrary casing

b1c665a

Don't check for Tools directory in stress dependencies

2464749

sbomer added 8 commits September 10, 2019 17:45

Fix netcoreapp version mismatch for CoreCLRTestLibrary

09bf28c

This library needs to be restored for netcoreapp5.0, and built for netcoreapp3.0 to match the tests.

Fix NuGetTargetMoniker for performance projects

4b26738

Fix restore TFM for test_dependencies and test_runtime

06b33c6

Switch TFM for CoreFX testhost and prevent error in external.csproj

3f32915

Clean up comments

10489bb

Fix NuGetTargetMoniker for test wrappers

b3039eb

Restore external.csproj up-front

3d9aae0

sbomer force-pushed the removeBuildTools branch from 9f55268 to a6d0e09 Compare September 11, 2019 00:46

sbomer added 2 commits September 12, 2019 16:06

Use UpdatePublishedVersions from arcade

adb5031

Cleanup and add better comments

5426d55

sbomer requested a review from jashook September 13, 2019 15:51

sbomer added 2 commits September 13, 2019 13:25

Fix RID for rhel

e756cb1

Ensure featuredefines are imported for test library

33b637c

sbomer changed the title ~~[WIP] Remove buildtools~~ Remove buildtools Sep 16, 2019

sbomer removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 16, 2019

jashook approved these changes Sep 16, 2019

View reviewed changes

hoyosjs reviewed Sep 17, 2019

View reviewed changes

echesakov approved these changes Sep 17, 2019

View reviewed changes

sbomer merged commit 0dbe04c into dotnet:master Sep 17, 2019

sbomer mentioned this pull request Sep 18, 2019

Fix update versions logic #26768

Merged

gbalykov mentioned this pull request Jan 31, 2020

Tizen build fail dotnet/runtime#13440

Closed

sdmaclea mentioned this pull request May 7, 2020

Libunwind1.5rc2 dotnet/runtime#36027

Merged

Navigation Menu

Repository navigation

Remove buildtools - #26108

Remove buildtools#26108
sbomer merged 44 commits into
dotnet:masterfrom
sbomer:removeBuildTools

sbomer commented Aug 9, 2019 •

edited

Loading

Uh oh!

hoyosjs Aug 12, 2019

Uh oh!

sbomer Aug 12, 2019

Uh oh!

sbomer commented Sep 13, 2019

Uh oh!

azure-pipelines Bot commented Sep 13, 2019

Uh oh!

sbomer commented Sep 16, 2019

Uh oh!

azure-pipelines Bot commented Sep 16, 2019

Uh oh!

jashook left a comment

Uh oh!

hoyosjs Sep 17, 2019

Uh oh!

sbomer Sep 17, 2019

Uh oh!

echesakov left a comment

Uh oh!

echesakov Sep 17, 2019

Uh oh!

echesakov Sep 17, 2019

Uh oh!

echesakov Sep 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		<Import Project="$([MSBuild]::GetPathOfFileAbove('Directory.Build.props', '$(MSBuildThisFileDirectory)../'))" />

		<Import Project="$(NuGetPackageRoot)\microsoft.dotnet.versiontools.tasks\$(MicrosoftDotNetVersionToolsTasksPackageVersion)\build\Microsoft.DotNet.VersionTools.Tasks.targets" />

Conversation

sbomer commented Aug 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hoyosjs Aug 12, 2019

Choose a reason for hiding this comment

Uh oh!

sbomer Aug 12, 2019

Choose a reason for hiding this comment

Uh oh!

sbomer commented Sep 13, 2019

Uh oh!

azure-pipelines Bot commented Sep 13, 2019

Uh oh!

sbomer commented Sep 16, 2019

Uh oh!

azure-pipelines Bot commented Sep 16, 2019

Uh oh!

jashook left a comment

Choose a reason for hiding this comment

Uh oh!

hoyosjs Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

sbomer Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

echesakov left a comment

Choose a reason for hiding this comment

Uh oh!

echesakov Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

echesakov Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

echesakov Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sbomer commented Aug 9, 2019 •

edited

Loading