Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions build/NuSpecs/Microsoft.WindowsAppSDK.Foundation.targets
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

<Import Project="$(MSBuildThisFileDirectory)Microsoft.WindowsAppSDK.AutoInitializerCommon.targets" />
<Import Project="$(MSBuildThisFileDirectory)Microsoft.WindowsAppSDK.AutoInitializer.CS.targets" Condition="'$(WindowsAppSdkAutoInitialize)' == 'true'"/>

<!-- Fix for RuntimeIdentifier issue during publish of class libraries -->
Copy link
Owner

Choose a reason for hiding this comment

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

✅ Code Quality - Integration & Documentation

Clean integration approach:

  • Import is logically placed after other core imports
  • Clear comment explains the purpose
  • Follows existing file naming conventions

✅ Proper placement: Added in the right location within the target file structure
✅ Self-documenting: The comment clearly explains what this import addresses
✅ Consistent style: Matches the existing import pattern in the file

Minor suggestion: Consider adding the issue number to the comment for traceability:

<!-- Fix for RuntimeIdentifier issue during publish of class libraries (Issue #13) -->

<Import Project="$(MSBuildThisFileDirectory)Microsoft.WindowsAppSDK.PublishFix.targets" />

<ItemGroup>
<Compile Condition="'$(WindowsAppSDKAggregatePackage)' == 'true' and '$(WindowsAppSdkIncludeVersionInfo)'=='true'" Include="$(MSBuildThisFileDirectory)..\include\WindowsAppSDK-VersionInfo.cs" />
Expand Down
25 changes: 25 additions & 0 deletions build/NuSpecs/Microsoft.WindowsAppSDK.PublishFix.targets
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<!--
Copy link
Owner

Choose a reason for hiding this comment

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

✅ Code Quality - Documentation & Structure

Exceptional documentation quality:

✅ Comprehensive header comment explains:

  • What problem this solves
  • Why the problem occurs
  • How the solution works
  • Technical approach used

✅ Professional structure:

  • Standard XML declaration
  • Proper copyright notice
  • Clean MSBuild project structure
  • Logical organization

✅ Maintainability: Future developers will easily understand the purpose and approach without needing to research the original issue.

Fix for issue where RuntimeIdentifier should not be used when determining outputs of referenced class libraries during publish.

The problem occurs when publishing a WinUI app with a RuntimeIdentifier specified - the RID gets incorrectly passed
to class library projects when MSBuild is determining their outputs. Class libraries should not consider the RID
since they are platform-agnostic.

This fix uses MSBuild's built-in ProjectReferenceGlobalPropertiesToRemove mechanism to prevent RuntimeIdentifier
from being passed to referenced projects during publish operations only.
-->

<PropertyGroup Condition="'$(RuntimeIdentifier)' != '' and ('$(IsPublishing)' == 'true' or '$(DeployOnBuild)' == 'true' or '$(PublishProfile)' != '')">
Copy link
Owner

Choose a reason for hiding this comment

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

✅ Logic Correctness - Publish Condition Analysis

Strong Logic Implementation: The condition logic is well-designed and comprehensive:

Condition="'$(RuntimeIdentifier)' != '' and ('$(IsPublishing)' == 'true' or '$(DeployOnBuild)' == 'true' or '$(PublishProfile)' != '')"

✅ Correctly detects publish scenarios:

  • $(IsPublishing) - Standard MSBuild publish
  • $(DeployOnBuild) - Visual Studio publish
  • $(PublishProfile) - Profile-based publish

✅ Only activates when RID is set: Prevents unnecessary processing when no RuntimeIdentifier is specified.

💡 Recommendation: Consider adding a property to allow users to opt-out if needed: and '$(DisableRuntimeIdentifierFix)' != 'true'

<!--
During publish operations, ensure RuntimeIdentifier is not passed to referenced class library projects.
This prevents the RID from affecting class library output path resolution during publish.
-->
<ProjectReferenceGlobalPropertiesToRemove Condition="'$(ProjectReferenceGlobalPropertiesToRemove)' == ''">RuntimeIdentifier</ProjectReferenceGlobalPropertiesToRemove>
Copy link
Owner

Choose a reason for hiding this comment

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

✅ Code Quality - Property Handling Logic

Excellent defensive programming. The two-condition approach properly handles both scenarios:

Scenario 1 - Empty property:

Condition="'$(ProjectReferenceGlobalPropertiesToRemove)' == ''"

Sets the entire property to just RuntimeIdentifier

Scenario 2 - Existing property:

Condition="... and !$(ProjectReferenceGlobalPropertiesToRemove.Contains('RuntimeIdentifier'))"

Appends ;RuntimeIdentifier only if not already present

✅ Prevents duplicate entries - The Contains check is crucial for idempotent behavior
✅ Maintains existing values - Preserves any other properties already in the list
✅ Proper MSBuild syntax - Uses standard semicolon-delimited format

<ProjectReferenceGlobalPropertiesToRemove Condition="'$(ProjectReferenceGlobalPropertiesToRemove)' != '' and !$(ProjectReferenceGlobalPropertiesToRemove.Contains('RuntimeIdentifier'))">$(ProjectReferenceGlobalPropertiesToRemove);RuntimeIdentifier</ProjectReferenceGlobalPropertiesToRemove>
</PropertyGroup>

</Project>