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

Ensure container generation uniformly reports outputs to MSBuild #47234

Merged
merged 2 commits into from
Mar 7, 2025

Conversation

baronfel
Copy link
Member

@baronfel baronfel commented Mar 3, 2025

Fixes dotnet/sdk-container-builds#622.

Adds Returns to the single- and multi-arch container Targets so that regardless of how many containers are created consumers can get the whole list.

Adds an output of the generated container base image digest Label to the CreateNewImage Task so that the ContainerLabel itemgroup is updated after the Task is completed.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Containers Related to dotnet SDK containers functionality untriaged Request triage from a team member labels Mar 3, 2025
@baronfel baronfel force-pushed the container-msbuild-outputs branch 3 times, most recently from 7219cad to 0790daf Compare March 4, 2025 15:19
@baronfel baronfel force-pushed the container-msbuild-outputs branch from 0790daf to e8c5ae1 Compare March 4, 2025 16:21
@baronfel baronfel marked this pull request as ready for review March 5, 2025 15:43
@baronfel baronfel requested review from Copilot and a team March 5, 2025 15:43

Choose a reason for hiding this comment

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

PR Overview

This pull request updates container generation logic so that both single- and multi-architecture builds uniformly report outputs to MSBuild while also providing the base image digest label for consumers. Key changes include:

  • Adding a new [Output] property (GeneratedDigestLabel) in the CreateNewImage interface.
  • Modifying the ImageBuilder to return a tuple for the base image digest label.
  • Updating CreateNewImage to capture and assign the digest label output.

Reviewed Changes

File Description
src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateNewImage.Interface.cs Added the new GeneratedDigestLabel output property.
src/Containers/Microsoft.NET.Build.Containers/ImageBuilder.cs Modified AddBaseImageDigestLabel to return a tuple with the base image digest label.
src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateNewImage.cs Updated ExecuteAsync to destructure the tuple and assign the digest label output.

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/Containers/Microsoft.NET.Build.Containers/ImageBuilder.cs:110

  • [nitpick] Consider using more descriptive tuple element names (e.g., 'digestLabelName' and 'digestValue') to clarify their purpose in the returned tuple.
internal (string name, string value) AddBaseImageDigestLabel()

src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateNewImage.cs:164

  • [nitpick] Consider renaming the tuple variables 'baseImageLabel' and 'baseImageDigest' to more descriptive names (e.g., 'digestLabelName' and 'digestValue') to better reflect their roles.
(baseImageLabel, baseImageDigest) = imageBuilder.AddBaseImageDigestLabel();
@baronfel
Copy link
Member Author

baronfel commented Mar 6, 2025

@dotnet/product-construction there's a test failure in the ASP.NET tests in the sourcebuild leg that I think is unrelated to this change - it's a port-in-use failure.

Copy link
Member

@surayya-MS surayya-MS left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@baronfel baronfel merged commit 0df21a7 into dotnet:main Mar 7, 2025
39 checks passed
@baronfel baronfel deleted the container-msbuild-outputs branch March 7, 2025 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Containers Related to dotnet SDK containers functionality untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure all container information is available from MSBuild in publicly-supported structures
2 participants