Use Arcade's publishing logic to publish a given VMR vertical

[jkoritzinsky]

Instead of having our own custom targets and tasks to publish a given VMR vertical, use the tooling in the Arcade SDK to publish a vertical.

Contributes to dotnet/arcade#15317

Depends on a bootstrap Arcade SDK containing dotnet/arcade#15572

[ViktorHofer]

A lot of great stuff in this PR, using standards and removing customization where possible. Love it!

I need to be honest with one part. I really dislike ripping apart VMR's Directory.Build.props file. Did you consider importing the file in Publishing.props and setting SkipArcadeSdkImport=true?

VMR's D.B files are stock / vanilla, without much customization as they need to support projects that don't import the Arcade SDK.

[jkoritzinsky]

A lot of great stuff in this PR, using standards and removing customization where possible. Love it!

I need to be honest with one part. I really dislike ripping apart VMR's Directory.Build.props file. Did you consider importing the file in Publishing.props and setting SkipArcadeSdkImport=true?

VMR's D.B files are stock / vanilla, without much customization as they need to support projects that don't import the Arcade SDK.

I don't like the idea of including Directory.Build.props in a non-standard way. It makes it more confusing to understand how the logic flows.

[ViktorHofer]

I don't like the idea of including Directory.Build.props in a non-standard way

Convince me that tearing apart D.B.props is better than importing it in Publish.proj. I really don't see how the former is better in terms of readability or UX. Publish.proj is a non Microsoft.NET.Sdk project so it never imports any D.B files. Manually importing it sounds like the right thing to me.

[jkoritzinsky]

I don't like the idea of including Directory.Build.props in a non-standard way

Convince me that tearing apart D.B.props is better than importing it in Publish.proj. I really don't see how the former is better in terms of readability or UX. Publish.proj is a non Microsoft.NET.Sdk project so it never imports any D.B files. Manually importing it sounds like the right thing to me.

Here's the problems that I see with importing DBP directly:

• Directory.Build.props would become unable to use any .NET SDK properties in it as Publish.props doesn't include the .NET SDK (main one that comes to mind would be the RID ones)
• As I've said, including DBP directly in the middle of a project is unintuitive, making it more difficult to know what's going on for someone less familiar with the build system
• Publish.proj does include some of the Arcade SDK by design, so the current SkipArcadeSdkImport logic would cause many of the properties to be reset or require them to be kept in sync with the Arcade SDK so different parts of the project don't get different values.
  • In particular, RepoLayout.props is already included.
  • We could split the SkipArcadeSdkImport property into two (so you could opt-out of importing the Arcade SDK and not get the property polyfills)
• Publish.proj has some logic that's triggered on DotNetBuildOrchestrator == true that we'd need to either adjust there and re-bootstrap or manually account for here if we were to just include DBP. Additionally, the following problem would trigger with the current usage of DotNetBuildOrchestrator.
• Unlike all other parts of the build system, DBP would not be included at the "start" of evaluation, so those properties may change existing property values, but not items due to evaluation order.

If you still feel strongly about it, I'm fine undoing it in a follow-up PR. Given that we want to get this in today, I'd rather wait until a follow-up than do it in this PR.

[ViktorHofer]

Thanks for taking the time to respond. My feedback isn't blocking.

If you still feel strongly about it, I'm fine undoing it in a follow-up PR. Given that we want to get this in today, I'd rather wait until a follow-up than do it in this PR.

I feel strongly about finding a solution that doesn't require us to tear our repository infrastructure apart like in this PR. When the Arcade SDK and repo entry points like Build.proj were originally added to arcade, several people advised to not follow this model that creates an entire separate orchestration layer with a different set of imports, inputs and outputs. What we are discussing here is the fallout from the direction that was taken back then.

I see two directions that we could take:

1. Import D.B.props "gracefully" by making sure that the above points that you shared are addressed. I don't see anything blocking from that list. I'm convinced that we can find a good solution here.
2. Allow Publish.proj / Sign.proj to run in the repo context instead of only in the special "Arcade almighty orchestrator" context. Basically, what I mean here is using the NoTargets SDK, import the repo infra by setting the D.B.props/targets paths accordingly and condition the BuildSteps.props import in Publish.proj out. This would solve the split relationship that is causing this annoyance. These projects would then automatically import the Arcade SDK similar as any other repo project.

[jkoritzinsky]

I'll focus on putting the pieces back together (and possibly solving item 2 as well) after the flat-flow switchover

[jkoritzinsky]

/azp run sdk-unified-build

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[jkoritzinsky]

Retargeted to release/10.0.1xx-preview4. Official build at https://dev.azure.com/dnceng/internal/_build/results?buildId=2693317&view=results.

Once the linked build is green, I can merge it in (and then it'll flow back to main in the standard SDK codeflow).

[mmitche]

In the latest build, PackageArtifacts and PDBArtifacts are missing. I'll take a look in a few to see if I can figure out why.

[jkoritzinsky]

I think I figured it out. The vertical manifests are expected to have the VerticalName attribute on them (that's how JoinVerticals knows the vertical an item is from).

New official build here: https://dev.azure.com/dnceng/internal/_build/results?buildId=2693958&view=results

[jkoritzinsky]

New build link: https://dev.azure.com/dnceng/internal/_build/results?buildId=2694174&view=results

[jkoritzinsky]

New build link: https://dev.azure.com/dnceng/internal/_build/results?buildId=2694366&view=results

[jkoritzinsky]

Build link: https://dev.azure.com/dnceng/internal/_build/results?buildId=2694444&view=results

[mmitche]

The latest issue appears to be that there are assets that are duplicated between verticals. These are PDBs. Two/three possible ideas:

• These all should be published (no real overlap) and we could resolve this by staging the PDBs to unique directories.
• These are cases where we should just select a single vertical
• (last resort)...ignore asset selection issues in PDBs for now.

[mmitche]

New build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2695185&view=results