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

Onboarding to ArPow (arcade-powered source-build) #6387

Merged
merged 8 commits into from
Jun 1, 2021

Conversation

MichaelSimons
Copy link
Member

@MichaelSimons MichaelSimons commented Apr 29, 2021

This PR does the following:

  1. Adds the local build infrastructure that lets ArPow (arcade-powered source-build) run in this repo. See https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/onboarding/local-onboarding.md for more details about how it works.

    To try it out locally, run this on Linux: ./build.sh -c Release --restore --build --pack /p:ArcadeBuildFromSource=true -bl

  2. Implements source-build CI.

    To make sure ArPow (arcade-powered source-build) keeps working in this repo, we need to add it to PR validation. We also need it to run in the official build to publish source-built artifacts that can be tested downstream.

    See https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/onboarding/ci-onboarding.md for ArPow CI onboarding info.

  3. Incorporates the existing source-build patches into the repo.

    Some background on source-build patches, for anyone who isn't familiar with previous pushes for patch incorporation:

    A patch is essentially just a commit that has been extracted from Git into a .patch file that can be applied on demand. The effort to build .NET from source involves creating patches because repos make changes that are incompatible with source-build and need to be fixed up after the original released source code has already been finalized. When the original repo gets PRs over time for servicing, the PR changes sometimes conflict with the source-build patches, just like a merge conflict. The patch files need to be fixed up when this happens, which is a significant maintenance problem for the source-build team.

    Several times, the source-build team has pushed for "patch incorporation". This means to merge the commit represented in the .patch file into the original repo's official branch. Doing so prevents patch merge conflicts, because there's no longer a patch to merge against. However, patches inevitably pile up again when getting subsequent servicing releases to work in source-build.

    ArPow lets us end this maintenance-heavy process. By running source-build inside CI, patch merge conflicts will immediately block PR validation, so fixup can be handled in place, not solely by the source-build team. Running source-build in CI also means creating new patches won't be necessary except in exceptional circumstances.

See https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/implementation-plan.md for more details on the ArPow implementation plan.

Fixes: dotnet/source-build#2068.

@MichaelSimons MichaelSimons changed the title ArPow stage 1: local source-build infrastructure Onboarding to ArPow (arcade-powered source-build) May 24, 2021
@MichaelSimons MichaelSimons marked this pull request as ready for review May 24, 2021 22:13
Copy link
Member

@benvillalobos benvillalobos left a comment

Choose a reason for hiding this comment

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

Without digging too much into it, running your repro command in WSL failed for me:

bevillal@DESKTOP-OB2IDE6:/mnt/c/src/linux/msbuild$ ./build.sh -c Release --restore --build --pack /p:ArcadeBuildFromSource=true -bl
Downloading 'https://dot.net/v1/dotnet-install.sh'
dotnet-install: Note that the intended use of this script is for Continuous Integration (CI) scenarios, where:
dotnet-install: - The SDK needs to be installed without user interaction and without admin rights.
dotnet-install: - The SDK installation doesn't need to persist across multiple CI runs.
dotnet-install: To set up a development environment or to run apps, use installers rather than this script. Visit https://dotnet.microsoft.com/download to get the installer.

dotnet-install: Downloading primary link https://dotnetcli.azureedge.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-linux-x64.tar.gz
dotnet-install: Extracting zip from https://dotnetcli.azureedge.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-linux-x64.tar.gz
dotnet-install: Adding to current process PATH: `/mnt/c/src/linux/msbuild/.dotnet`. Note: This change will be visible only when sourcing script.
dotnet-install: Note that the script does not resolve dependencies during installation.
dotnet-install: To check the list of dependencies, go to https://docs.microsoft.com/dotnet/core/install, select your operating system and check the "Dependencies" section.
dotnet-install: Installation finished successfully.
Some command line switches were read from the auto-response file "MSBuild.rsp". To disable this file, use the "-noAutoResponse" switch.
/mnt/c/src/linux/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/MSBuild.dll /nologo -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,/mnt/c/src/linux/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,/mnt/c/src/linux/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll -maxcpucount /m -verbosity:m /v:minimal /bl:/mnt/c/src/linux/msbuild/artifacts/log/Release/ToolsetRestore.binlog /clp:Summary /clp:ErrorsOnly;NoSummary /nr:true /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /p:__ToolsetLocationOutputFile=/mnt/c/src/linux/msbuild/artifacts/toolset/6.0.0-beta.21227.1.txt /t:__WriteToolsetLocation /warnaserror /mnt/c/src/linux/msbuild/artifacts/toolset/restore.proj
/mnt/c/src/linux/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/MSBuild.dll /nologo -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,/mnt/c/src/linux/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,/mnt/c/src/linux/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll -maxcpucount /m -verbosity:m /v:minimal /bl:/mnt/c/src/linux/msbuild/artifacts/log/Release/Build.binlog /clp:Summary /nr:true /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /p:Configuration=Release /p:RepoRoot=/mnt/c/src/linux/msbuild /p:Restore=true /p:Build=true /p:Rebuild=false /p:Test=false /p:Pack=true /p:IntegrationTest=false /p:PerformanceTest=false /p:Sign=false /p:Publish=false /p:ArcadeBuildFromSource=true /warnaserror /home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/Build.proj
  Cloning repository at: /mnt/c/src/linux/msbuild/ -> /mnt/c/src/linux/msbuild/artifacts/source-build/self/src/ ...
/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/SourceBuildArcadeBuild.targets(123,5): error MSB3073: The command "/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/git-clone-to-dir.sh  --source "/mnt/c/src/linux/msbuild/" --dest "/mnt/c/src/linux/msbuild/artifacts/source-build/self/src/" --copy-wip --clean" exited with code 1. [/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/Build.proj]

Build FAILED.

/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/SourceBuildArcadeBuild.targets(123,5): error MSB3073: The command "/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/git-clone-to-dir.sh  --source "/mnt/c/src/linux/msbuild/" --dest "/mnt/c/src/linux/msbuild/artifacts/source-build/self/src/" --copy-wip --clean" exited with code 1. [/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/Build.proj]
    0 Warning(s)
    1 Error(s)

Say something needs to be updated for ArPow in the future, are we expected to manage those updates or will there be some darc subscription that updates version numbers?

@@ -4,6 +4,7 @@
<Dependency Name="Microsoft.DotNet.Arcade.Sdk" Version="6.0.0-beta.21227.1">
<Uri>https://github.com/dotnet/arcade</Uri>
<Sha>cca78ffe3eefdc217e43c2421f2f23355f16da2d</Sha>
<SourceBuild RepoName="arcade" ManagedOnly="true"/>
Copy link
Member

Choose a reason for hiding this comment

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

Do we not need a SourceBuild attribute with ManagedOnly set to true for NuGet.Build.Tasks, Microsoft.Extensions.DependencyModel, or Microsoft.Net.Compilers.Toolset? are they not used for ArPow?

Copy link
Member Author

Choose a reason for hiding this comment

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

We will be adorning more dependencies with the SourceBuild attribute but not all upstream dependencies have their SourceBuild intermediate NuGet packages created yet or msbuild is not on a version that is source built yet. Once all repos are on ArPow we will make the appropriate updates here to eliminate prebuilts. We will also be turning on prebuilt detection in the source build CI leg to prevent regressions.

Copy link
Member

Choose a reason for hiding this comment

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

Note that of the list, we should only need the Roslyn one. The others are dependencies used to run tests/build a test environment that shouldn't be relevant in sourcebuild.

<GitHubRepositoryName>msbuild</GitHubRepositoryName>
</PropertyGroup>

<Target Name="ConfigureInnerBuildArgs" BeforeTargets="GetSourceBuildCommandConfiguration">
Copy link
Member

Choose a reason for hiding this comment

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

A bit more of a design question, should the GetSourceBuildCommandConfiguration target instead have a DependsOnTargets=ConfigureInnerBuildArgs? Or is that step not required of all repos?

I assume it's done this way to prevent repos that don't have the target (that maybe don't care about sourcebuild but use arcade) to outright fail

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, ConfigureInnerBuildArgs is not required.


<Target Name="ConfigureInnerBuildArgs" BeforeTargets="GetSourceBuildCommandConfiguration">
<PropertyGroup>
<InnerBuildArgs>$(InnerBuildArgs) /p:Projects="$(InnerSourceBuildRepoRoot)\MSBuild.SourceBuild.slnf"</InnerBuildArgs>
Copy link
Member

Choose a reason for hiding this comment

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

How does this file get generated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you clarify which file you are referring to? The solution filter is checked in - https://github.com/dotnet/msbuild/blob/main/MSBuild.SourceBuild.slnf

@MichaelSimons
Copy link
Member Author

Say something needs to be updated for ArPow in the future, are we expected to manage those updates or will there be some darc subscription that updates version numbers?

Any version updates will happen via darc. There may infrastructure changes that the source-build team will make or ask the source-build champs to help out with depending on the nature of the changes.

@MichaelSimons
Copy link
Member Author

Without digging too much into it, running your repro command in WSL failed for me:

Thanks for pointing this out. I will take a look at this. I have not tried this command in WSL before.

@rainersigwald
Copy link
Member

Without digging too much into it, running your repro command in WSL failed for me:

Thanks for pointing this out. I will take a look at this. I have not tried this command in WSL before.

One possible complicating factor here: looks like @benvillalobos is using a Windows checkout but building in Linux (path is /mnt/c/src/linux/msbuild/). That often causes problems due to filesystem case sensitivity or git doing CRLF stuff. @benvillalobos, can you try the same somewhere that's in the Linux filesystem?

@benvillalobos
Copy link
Member

@rainersigwald Look like I get the same issue from /home/:

bevillal@DESKTOP-OB2IDE6:~/src/msbuild$ ./build.sh -c Release --restore --build --pack /p:ArcadeBuildFromSource=true -blDownloading 'https://dot.net/v1/dotnet-install.sh'
dotnet-install: Note that the intended use of this script is for Continuous Integration (CI) scenarios, where:
dotnet-install: - The SDK needs to be installed without user interaction and without admin rights.
dotnet-install: - The SDK installation doesn't need to persist across multiple CI runs.
dotnet-install: To set up a development environment or to run apps, use installers rather than this script. Visit https://dotnet.microsoft.com/download to get the installer.

dotnet-install: Downloading primary link https://dotnetcli.azureedge.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-linux-x64.tar.gz
dotnet-install: Extracting zip from https://dotnetcli.azureedge.net/dotnet/Sdk/6.0.100-preview.2.21155.3/dotnet-sdk-6.0.100-preview.2.21155.3-linux-x64.tar.gz
dotnet-install: Adding to current process PATH: `/home/bevillal/src/msbuild/.dotnet`. Note: This change will be visible only when sourcing script.
dotnet-install: Note that the script does not resolve dependencies during installation.
dotnet-install: To check the list of dependencies, go to https://docs.microsoft.com/dotnet/core/install, select your operating system and check the "Dependencies" section.
dotnet-install: Installation finished successfully.
Some command line switches were read from the auto-response file "MSBuild.rsp". To disable this file, use the "-noAutoResponse" switch.
/home/bevillal/src/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/MSBuild.dll /nologo -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,/home/bevillal/src/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,/home/bevillal/src/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll -maxcpucount /m -verbosity:m /v:minimal /bl:/home/bevillal/src/msbuild/artifacts/log/Release/ToolsetRestore.binlog /clp:Summary /clp:ErrorsOnly;NoSummary /nr:true /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /p:__ToolsetLocationOutputFile=/home/bevillal/src/msbuild/artifacts/toolset/6.0.0-beta.21227.1.txt /t:__WriteToolsetLocation /warnaserror /home/bevillal/src/msbuild/artifacts/toolset/restore.proj
/home/bevillal/src/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/MSBuild.dll /nologo -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,/home/bevillal/src/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,/home/bevillal/src/msbuild/.dotnet/sdk/6.0.100-preview.2.21155.3/dotnet.dll -maxcpucount /m -verbosity:m /v:minimal /bl:/home/bevillal/src/msbuild/artifacts/log/Release/Build.binlog /clp:Summary /nr:true /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /p:Configuration=Release /p:RepoRoot=/home/bevillal/src/msbuild /p:Restore=true /p:Build=true /p:Rebuild=false /p:Test=false /p:Pack=true /p:IntegrationTest=false /p:PerformanceTest=false /p:Sign=false /p:Publish=false /p:ArcadeBuildFromSource=true /warnaserror /home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/Build.proj
  Cloning repository at: /home/bevillal/src/msbuild/ -> /home/bevillal/src/msbuild/artifacts/source-build/self/src/ ...
/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/SourceBuildArcadeBuild.targets(123,5): error MSB3073: The command "/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/git-clone-to-dir.sh  --source "/home/bevillal/src/msbuild/" --dest "/home/bevillal/src/msbuild/artifacts/source-build/self/src/" --copy-wip --clean" exited with code 1. [/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/Build.proj]

Build FAILED.

/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/SourceBuildArcadeBuild.targets(123,5): error MSB3073: The command "/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/SourceBuild/git-clone-to-dir.sh  --source "/home/bevillal/src/msbuild/" --dest "/home/bevillal/src/msbuild/artifacts/source-build/self/src/" --copy-wip --clean" exited with code 1. [/home/bevillal/.nuget/packages/microsoft.dotnet.arcade.sdk/6.0.0-beta.21227.1/tools/Build.proj]
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:01.19
Build failed with exit code 1. Check errors above.

@MichaelSimons
Copy link
Member Author

I was able to build without issue in my WSL environment (Debian Buster). I built in a fresh clone created in /home. @benvillalobos, what distro are you using?

.vsts-dotnet.yml Show resolved Hide resolved
eng/Packages.props Show resolved Hide resolved
@benvillalobos
Copy link
Member

@MichaelSimons Ubuntu 20.04.1 LTS (GNU/Linux 4.4.0-19041-Microsoft x86_64) from a clean clone / checkout of this branch.

@MichaelSimons
Copy link
Member Author

@benvillalobos, I was able to reproduce it with a WSL Ubuntu environment. I opened dotnet/source-build#2180 to track this issue. Do you see this as a blocker for merging this PR?

@@ -4,6 +4,7 @@
<Dependency Name="Microsoft.DotNet.Arcade.Sdk" Version="6.0.0-beta.21227.1">
<Uri>https://github.com/dotnet/arcade</Uri>
<Sha>cca78ffe3eefdc217e43c2421f2f23355f16da2d</Sha>
<SourceBuild RepoName="arcade" ManagedOnly="true"/>
Copy link
Member

Choose a reason for hiding this comment

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

Note that of the list, we should only need the Roslyn one. The others are dependencies used to run tests/build a test environment that shouldn't be relevant in sourcebuild.

.vsts-dotnet-ci.yml Show resolved Hide resolved
@benvillalobos
Copy link
Member

@MichaelSimons So long as it runs on whatever machine is generating these source builds and there's an issue tracking it (thanks for that), I don't consider it blocking

@rainersigwald
Copy link
Member

Oh wait, have we test run the official build change? That's my only blocking issue.

@MichaelSimons
Copy link
Member Author

Oh wait, have we test run the official build change? That's my only blocking issue.

I have not. Can you tell me if there are any special things to know when queuing?

@benvillalobos
Copy link
Member

benvillalobos commented May 25, 2021

Pushed up to exp/michaelsimons/ArPow. Should show up here soon: https://dev.azure.com/devdiv/DevDiv/_build?definitionId=9434

Side note: +1 for a bot that looks at a particular label and creates the exp/ branch for us. #6294

@benvillalobos
Copy link
Member

@MichaelSimons
Copy link
Member Author

The test run of the official build failed. It will require a new Arcade version w/this fix - dotnet/arcade#7453

@MichaelSimons
Copy link
Member Author

@rainersigwald - thanks for the catch on running a test build!

@MichaelSimons
Copy link
Member Author

To fix the issue with the official builds, we need Arcade 6.0.0-beta.21278.2 or newer. See #6478.

@rainersigwald
Copy link
Member

@Forgind can you please prioritize that?

@Forgind
Copy link
Member

Forgind commented Jun 1, 2021

I made the change to #6478, and it passed 2 legs so far. It looks like RPS failed over the weekend, so I'll have to figure that out before we can actually merge it.

@Forgind
Copy link
Member

Forgind commented Jun 1, 2021

I merged #6478 and fixed the merge conflict here. Should be good to merge if it passes tests, right? (I haven't actually been following this PR.)

@rainersigwald
Copy link
Member

@Forgind let's also spin an official build by pushing the merge to exp/michaelsimons/ArPow too please

@MichaelSimons
Copy link
Member Author

@Forgind let's also spin an official build by pushing the merge to exp/michaelsimons/ArPow too please

I pushed it. The new test build is https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=4822514.

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

The official build failure on the record parameter name thing is not blocking. I'm happy.

@MichaelSimons
Copy link
Member Author

PR validation is green and according to @rainersigwald the failure in the test official build is not blocking. Can this be merged now?

@Forgind Forgind merged commit 4d6df82 into dotnet:main Jun 1, 2021
@MichaelSimons MichaelSimons deleted the ArPow-Stage1 branch June 1, 2021 21:02
Forgind pushed a commit that referenced this pull request Jun 2, 2021
Fixes dotnet/source-build#2068

Context
This was something I discovered while trying to consume the Source-Build intermediate package produced by the first ArPow changes made in #6387

Changes Made
Set the SourceBuildManagedOnly property correctly.

How Tested
Verified the resulting Microsoft.SourceBuild.Intermediate.msbuild package is not named rid specific.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ArPow] Onboard msbuild to arcade-powered source-build
5 participants