Add DeltaBuild into Arcade

[tekian]

To double check:

• The right tests are in and and the right validation has happened. Guidance: https://github.com/dotnet/arcade/tree/main/Documentation/Validation

[markwilkie]

If @riarenas , @chcosta , @AlitzelMendez could take a look that'd be super helpful. Jan and I have been talking, and basically he'd like to add this builddelta tool to Arcade. It's not intended to be in the primary tool chain path, but available for those who want it.

[riarenas]

I lack the necessary context, so I'll ask:

• What is the business goal of having this tool available through arcade instead of however it's distributed now?
• Who is going to own the tool, including paying attention to issues related to it? Especially after the recent reorg I don't think we should pick up anything new for now, even if we would just be directing any issues to another team.
• Are tests coming in future iterations?

[RussKie]

I plan to start reviewing this early next week.

[tekian]

What is the business goal of having this tool available through arcade instead of however it's distributed now?

We'd like to reuse the tool between multiple .NET repositories. Today, the tool is not distributed in any way. Source of the tool is built as a prerequisite before source of unrelated project is built. It's a new tool.

Who is going to own the tool, including paying attention to issues related to it?

That's going to be myself, @mobratil, @RussKie and more broadly the .NET teams using it.

Are tests coming in future iterations?

Absolutely. The tool works as it is now, but we wouldn't be able to catch any regressions if we decide to add or change the behavior. After we have tool published as a .NET tool, I'm going to add a test suite for it.

[tekian]

@riarenas With @RussKie 's help, we now have all 15 checks green. What should be our next step? 🙂

[riarenas]

I'd like to see an official test build with these changes, especially due to the workarounds for symbol publishing errors you are introducing that will only be exercised for real during official builds and not during PR builds.

@RussKie has done this before, and the steps are basically to follow the validation steps in this document: https://github.com/dotnet/arcade/blob/main/Documentation/CorePackages/Publishing.md#validating-the-changes

I'm not familiar with the vs script you are introducing into the repo. My sense is that it shouldn't be added in the PR as it's not related to this tool.

Ideally we won't check this in without unit tests either, but that's not blocking as you mentioned you planned followup changes.

[ViktorHofer]

cc @rainersigwald @dsplaisted

[RussKie]

👍
Few suggestions to the readme.

[RussKie]

Also, more importantly, what else we need to do to get this merged?

[mmitche]

It's because the shas match between the public and internal repos, so Maestro will preference the public repo URI. In that case the default channel doesn't apply.

I very rarely recommend using default channels for one-off testing like this. Instead, just do the following after the build is complete:

darc add-build-to-channel --id <BAR ID> --channel "General Testing"

and wait for it to complete.

[RussKie]

Available test artifacts:

• https://dev.azure.com/dnceng/public/_artifacts/feed/general-testing/NuGet/Microsoft.DotNet.DeltaBuild/overview/8.0.0-beta.23325.1

[RussKie]

We need a way to reference and invoke the tool.

I'm not exactly clear on how to do the first step - i.e., reference. The tool has to run (if opted in) before the build because it defines the subset of projects that get built.
One way could be via global.json msbuild-sdks node, e.g.:

  "msbuild-sdks": {
    "Microsoft.DotNet.Arcade.Sdk": "8.0.0-beta.23322.2",
    "Microsoft.DotNet.Helix.Sdk": "8.0.0-beta.23322.2",
+   "Microsoft.DotNet.DeltaBuild": "8.0.0-beta.23325.1"
  }

...but it means we need to create DeltaBuildSdk. I don't know how this works though...

The other option is to add an unconditional reference to https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/Tools.proj, e.g.:

  <ItemGroup>
    <PackageReference Include="Microsoft.DotNet.NuGetRepack.Tasks" Version="$(MicrosoftDotnetNuGetRepackTasksVersion)" Condition="'$(UsingToolNuGetRepack)' == 'true'" IsImplicitlyDefined="true" />
+   <PackageReference Include="Microsoft.DotNet.DeltaBuild" Version="8.0.0-beta.23325.1" IsImplicitlyDefined="true">
+     <PrivateAssets>all</PrivateAssets>
+   </PackageReference>
  </ItemGroup>

...and then create DeltaBuild.props and DeltaBuild.targets that contain some targets that run before Build (or Restore?) target. Again this requires further thoughts and iterations.

[RussKie]

By targeting the current TFM only we make the tool only usable by projects targeting .NET 8. Which isn't good. We need to be targeting something like net472.

[tekian]

I had .NET Framework TFM but there was a build warning/error that a .NET Tool can only be .NET Core. Should we add .NET 7? Ideas?

[RussKie]

@markwilkie @mmitche @riarenas is there someone we could we talk to learn how to best plug this tool into the Arcade SDK and the whole build process?

[mmitche]

@markwilkie @mmitche @riarenas is there someone we could we talk to learn how to best plug this tool into the Arcade SDK and the whole build process?

I think the package ref in tools.proj, conditionalized based on whether the feature is enabled (this is important since it shouldn't be enabled by default) is a good way to start.

[RussKie]

I think the package ref in tools.proj, conditionalized based on whether the feature is enabled (this is important since it shouldn't be enabled by default) is a good way to start.

I tried to do this locally by defining UseDeltaBuild=true property in a test project's Directory.Build.props and conditionalising the import in the Arcade's Tools.proj (in my local nuget cache), and it didn't work. The binlog was showing the property is set, however, that was too late for the Tools.proj eval, or it had no knowledge of that...

[mmitche]

Right, thanks. I was kind of wondering whether the docs should have said "General Testing Internal". I think the docs should be updated, though there are three possibilities:

• Option 1: Instead of "Create a branch on the Azure DevOps internal mirror of the repo that includes the pipeline changes." use the public repo. Then "General Testing" would work.
• Option 2: Instead of "General Testing" specify "General Testing Internal".
• Option 3: Remove default channel creation steps and use darc add-build-to-channel --id <BAR ID> --channel "General Testing" instead.

Which version do you prefer? I'm happy to update the docs.

The internal vs. public channel isn't really about the repo being public or internal. It's about whether the artifacts can be public. For instance, we publish wpf-int to public channels, even though the source is internal. It's simply that Maestro has some logic in it that preferences the repo URI to be public, if the commit built is public. So it takes the AzDO URI + sha that was built officially, e.g. dotnet-extensions @ foosha, translates the URI to what the github equivalent would be, i.e. dotnet/extensions @ foosha, then attempts to do an anonymous get. If the URI 404's, then it says that the URI of the repo built was the original URI, otherwise it adds github.

Default channels always preference a build's github uri over the azdo one rather than letting both go through so that we cannot mistakenly publish internal bits to public endpoints. A good example of how this could happen is this:

• A dev has a security fix that they open in an AzDO PR, mistakenly pointing the target branch to release/5.0.
• It merges and officially builds.
• The default channel picks up the release/5.0 branch + AzDO URI and publishes public bits.

Instead in this case, the default channel for public repos always pulls from GitHub, but the commit doesn't show up on github. So the AzDO URI does not match the default channel and no sensitive bits are accidentally published.

TLDR: It's really just about what the purpose of your bits are. You may assign any build to a channel based on whether you want the bits to be public or not. You need to take into account whether commit being built is public or not when you set up a default channel. However, it is simpler to just manually assign in these one off situations, because you may be pushing the commit you build to both github and azdo, or only azdo.

I think we should document Option 3

[ViktorHofer]

Arcade usually only target NetCurrent. Why do you need to support net7.0?

[RussKie]

This is a dotnet tool, which needs to work for all kinds of solutions that may target the whole plethora of TFMs. The Arcade SDK isn't exclusive to .NET 8 projects and, for example, can be used to build .NET Framework projects.
https://github.com/microsoft/slngen tool is another example, it targets the whole specter of TFMs:

[RussKie]

Right now, an attempt to install this tool fails:

C:\>dotnet tool install -g Microsoft.DotNet.DeltaBuild --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/general-testing/nuget/v3/index.json --prerelease

C:\Users\jagutt\AppData\Local\Temp\79bb3973-33dd-43b6-b8e8-78303e953957\restore.csproj : error NU1202: Package Microsoft.DotNet.DeltaBuild 8.0.0-beta.23325.1 is not compatible with net7.0 (.NETCoreApp,Version=v7.0) / any. Package Microsoft.DotNet.DeltaBuild 8.0.0-b
eta.23325.1 supports: net8.0 (.NETCoreApp,Version=v8.0) / any
The tool package could not be restored.

[ViktorHofer]

But why do you need only net7.0 then? net6.0 is currently the minimum supported TFM and is represented as an msbuild property: $(NetMinimum). net7.0 should probably also be replaced with $(NetPrevious).