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

Isolate package flow between repos #18557

Merged
merged 44 commits into from
Feb 22, 2024
Merged

Conversation

mthalman
Copy link
Member

@mthalman mthalman commented Feb 7, 2024

Contributes to dotnet/source-build#3608

As a prerequisite to enabling parallel builds of the repos in the VMR, it's necessary to isolate the package flow between repos. This ensures that repo dependencies are defined correctly. Prior to these changes, the packages outputted by repos went to the same directory (Shipping or NonShipping) and those two directories were used as the package feed inputs to dependent repos. This would be dangerous in the context of running builds in parallel across repos because any ill-defined dependencies could lead to race conditions. For example, consider the msbuild repo's dependency on System.Text.Json from the runtime repo. If runtime was not defined as a dependency of msbuild, then there's no guarantee that System.Text.Json will exist when it restores it. Based on timing, it may exist in one build and then not exist in another build.

To solve this problem, the packages output from a repos build are placed in a repo-specific package location as a sub-directory of the package location (Shipping or NonShipping). Previously, all repos would output their packages to artifacts/packages/Release/[NonShipping|Shipping]. With these changes, they output to artifacts/packages/Release/[NonShipping|Shipping]/<repo-name>. This isolates all packages on a per-repo basis. The next step is to provide access to these packages based on dependencies. Going back to msbuild's dependency on runtime, this is accomplished by modifying msbuild's nuget.config file to include feeds specific to runtime:

<add key="source-built-runtime" value="/vmr/artifacts/packages/Release/Shipping/runtime/" />
<add key="source-built-transport-runtime" value="/vmr/artifacts/packages/Release/NonShipping/runtime/" />

@mthalman mthalman requested a review from a team as a code owner February 7, 2024 19:39
@mthalman
Copy link
Member Author

mthalman commented Feb 7, 2024

This kind of package isolation also has use in other scenarios outside the context of parallel build support. @mmitche - Can you comment more on that?

@NikolaMilosavljevic
Copy link
Member

Presumably blob-feed/assets can continue to be shared and does not require repo-level isolation?

@mthalman
Copy link
Member Author

mthalman commented Feb 7, 2024

Presumably blob-feed/assets can continue to be shared and does not require repo-level isolation?

I believe so since I don't know of any repos that depend on the assets directory (except for installer but that's at the end of the build anyway).

Comment on lines 8 to 9
<RepositoryReference Include="arcade" />
<RepositoryReference Include="templating" />
<RepositoryReference Include="source-build-reference-packages" Condition="'$(DotNetBuildSourceOnly)' == 'true'" />
Copy link
Member

Choose a reason for hiding this comment

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

How did you determine that list? Does this represent the source-build or the vertical build graph? As an example, is the removal of templating here due to the repository not being a dependency for source build but for vertical build? cc @mmitche

Copy link
Member Author

Choose a reason for hiding this comment

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

The removal of templating here is because there's literally no dependency on the templating repo output.

But you bring up a good point about the difference between vertical and source build. In some cases, there are dependencies that exist purely to satisfy source build. In order words, the dependencies for vertical build would be a subset of the dependencies for source build. I'll investigate that more so that we can define these depends with the source-only flag as appropriate.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we will need to go and annotate these dependencies with SB properties in some cases. The "Microsoft" graph should emulate as closely the active flow that happens via Maestro on a per-release basis. The SB graph will start with this flow, and then add new edges where we repos are redistributing implementation packages of older versions.

@MichaelSimons
Copy link
Member

MichaelSimons commented Feb 7, 2024

Are there a set of rules/guidelines for determining when to/when not to declare a dependency as a repository dependency? It would be good to capture this in a guideline.

@ViktorHofer
Copy link
Member

... and how to keep those in sync.

@mthalman
Copy link
Member Author

Ugh, looks like I need to resolve conflicts still.

@mthalman
Copy link
Member Author

Yeah, the changes from #18591 dealt a serious blow here. This will take a bit of time.

@ViktorHofer
Copy link
Member

@mthalman This is really great! Would you mind updating the PR description to the current state? I think this will help devs coming back to this PR. When merging please also make sure that the commit description is up-to-date in regards to the added feeds and the output paths (artifacts/packages vs artifacts/obj/x64/Release/blob-feed/...).

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.

5 participants