Skip to content

Define a concept of Artifact Visibility#15344

Merged
jkoritzinsky merged 12 commits into
dotnet:mainfrom
jkoritzinsky:artifact-visibility
Dec 24, 2024
Merged

Define a concept of Artifact Visibility#15344
jkoritzinsky merged 12 commits into
dotnet:mainfrom
jkoritzinsky:artifact-visibility

Conversation

@jkoritzinsky

@jkoritzinsky jkoritzinsky commented Dec 17, 2024

Copy link
Copy Markdown
Member

Asset Visibility

As part of building the VMR, we've found that we need to produce assets in multiple jobs that we used to produce in only one job. We have talked about doing asset selection through a list of jobs or asset name patterns. This works when we're only producing the assets that we want to publish.

However, we've discovered that in the VMR, we also need to build some additional packages that we never want to ship that we are effectively forced to name like a shipping package (in particular, we need to build a crossgen2 and ilc for the build host machine). These host-machine packages aren't assets we want to ship as they are built using LKG assets and assets that aren't targeting our "portable" build setup.

To handle these cases, we'd have to be very careful with a priority list of verticals as it would need to consider the machines we build on (so macOS x64 and macOS arm64 ordering would have to be ordered such that the primary vertical matches our build machine).

We looked at different artifact types and determined we have 3 visibilities, separate from Shipping/NonShipping:

  • Vertical: Used within a VMR vertical, not published in the merged manifest. Not allowed in non-VMR builds.
  • Internal: Used within a single AzDo pipeline run. Published from a job up to AzDO, but excluded from BAR and darc publishing. Good for cross-job and cross-BuildPass -only assets.
  • External (default): Regular publishing.

To double check:

Validation that the jobs are dropped before going to BAR and the dropped artifacts are not uploaded to feeds by darc:

WindowsDesktop job that published to BAR with all rid-agnostic assets and exe installers marked as "Internal": https://dev.azure.com/dnceng/internal/_build/results?buildId=2604238&view=results
BAR record for that job showing that no rid-agnostic packages nor exe installers are present: https://maestro.dot.net/channel/529/azdo:dnceng:internal:dotnet-windowsdesktop/build/250107
Build Promotion job that shows that the internal assets were excluded from upload (the job failure later is due to an unrelated ongoing FR issue): https://dev.azure.com/dnceng/internal/_build/results?buildId=2604256&view=logs&j=ba23343f-f710-5af9-782d-5bd26b102304&t=74531eb2-9b39-5603-839e-94e3ba212b65&l=278

…vertical builds.

Don't publish artifacts with non-External visibility
@jkoritzinsky jkoritzinsky marked this pull request as ready for review December 18, 2024 21:02
Comment thread src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj Outdated
Comment thread src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs Outdated
Comment thread src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs Outdated
ViktorHofer
ViktorHofer previously approved these changes Dec 18, 2024

@ViktorHofer ViktorHofer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM aside from one comment about the code in the PublishArtifactsInManifestBase type.

Comment thread src/Microsoft.DotNet.Arcade.Sdk/tools/Publish.proj Outdated
Comment thread src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs Outdated
@mmitche

mmitche commented Dec 18, 2024

Copy link
Copy Markdown
Member

Can you add documentation for this in https://github.com/dotnet/arcade/blob/992d2e5d76cccb72ad611ba2141e25db86cd59bd/Documentation/ArcadeSdk.md?

…fest from being inserted in the first place.

Remove changes to the publishing pipeline as it won't ever see a non External visibility any more.

Obsoletes dotnet/arcade-services#4254
@jkoritzinsky

Copy link
Copy Markdown
Member Author

I've updated this PR to exclude the assets before they even get into the manifest, but after uploading. That should remove the need to account for it in various places.

@jkoritzinsky

Copy link
Copy Markdown
Member Author

Added docs. This is ready for another round of review.

Comment thread src/Microsoft.DotNet.Build.Tasks.Feed/src/PushToBuildStorage.cs Outdated
@jkoritzinsky jkoritzinsky merged commit 6797ec2 into dotnet:main Dec 24, 2024
@jkoritzinsky jkoritzinsky deleted the artifact-visibility branch December 24, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants