Skip to content

Conversation

@joperezr
Copy link
Member

@joperezr joperezr commented Sep 26, 2025

Description

Official builds are currently broken since arcade updated the build scripts that were edited in this change: #11504 causing the buildExtension switch to not be handled correctly. Instead of trying to pass in a switch, this change just directly passes in the property to the build which will make it so the extension gets built.

FYI: @radical @adamint

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • No
  • Did you add public API?
    • No
  • Does the change make any security assumptions or guarantees?
    • No
  • Does the change require an update in our Aspire docs?
    • No

Copilot AI review requested due to automatic review settings September 26, 2025 19:39
@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11678

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11678"

@github-actions github-actions bot added the area-engineering-systems infrastructure helix infra engineering repo stuff label Sep 26, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This change fixes broken official builds by modifying how extensions are built in the internal build pipeline. Instead of using a switch parameter that isn't being handled correctly after arcade build script updates, the change passes the property directly to the build command.

  • Removes the -buildExtension switch from the build command
  • Adds /p:BuildExtension=true property to directly enable extension building
  • Includes a minor formatting fix in the azure-pipelines.yml file

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
eng/pipelines/templates/BuildAndTest.yml Replaces -buildExtension switch with /p:BuildExtension=true property
eng/pipelines/azure-pipelines.yml Minor formatting fix for policheck configuration

$(_InternalBuildArgs)
/p:TargetRids=${{ join(':', parameters.targetRids) }}
/p:SkipTestProjects=true
/p:BuildExtension=true
Copy link
Member

Choose a reason for hiding this comment

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

Should we also go ahead and remove the -buildExtension, and --build-extension params from the sh and ps1 scripts?

Copy link
Member

Choose a reason for hiding this comment

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

Why? Don't we want this for PRs?

Copy link
Member Author

Choose a reason for hiding this comment

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

bash I think is working fine with the switch (at least that seemed to be the case locally). I do think that this should be cleaned up but perhaps we do want to actually fix it so the switch is supported? I'll leave that up to @adamint but I think we should not block this PR on that as currently the builds from main are broken.

@joperezr joperezr enabled auto-merge (squash) September 26, 2025 19:58
@joperezr joperezr merged commit 2fbae16 into dotnet:main Sep 26, 2025
311 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 9.6 milestone Sep 26, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-engineering-systems infrastructure helix infra engineering repo stuff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants