Keep runtimelab IntermediateArtifacts on classic build artifacts#131033
Conversation
#128498 migrated this step to PublishPipelineArtifact, which would break runtimelab consumers: they still require multiple jobs publishing to the same 'IntermediateArtifacts' artifact. Revert just this step to 1ES.PublishBuildArtifacts@1 / PublishBuildArtifacts@1. This can be removed once all runtimelab branches moves to v3/v4 publishing.
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the intermediate-artifact upload step under eng/pipelines/ to use classic Azure DevOps Build Artifacts (container) instead of Pipeline Artifacts, preserving behavior needed by downstream consumers that rely on multiple jobs publishing into a single same-named artifact.
Changes:
- Replaces the Pipeline Artifact publishing template usage with Build Artifact publishing tasks (with an official vs non-official task split).
- Adds an inline note explaining why classic Build Artifacts are required for this specific artifact.
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "868e1ed7cc0c21c68f9d415309c10bb5edaae559",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "c536338d89a6ccc0bcbca46e44e3343d990e06aa",
"last_reviewed_commit": "868e1ed7cc0c21c68f9d415309c10bb5edaae559",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "c536338d89a6ccc0bcbca46e44e3343d990e06aa",
"last_recorded_worker_run_id": "29690737115",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "868e1ed7cc0c21c68f9d415309c10bb5edaae559",
"review_id": 4730927734
}
]
} |
There was a problem hiding this comment.
Holistic Review
Motivation: PR #128498 migrated all eng/pipelines artifact publishing from classic build (container) artifacts to pipeline artifacts. That migration is incompatible with the IntermediateArtifacts step, because dotnet/runtimelab consumers rely on multiple jobs publishing into a single same-named IntermediateArtifacts artifact — a merge semantic that pipeline artifacts (PublishPipelineArtifact@1) do not support. This PR reverts only that one step back to classic build artifacts to avoid breaking runtimelab.
Approach: The step previously delegated to the shared publish-pipeline-artifacts.yml template. Since #128498 also deleted the corresponding publish-build-artifacts.yml template, this PR inlines the classic-artifact publish directly: an ${{ if parameters.isOfficialBuild }} / ${{ else }} split selecting 1ES.PublishBuildArtifacts@1 for official builds and PublishBuildArtifacts@1 otherwise, both with ArtifactType: container. This faithfully reproduces the pre-#128498 behavior (same PathtoPublish, ArtifactName: IntermediateArtifacts, ArtifactType: container) and matches the official/non-official task-selection pattern used by the surviving publish-pipeline-artifacts.yml template. A clear NOTE comment documents why this step diverges from the rest of the migrated pipeline and records that it can be removed once runtimelab moves to v3/v4 publishing.
Summary: This is a small, well-scoped, and well-documented infrastructure revert. The compile-time if/else and input keys are correct for both the 1ES and non-1ES task variants, the artifact name/type are preserved so runtimelab's multi-job merge continues to work, and the inlining is the right choice given the shared template no longer exists. The single-file change is self-contained with no other consumers of this step affected (only eng/pipelines/runtimelab-official.yml references it). No correctness, security, or maintainability concerns found. LGTM.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 47.1 AIC · ⌖ 10.4 AIC · ⊞ 10K
|
/ba-g unrelated infrastructure issues |
#128498 migrated this step to PublishPipelineArtifact, which would break runtimelab consumers: they still require multiple jobs publishing to the same 'IntermediateArtifacts' artifact. Revert just this step to 1ES.PublishBuildArtifacts@1 / PublishBuildArtifacts@1.
This can be removed once all runtimelab branches moves to v3/v4 publishing.