Improve the UX and guideance around the SourceBuild metadata in the Version.Details.xml

[MichaelSimons]

There are a couple usability issues around the SourceBuild metadata in the Version.Details.xml that should be addressed.

1. The SourceBuild metadata is only allowed on one dependency per repo (this is related to [ArPow] intermediate nupkg restore tooling should detect and fail if multiple <SourceBuild .../> elements exist for one upstream #2050). When a developer adds multiple, the UX is not intuitive on what is wrong. This is illustrated in Source build creates duplicate PackageReferences #3372.
2. In the past there have been numerous issues at release time when stable versions flow in. When the SourceBuild metadata is placed on a stable versioned dependency this breaks the source-build intermediate loading because the intermediate is non-shipping and usually doesn't get the stable versioning. When this happens the SourceBuild metadata is typically moved to a nonstable versioned dependency.

There are likely several options here. One possible solution is to explicitly add the SourceBuild.Intermediates as dependencies and only mark those as source-build.

[mmitche]

We discussed this in the Unified Build meeting today. Summary:

We should have repos take direct dependencies on the source build intermediate packages. This could take two forms:

• As a special type of dependency as defined here; Add a SourceBuild dependency section into Version.Details.xml arcade-services#2431
• Simply taking a dependency on the SB intermediates (cheaper, but less elegant)

After repos completed this work, which is largely mechanical, we would change the SB Version.Details.xml logic to disallow source build elements anywhere else.

This could be completed in parallel with documentation work in #3358

[ellahathaway]

Proposal for changes within the scope of this issue:

Introduction:

The purpose of this proposal is to introduce a new element, SourceBuild, to the existing Version.Details.Xml file. This proposal outlines the structure of the element, its integration with Darc, and the rollout plan.

1. Use in Version.Details.xml:

• Location: The SourceBuild elements will be placed at the bottom of the Version.Details.Xml file.
• Description: This section containing the SourceBuild elements will be clearly demarcated by a comment explaining the section's purpose, which is to pull in intermediates for the specified repositories.
• Benefits of Using Version.Details.Xml:
  • Improved UX: Consolidating this information into Version.Details.Xml enhances user experience by providing a centralized location for all version-related details.
  • Darc Infrastructure: Leveraging the existing infrastructure of Darc simplifies integration and ensures consistency across repositories.

2. Element Details:

• The SourceBuild attribute will contain the following information:
  • RepoName: Name of the repository.
  • ManagedOnly: Boolean indicating whether a RID suffix is necessary on the intermediate NuGet package.
  • Uri: URI of the source repository.
  • Sha: Commit hash.
  • Version: Version of the build.

3. Example Structure:

<SourceBuild RepoName="source-build-reference-packages" ManagedOnly="true" Version="9.0.0-alpha.1.24072.5"> 
  <Uri>https://github.com/dotnet/source-build-reference-packages</Uri>
  <Sha>2249c9a81ec9b6cd86791c21e6e69fbc07099788</Sha>
</SourceBuild>

4. Integration with Darc:

• Darc Changes: Darc will be updated to recognize and utilize the SourceBuild element.
• Functionality: Darc will incorporate this attribute into its dependency management process.

5. Rollout Plan:

• Documentation: Provide comprehensive documentation on how to integrate and utilize the attribute within repositories. Update documentation that currently describes the current SourceBuild metadata.
• Changes: Apply the new elements/structure to each repo's Version.Details.xml. A tracking issue should be used to track these changes in the repos.

[mthalman]

Location: The SourceBuild elements will be placed at the bottom of the Version.Details.Xml file.

A child of the Dependencies element then? Or within ProductDependencies/ToolsetDependencies?

ManagedOnly: Boolean indicating whether a RID suffix is necessary on the intermediate NuGet package.

I don't know the history of this setting. Frankly I had no idea what it meant. Since we're redesigning the schema, we should update this to a more descriptive name that has better relevancy. Should this just be "IsRidSpecific"? I want the default value to be false and optional. Today we define ManagedOnly="true" all over the place; let's keep it tidy instead.

[MichaelSimons]

One piece I didn't see included is the inclusion of coherency constructs (e.g. CoherentParentDependency="Microsoft.NET.Sdk.WorkloadManifestReader"). It feels like this would be necessary.

This approach is going to be expensive to implement as it is going to require a non-trivial amount of infra changes. This makes it a bit of a hard sell to me given the priority of the other UB work. This is not a criticism that it's a bad approach. I'm just questioning the ROI versus other approaches. @mmitche what are you thoughts?

[ellahathaway]

A child of the Dependencies element then? Or within ProductDependencies/ToolsetDependencies?

Good point. This should've been clarified. I imagine this element acting as if we are "simply taking a dependency on the SB intermediates" but it looks more elegant and is separated from the other dependencies. As such, it would be a child of the "Dependencies" element. Given that some repos have their direct dependencies on SB intermediates in ProductDependencies and other repos have their direct dependencies on SB intermediates in ToolsetDependencies, I think it makes the most sense for this to be separated out from both of those elements for consistency across repos. That said, this does add a layer of complexity to the implementation.

we should update this to a more descriptive name that has better relevancy. Should this just be "IsRidSpecific"?

I agree. I think that "IsRidSpecific" is more self-describing and is a good substitute to the current "IsManagedOnly".

One piece I didn't see included is the inclusion of coherency constructs (e.g. CoherentParentDependency="Microsoft.NET.Sdk.WorkloadManifestReader"). It feels like this would be necessary

The lack of inclusion of this property is likely due to my misunderstanding of it. My understanding of CoherentParentDependency is that it's needed when the repo version and the source-built intermediate package version don't match. I was thus assuming in this case that the CoherentParentDependency could be left on the dependency itself rather than the new SB metadata. I will have to look into this more.

This approach is going to be expensive to implement as it is going to require a non-trivial amount of infra changes. This makes it a bit of a hard sell to me given the priority of the other UB work.

I completely agree. This is definitely an investment. In terms of a cheaper solution, it is always possible to take the intermediates as direct dependencies as @mmitche suggested in a comment above.

[mmitche]

CoherentParentDependency would be required for the SB intermediates. It says that the source build dependency should be taken from another repo's dependency. I'm not totally sure we have a case where it is used in SB, but for non-SB, it's like windowsdeskop saying: "Take the version of winforms that my dependency on wpf has". That ensures dependency compatibility in certain cases.

My general thoughts on this are that we if we are going to make major changes, we should do so with the forward flow/backward source flow for general Unified Build in mind. I would be sure to get some of @premun's input here. The goals would probably be to:

• Represent the component's dependencies on individual packages
• Represent the dependency on the last source version synced from the VMR
• Represent the version of the component (source sha and other info) in the VMR

@premun has some initial design here: https://github.com/dotnet/arcade/blob/main/Documentation/UnifiedBuild/VMR-Full-Code-Flow.md#synchronization-configuration

I think in the interim if we wanted to make improvements, the cheap solution is to take explicit dependencies on the source build intermediates. That avoids issues with stable flow. It doesn't solve the first issue, but we could do some minor improvements in the intermediates infra to detect multiple tagged dependencies and error out. That would get caught in most repos' SB legs.

[MichaelSimons]

It doesn't solve the first issue, but we could do some minor improvements in the intermediates infra to detect multiple tagged dependencies and error out. That would get caught in most repos' SB legs.

One of the cheapest ways to fix this UX issue would be to allow the SB metadata on multiple dependencies. The only time this should cause an error is when the dependencies are incoherent (e.g. on different versions/shas).

[mmitche]

It doesn't solve the first issue, but we could do some minor improvements in the intermediates infra to detect multiple tagged dependencies and error out. That would get caught in most repos' SB legs.

One of the cheapest ways to fix this UX issue would be to allow the SB metadata on multiple dependencies. The only time this should cause an error is when the dependencies are incoherent (e.g. on different versions/shas).

While I think this will reduce friction in the short term, during previews, I think this is more likely to cause unexpected errors when we first hit servicing and some repos start incrementally servicing.

[premun]

The new <Source> tag has already been implemented but it's not really used anywhere and can be changed.
This just stores the VMR SHA the repository was synchronized from the last time.

<Source Uri="https://github.com/dotnet/dotnet" Sha="86ba5fba7c39323011c2bfc6b713142affc76171" />

To me, this is an unrelated feature to the one proposed by Ella and they can live side by side.

To the UX problem described in the issue:

• I agree that we don't have time to invest properly in this and possibly refactoring the XMLs to only have these tags on intermediates is the cheapest best effort thing to do?
• I am not 100% sure whether we all the work proposed to here wouldn't go in vain once we switch onto the flat flow - all this coherency trouble etc might not be relevant anymore? The VMR will be resolving the coherency by just being coherent?

[mmitche]

The VMR repos may still have coherency attributes, but not for dependencies produced within the VMR.

[ellahathaway]

The cheap solution is to take explicit dependencies on the source build intermediates.
Possibly refactoring the XMLs to only have these tags on intermediates is the cheapest best effort thing to do.

The general sentiment seems to be to take explicit dependencies on the intermediates & add the tags on those dependencies. I agree that this is the most cost-effective solution. This work will be coupled with the work I'm currently doing to update the documentation on adding dependencies.

We could do some minor improvements in the intermediates infra to detect multiple tagged dependencies and error out. That would get caught in most repos' SB legs.

I agree with this. I can improve the infra to alert/error when duplicate tags are added. That solves the initial UX problem described in the issue.