Skip to content

Update EmsdkPackageVersion to use version from dependency flow#128261

Open
akoeplinger wants to merge 3 commits into
mainfrom
akoeplinger-patch-2
Open

Update EmsdkPackageVersion to use version from dependency flow#128261
akoeplinger wants to merge 3 commits into
mainfrom
akoeplinger-patch-2

Conversation

@akoeplinger
Copy link
Copy Markdown
Member

It got hardcoded a while ago to workaround a build problem.

It got hardcoded a while ago to workaround a build problem.
Copilot AI review requested due to automatic review settings May 15, 2026 16:44
@github-actions github-actions Bot added the area-codeflow for labeling automated codeflow label May 15, 2026
Copy link
Copy Markdown
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 PR updates the build’s Emscripten SDK provisioning versioning so it no longer relies on a hardcoded EmsdkPackageVersion value in eng/Versions.props, instead sourcing it from a dependency-flow-updated property.

Changes:

  • Replace the hardcoded EmsdkPackageVersion (10.0.0) with $(MicrosoftDotNetApiCompatTaskPackageVersion).
  • Add an explanatory comment about why a stable, dependency-flow-backed version property is used.

Comment thread eng/Versions.props Outdated
@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings May 18, 2026 13:20
Copy link
Copy Markdown
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

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

Comment thread eng/Versions.props
Comment on lines 170 to +174
<!-- emscripten workload package when testing workloads -->
<!-- we're using MicrosoftDotNetApiCompatTaskPackageVersion since the emscripten workload package ID contains a changing version. This one uses sdk-style feature band version numbers. -->
<MicrosoftNETRuntimeEmscriptenVersion>$(MicrosoftDotNetApiCompatTaskPackageVersion)</MicrosoftNETRuntimeEmscriptenVersion>
<EmsdkPackageVersion>10.0.0</EmsdkPackageVersion>
<!-- we're using MicrosoftNETCoreAppRefPackageVersion since the emsdk package ID contains a changing version. This one uses runtime version numbers. -->
<EmsdkPackageVersion>$(MicrosoftNETCoreAppRefPackageVersion)</EmsdkPackageVersion>
@github-actions
Copy link
Copy Markdown
Contributor

Caution

Security scanning requires review for Code Review

Details

The threat detection results could not be parsed. The workflow output should be reviewed before merging.

Review the workflow run logs for details.

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #128261

Holistic Assessment

Motivation: Justified — removing a hardcoded version (10.0.0) that was a workaround and replacing it with the dependency-flowed version eliminates manual bump maintenance.

Approach: Correct — uses $(MicrosoftNETCoreAppRefPackageVersion) (currently 11.0.0-preview.5.26257.113) which provides runtime version numbers matching the emsdk package ID pattern. This mirrors how MicrosoftNETRuntimeEmscriptenVersion already uses a flowed version on the adjacent line.

Summary: ✅ LGTM. Single-property change in eng/Versions.props, aligning EmsdkPackageVersion with dependency flow. The consumers in eng/AcquireEmscriptenSdk.targets and src/mono/mono.proj will now automatically get the correct version without manual updates.


Detailed Findings

✅ Correctness — Version source is appropriate

MicrosoftNETCoreAppRefPackageVersion (defined in eng/Version.Details.props) provides runtime-style version numbers (e.g., 11.0.0-preview.5.26257.113), which matches the versioning scheme used by the emsdk NuGet packages (Microsoft.NET.Runtime.Emscripten.{EmsdkVersion}.{component}). The comment added in the PR correctly explains the rationale.

✅ Consistency — Follows established pattern

The adjacent MicrosoftNETRuntimeEmscriptenVersion already uses a flowed version ($(MicrosoftDotNetApiCompatTaskPackageVersion)) for the same reason. This PR makes EmsdkPackageVersion consistent with that approach.

✅ Scope — Minimal and focused

Only one property value changed, with appropriate explanatory comments added. No risk of unintended side effects.

Generated by Code Review for issue #128261 · ● 1.7M ·

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

Labels

area-codeflow for labeling automated codeflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants