Skip to content

Update build scripts to use common metadata file #534

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update build scripts to use common metadata file #534

wants to merge 4 commits into from

Conversation

hallipr
Copy link
Member

@hallipr hallipr commented Sep 23, 2025 •
edited
Loading

What does this PR do?

  • Updates all build, test, package and release scripts to use a common build_info.json file
    • This file coordinates version numbers, package properties and platform details for all build artifacts
    • all other build and pack scripts should use build_info rather than pulling properties from project files
  • Removes the static build matrix
    • build_info.json also includes dynamically generated build and tests matrices
    • rather than special casing steps in yaml, we should update the build matrix in the script and use simple boolean variables to conditionally run pipeline steps
  • Updates the pack scripts to be server-generic. All ids and server specific settings that go into a package should be pulled from build_info.json, which is built using properties from the servers' project files

Sample build_info.json

GitHub issue number?

closes #53

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Updated command list in /docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

@joshfree joshfree moved this from Untriaged to In Progress in Azure MCP Server Sep 23, 2025
@hallipr hallipr force-pushed the users/pahallis/metadata-json branch 17 times, most recently from 11811f5 to 43e55ed Compare September 30, 2025 18:40
@hallipr hallipr linked an issue Sep 30, 2025 that may be closed by this pull request
Build artifacts should have package-generic metadata #53
Open
@hallipr hallipr force-pushed the users/pahallis/metadata-json branch 5 times, most recently from 107bb97 to 0e55b21 Compare September 30, 2025 21:24
weshaggard
weshaggard reviewed Sep 30, 2025
View reviewed changes
weshaggard
weshaggard reviewed Sep 30, 2025
View reviewed changes
@hallipr hallipr force-pushed the users/pahallis/metadata-json branch from 0e55b21 to 38feff4 Compare September 30, 2025 21:52
weshaggard
weshaggard reviewed Sep 30, 2025
View reviewed changes
@hallipr
Copy link
Member Author

hallipr commented Oct 6, 2025

Also we should support pipeline tagging in the same way we do for unified pull request builds in the language repos, so that users can filter by server name in the tags dropdown for the pipeline.

I'll add a followup PR

@hallipr hallipr force-pushed the users/pahallis/metadata-json branch 2 times, most recently from af781a5 to d186241 Compare October 7, 2025 19:21
@joshfree joshfree added this to the 2025-10 milestone Oct 12, 2025
@hallipr hallipr force-pushed the users/pahallis/metadata-json branch from 3af3798 to 89e3351 Compare October 13, 2025 16:53
@hallipr hallipr requested review from conniey and g2vinay October 13, 2025 16:55
weshaggard
weshaggard reviewed Oct 13, 2025
View reviewed changes
weshaggard
weshaggard reviewed Oct 14, 2025
View reviewed changes
weshaggard
weshaggard reviewed Oct 14, 2025
View reviewed changes
weshaggard
weshaggard reviewed Oct 14, 2025
View reviewed changes
weshaggard
weshaggard reviewed Oct 14, 2025
View reviewed changes
weshaggard
weshaggard reviewed Oct 14, 2025
View reviewed changes
weshaggard
weshaggard reviewed Oct 14, 2025
View reviewed changes
weshaggard
weshaggard requested changes Oct 14, 2025
View reviewed changes
Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

Add some feedback but the main blocker is the use of the deleted matrix file that needs to be fixed.

@hallipr hallipr force-pushed the users/pahallis/metadata-json branch 2 times, most recently from 89d0038 to ca2572c Compare October 14, 2025 19:44
@hallipr hallipr force-pushed the users/pahallis/metadata-json branch from ca2572c to 4fa5e8d Compare October 14, 2025 20:16
weshaggard
weshaggard approved these changes Oct 14, 2025
View reviewed changes
Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

Changes look good but I don't want the release to fail the first time Fabric attempts to use it so please work with them to get naming figured out or opt them out of those stages if we don't have a value yet.

scbedd
scbedd requested changes Oct 14, 2025
View reviewed changes
Copy link
Contributor

@scbedd scbedd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Overall definitely looks solid to me. I have some concerns around usage of member variables for which I left comments. After those resolve, will swap to approve.

If this turns out solid livetests etc we should get this in ASAP.

eng/pipelines/templates/jobs/vsix/release-vsix.yml
echo "vsce publish --azure-credential --packagePath ""$($baseName).vsix"" --manifestPath ""$($baseName).manifest"" --signaturePath ""$($baseName).signature.p7s"""
vsce publish --azure-credential --packagePath "$($baseName).vsix" --manifestPath "$($baseName).manifest" --signaturePath "$($baseName).signature.p7s"
echo "vsce publish --azure-credential --packagePath ""$baseName.vsix"" --manifestPath ""$baseName.manifest"" --signaturePath ""$baseName.signature.p7s"""
vsce publish --azure-credential --packagePath "$baseName.vsix" --manifestPath "$baseName.manifest" --signaturePath "$baseName.signature.p7s"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vsce publish --azure-credential --packagePath "$baseName.vsix" --manifestPath "$baseName.manifest" --signaturePath "$baseName.signature.p7s"
vsce publish --azure-credential --packagePath "$($baseName.vsix)" --manifestPath "$($baseName.manifest)" --signaturePath "$($baseName.signature.p7s)"

These won't resolve in a pshell script as is right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Image

eng/pipelines/templates/jobs/vsix/release-vsix.yml
npm install -g @vscode/vsce
echo "vsce publish --azure-credential --packagePath ""$($baseName).vsix"" --manifestPath ""$($baseName).manifest"" --signaturePath ""$($baseName).signature.p7s"""
vsce publish --azure-credential --packagePath "$($baseName).vsix" --manifestPath "$($baseName).manifest" --signaturePath "$($baseName).signature.p7s"
echo "vsce publish --azure-credential --packagePath ""$baseName.vsix"" --manifestPath ""$baseName.manifest"" --signaturePath ""$baseName.signature.p7s"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "vsce publish --azure-credential --packagePath ""$baseName.vsix"" --manifestPath ""$baseName.manifest"" --signaturePath ""$baseName.signature.p7s"""
echo "vsce publish --azure-credential --packagePath `"$($baseName.vsix)`" --manifestPath `"$($baseName.manifest)`" --signaturePath `"$($baseName.signature.p7s)`""

eng/pipelines/templates/jobs/sign-and-pack.yml
parameters:
DependsOn: Sign

- ${{ each os in split('linux|windows|macOS', '|') }}:
Copy link
Contributor

Choose a reason for hiding this comment

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

Putting a pin in this. I believe that Dozie is handling the different name of the OS in the build matrix in verify-packages.yml.

Will verify and come back to this.

eng/scripts/Build-Code.ps1

if (!(Test-Path $BuildInfoPath)) {
LogError "Build info file $BuildInfoPath does not exist. Run eng/scripts/New-BuildInfo.ps1 to create it."
$exitCode = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably immediately bail here right?

eng/scripts/Build-Code.ps1

Invoke-LoggedMsBuildCommand $command -GroupOutput

$package = [ordered]@{
Copy link
Contributor

Choose a reason for hiding this comment

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

I am missing it in the large context of this PR. Where do we generate the package.json? I think you added it to the build of the csproj right?

eng/scripts/Build-Local.ps1
param(
[string] $ServerName,
[switch] $Trimmed,
[switch] $NoTrimmed,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change from Trimmed to NoTrimmed?

eng/scripts/Compress-ForSigning.ps1
}

if ($exitCode -ne 0) {
exit $exitCode
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not worth it in this PR, but later on we should parse these common arguments (for BuildInfoJson at least) in a common verification function. LATER. We should merge this PR now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anuchandy anuchandy anuchandy left review comments

Copilot code review Copilot Copilot left review comments

@scbedd scbedd scbedd requested changes

@weshaggard weshaggard weshaggard approved these changes

@joshfree joshfree Awaiting requested review from joshfree joshfree is a code owner automatically assigned from microsoft/azure-mcp

@jongio jongio Awaiting requested review from jongio jongio is a code owner automatically assigned from microsoft/azure-mcp

@wbreza wbreza Awaiting requested review from wbreza wbreza is a code owner automatically assigned from microsoft/azure-mcp

@KarishmaGhiya KarishmaGhiya Awaiting requested review from KarishmaGhiya KarishmaGhiya is a code owner automatically assigned from microsoft/azure-mcp

@tmeschter tmeschter Awaiting requested review from tmeschter tmeschter is a code owner automatically assigned from microsoft/azure-mcp

@msalaman msalaman Awaiting requested review from msalaman msalaman is a code owner automatically assigned from microsoft/azure-mcp

@alzimmermsft alzimmermsft Awaiting requested review from alzimmermsft alzimmermsft is a code owner automatically assigned from microsoft/azure-mcp

@benbp benbp Awaiting requested review from benbp

@chidozieononiwu chidozieononiwu Awaiting requested review from chidozieononiwu

@g2vinay g2vinay Awaiting requested review from g2vinay

@conniey conniey Awaiting requested review from conniey

Requested changes must be addressed to merge this pull request.

Assignees

@hallipr hallipr

Labels

Central-EngSys

Projects

Azure MCP Server
Status: In Progress

Milestone

2025-10

Development

Successfully merging this pull request may close these issues.

Build artifacts should have package-generic metadata

7 participants

@hallipr @benbp @anuchandy @weshaggard @chidozieononiwu @scbedd @joshfree