Bootstrap arcade #31436

weshaggard · 2018-07-27T20:46:50Z

This PR moves away from the init-tools bootstrapping model and to the model used by arcade. We still heavily depend on BuildTools but we are bootstrapping it via the arcade mechanism instead and we will slowly transition more things away from BuildTools and instead using standalone arcade packages as we progress through more of our project https://github.com/dotnet/corefx/projects/3.

This PR is far from ready but I wanted to get the initial version pushed out in PR for folks to review before I go on holiday. I will pick it up again after I'm back (~3 weeks).

@eerhardt @wtgodbe @tmat please have a look.

FYI @markwilkie @mmitche

weshaggard · 2018-07-27T20:54:08Z

eng/Versions.props

+    <UsingToolNetFrameworkReferenceAssemblies>true</UsingToolNetFrameworkReferenceAssemblies>
+
+    <!-- Libs -->
+    <CredentialManagementVersion>1.0.2</CredentialManagementVersion>


These were copied over from arcade but these versions should be cleaned up and reconciled with what we have in dependencies.props.

weshaggard · 2018-07-27T20:57:07Z

eng/common/build.ps1


 function Build([string] $buildDriver, [string]$buildArgs) {
  & $buildDriver $buildArgs $ToolsetBuildProj `
-    /m /nologo /clp:Summary /warnaserror `


Need to file an issue to fix all the build warnings to re-enable /warnaserror

weshaggard · 2018-07-27T20:57:24Z

eng/common/build.sh

      solution=$2
      shift 2
      ;;
+    --projects)


Need to fix this in the arcade version of this build.sh script.

weshaggard · 2018-07-27T21:01:11Z

Things yet to be done:

Need to update all the official build definitions
Update the developer documentation
- Decide if we want an intermediate command line parser to help with common scenarios like we had with run.exe
Update the maestro dependency updates for the various tools versions
Decide if we still need a separate sync step or if we should do it inline with the build
Need to file issues for warningsaserrors
Need to port some changes from eng/common/build.sh to arcade.

buildpipeline/linux.arm64.groovy

eerhardt · 2018-07-30T15:50:34Z

build.cmd

-echo.
-goto :Build
+@echo off
+powershell -ExecutionPolicy ByPass -NoProfile %~dp0eng\common\Build.ps1 -projects %~dp0build.proj -restore -build %*


(nit) casing on the file name Build.ps1 vs. build.ps1

eerhardt · 2018-07-30T15:52:07Z

build.sh

-"$__scriptpath/build-managed.sh" -BuildPackages=true "$@"
-exit $?
+scriptroot="$( cd -P "$( dirname "$source" )" && pwd )"
+"$scriptroot/eng/common/build.sh" --build --restore --projects %~dp0build.proj $@


(nit) I think it would be great if we kept the windows and non-Windows scripts as close as possible. Here the arguments are defined in a different order in build.sh than they are in build.cmd. This makes it hard to tell that they are passing the same arguments in each.

/build.sh" --build --restore --projects %~dp0build.proj $@
vs
Build.ps1 -projects %~dp0build.proj -restore -build %*

eerhardt · 2018-07-30T15:54:14Z

eng/build-test.sh

+done
+
+scriptroot="$( cd -P "$( dirname "$source" )" && pwd )"
+"$scriptroot/common/build.sh" --build --projects %~dp0../src/tests.builds $@


(nit) Similar comment here - the order of parameters should be the same as the windows script.

eerhardt · 2018-07-30T15:55:22Z

eng/Versions.props

+</PropertyGroup>
+
+  <PropertyGroup>
+    <RestoreSources>


It feels really weird to have this property in a Versions.props file.

eerhardt · 2018-07-30T15:56:41Z

eng/build-test.cmd

@@ -0,0 +1,3 @@
+@echo off


Who calls these build-test scripts?

The CI scripts currently do. I originally removed them but I wasn't able to easily change the project without a wrapper script so I decided to keep them, at least for now.

NuGet.config

run.cmd

eerhardt · 2018-07-30T16:00:13Z

build.proj

  <Import Project="$(ToolsDir)VersionTools.targets" Condition="Exists('$(ToolsDir)VersionTools.targets')" />

  <ItemGroup>
+    <Project Include="src\Native\build-native.proj" Condition="'$(BuildNative)'=='true'"/>


Just curious why this happens out of the "normal" order? Wouldn't it make more sense if src\dirs.proj would build this project, since this project is under the src directory?

We can consider moving it to src\dirs.proj but it isn't a normal project and is controlled by a property from the root build so I thought it made more sense here.

src/Native/build-native.proj

Directory.Build.props

eerhardt · 2018-07-30T16:07:35Z

global.json

@@ -0,0 +1,8 @@
+{
+    "sdk": {
+        "version": "2.1.401-preview-009081"


We should be aware that this is going to affect anyone who uses VS in corefx....

yes these changes will definitely affect folks dev flow.

eerhardt · 2018-07-30T16:09:08Z

Looking good. I really like how many root level files we can now just get rid of.

I didn't really look at the eng\common stuff, since I assume that is copied in all the other repos and just copied here, so it has been reviewed extensively.

karelz · 2018-09-05T20:18:47Z

What is status of this PR? Looks like 2 weeks without any update ...

weshaggard · 2018-09-05T21:02:08Z

What is status of this PR?

I'm hoping to get back to working on this PR soon. I'm mostly using it to help with testing the engineering changes coming with arcade.

weshaggard · 2018-10-10T21:34:52Z

test Linux arm64 Release Build

weshaggard · 2018-10-10T21:35:06Z

test Windows x64 Debug Build

danmoseley · 2018-10-10T23:25:07Z

Directory.Build.props

  </PropertyGroup>

  <!-- Default properties for CI builds -->
  <!-- TODO: Remove obsolete condition when moved to arcade. -->


Can you remove  now?

Yes I can remove this comment :)

Directory.Build.props

build.cmd

Directory.Build.props

weshaggard · 2018-10-11T16:02:07Z

test Tizen armel Debug Build

Documentation/project-docs/developer-guide.md

build.proj

clean.cmd

weshaggard · 2018-10-11T21:07:47Z

test Windows x64 Debug Build

at System.Diagnostics.Tests.ProcessTests.GetProcessesByName_ProcessName_ReturnsExpected() in D:\j\workspace\windows-TGrou---74aa877a\src\System.Diagnostics.Process\tests\ProcessTests.cs:line 986

https://mc.dot.net/#/user/weshaggard/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Fmaster~2F/test~2Ffunctional~2Fcli~2F/384f977bc934102ff1399386ba54a0b00ce0b756/workItem/System.Diagnostics.Process.Tests/analysis/xunit/System.Diagnostics.Tests.ProcessTests~2FGetProcessesByName_ProcessName_ReturnsExpected

This one process test seems to keep failing on Nano. @danmosemsft @stephentoub have you guys seen it hitting other PR's?

Generating the resources as part of he build requires he resgen tool which only works for builds that are on windows with the full VS/.NET SDK installed. That isn't always the case so instead we will generate the manually and commit them until the CLI issue is fixed. dotnet/msbuild#2221

weshaggard · 2018-10-12T01:00:02Z

I believe this PR is now ready once CI is green and my current official build test passes.

Issue filed https://github.com/dotnet/corefx/issues/32780

Summary of changes: - Update to latest version of .NET SDK - Delete all the root level wrapper scripts - Switch bootstrapping model to pull in arcade scripts - Update documentation - Update CI scripts - Update official build definitions BuildToolsVersion.txt and DotnetCLIVersion.txt are no longer used and instead the cli version is in global.json and the BuildTools version is in dependencies.props for the time being until we eliminate it completely and move everything to arcade. Root build scripts now call into eng/common build scripts. With wrapper scripts under eng to allow for custom argument alias given we have removed run.exe. Restructed Build.proj in the root to separte the different phases into targets so we can call them individually through the arcade scripts or our CI/official build definitions. Wrapped native build into a build-native.proj file so we can trigger it directly from msbuild in our normal build flow. We can still directly call the native build-native.cmd/sh for bootstraping new platforms but that isn't the default any longer for our builds. CI and official builds no longer call sync and then build we just call build directly and let it do all the phases. Fixed issues with BuildTools were we need to define RootIntermediateOutputPath explicitly now given in BuildTools it looks for init-tools.msbuild which no longer exists. Update the PackagesDir to point to NugetPackageRoot now so we can configure the packages to use the machine wide cache or local repo cache. Currently our docker build definitions don't presist the user wide nuget cache so we need to pass --ci to get the packages and tmp folder localized to the repo which is presisted across commands. Our manual shims were having issues so removed the default references and reduced the amount of seeds passed to help eliminate potential reference resolution issues.

weshaggard · 2018-10-12T02:38:18Z

test Linux arm64 Release Build

weshaggard · 2018-10-12T03:24:40Z

test Linux arm64 Release Build

weshaggard · 2018-10-12T16:16:08Z

test Linux arm64 Release Build

michellemcdaniel · 2018-10-12T17:02:33Z

As part of this work, do you have someone looking at updating the two groovy files (netci.groovy, perf.groovy) to use the new parameter passing mechanism? All of the jobs in ci.dot.net and ci2.dot.net are now failing.

weshaggard · 2018-10-12T17:54:39Z

As part of this work, do you have someone looking at updating the two groovy files (netci.groovy, perf.groovy) to use the new parameter passing mechanism? All of the jobs in ci.dot.net and ci2.dot.net are now failing.

The normal netci.groovy legs that run by default should still be working but I didn't look at perf.groovy I will take a look at those.

michellemcdaniel · 2018-10-12T20:14:59Z

Hm, I ask, because the windows legs seem to be failing in ci.dot.net:

https://ci.dot.net/job/dotnet_corefx/job/master/job/windows_nt_release/7670/

weshaggard · 2018-10-12T23:20:44Z

I've fixing them in PR #32789 although they aren't easy to test so I will verify some manually but update all of them then merge the CI update. If there are other issues we find we can address them.

Bootstrap arcade Commit migrated from dotnet/corefx@2739b08

weshaggard added area-Infrastructure-libraries * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) labels Jul 27, 2018

weshaggard self-assigned this Jul 27, 2018

weshaggard commented Jul 27, 2018

View reviewed changes

eng/common/build.sh

solution=$2

shift 2

;;

--projects)

Copy link

Member Author

weshaggard Jul 27, 2018

Choose a reason for hiding this comment

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

Need to fix this in the arcade version of this build.sh script.

weshaggard commented Jul 27, 2018

View reviewed changes

buildpipeline/linux.arm64.groovy Show resolved Hide resolved

weshaggard force-pushed the BootstrapArcade branch from 3d76ab4 to 3cdb6ce Compare July 27, 2018 21:21