Skip to content

[WIP] Update to wix5 #62885

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.

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

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft

[WIP] Update to wix5 #62885

wants to merge 19 commits into from

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Jul 23, 2025

@github-actions github-actions bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jul 23, 2025
@wtgodbe
Copy link
Member Author

wtgodbe commented Jul 23, 2025

First test build, since we want to test signing too: https://dev.azure.com/dnceng/internal/_build/results?buildId=2756469&view=results

@wtgodbe
Copy link
Member Author

wtgodbe commented Jul 23, 2025

##[error]C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(43,3): error MSB4011: "D:\a_work\1\s.packages\microsoft.dotnet.arcade.sdk\10.0.0-beta.25372.103\tools\BeforeCommonTargets.targets" cannot be imported again. It was already imported at "D:\a_work\1\s.packages\microsoft.dotnet.arcade.sdk\10.0.0-beta.25372.103\Sdk\Sdk.targets (13,3)". This is most likely a build authoring error. This subsequent import will be ignored. [D:\a_work\1\s\src\Installers\Windows\AspNetCoreModule-Setup\ANCMIISExpressV2\AncmIISExpressV2.wixproj]

This is because Arcade's sdk.targets imports that file: https://github.com/dotnet/arcade/blob/2586309f3f8553152e2d1d54612e9199975f9cd4/src/Microsoft.DotNet.Arcade.Sdk/sdk/Sdk.targets#L13

And so does Microsoft.Common.CurrentVersion.targets:

<Import Project="$(CustomBeforeMicrosoftCommonTargets)" Condition="'$(CustomBeforeMicrosoftCommonTargets)' != '' and Exists('$(CustomBeforeMicrosoftCommonTargets)')"/>

due to: https://github.com/dotnet/arcade/blob/2586309f3f8553152e2d1d54612e9199975f9cd4/src/Microsoft.DotNet.Arcade.Sdk/tools/Settings.props#L7

This makes me think that typically Microsoft.Common.CurrentVersion.targets would come before sdk.targets in the preprocessed file, but in this case it's the other way around. We're doing a bunch of weird custom import stuff here so that's not terribly surprising. Investigating

@wtgodbe
Copy link
Member Author

wtgodbe commented Jul 24, 2025

I see, the Wix sdk.targets imports Microsoft.Common.targets now:

<Import Project="$(MSBuildToolsPath)\Microsoft.Common.targets" />

Which causes Microsoft.Common.CurrentVersion.targets to get imported, which causes the double-import of BeforeCommonTargets.targets from Arcade. @NikolaMilosavljevic @joeloff did either of you guys run into this?

@joeloff
Copy link
Member

joeloff commented Jul 24, 2025

I see, the Wix sdk.targets imports Microsoft.Common.targets now:

<Import Project="$(MSBuildToolsPath)\Microsoft.Common.targets" />

Which causes Microsoft.Common.CurrentVersion.targets to get imported, which causes the double-import of BeforeCommonTargets.targets from Arcade. @NikolaMilosavljevic @joeloff did either of you guys run into this?

Nope, didn't run into that for the SDK

@@ -1,13 +1,12 @@
<?xml version="1.0" encoding="utf-8"?>
<Theme xmlns="http://wixtoolset.org/schemas/thmutil/2010">
<Theme xmlns="http://wixtoolset.org/schemas/v4/thmutil">
<Window Width="485" Height="347" HexStyle="100a0000" FontId="0">#(loc.Caption)</Window>
<Font Id="0" Height="-12" Weight="500" Foreground="000000" Background="FFFFFF">Segoe UI</Font>
<Font Id="1" Height="-24" Weight="500" Foreground="000000">Segoe UI</Font>
<Font Id="2" Height="-22" Weight="500" Foreground="666666">Segoe UI</Font>
<Font Id="3" Height="-12" Weight="500" Foreground="000000" Background="FFFFFF">Segoe UI</Font>
<Font Id="4" Height="-12" Weight="500" Foreground="ff0000" Background="FFFFFF" Underline="yes">Segoe UI</Font>
Copy link
Member

@joeloff joeloff Jul 24, 2025

Choose a reason for hiding this comment

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

Take a look at the SDK PR - you should consider using the predefined system colors. These now work with high contrast themes, making the installer pass a number of accessibility tests that failed in v3

@NikolaMilosavljevic
Copy link
Member

@wtgodbe would it be possible to incorporate wixpack work into this PR? It would help uncover any issues with wixpacks and CreateWixBuildWixpack task that is needed for resign/repack functionality.

Essentially, you'd want the same target that SDK has: https://github.com/dotnet/sdk/blob/1b4646d2244ffa462e59f10bc752f2ff6f94b569/src/Layout/pkg/windows/Directory.Build.targets#L46-L68 CoreCompile is a Wix5 target - running the new task after that target ensures that we have all the wix properties set.

<WixpackOutputDir>$(ArtifactsNonShippingPackagesDir)</WixpackOutputDir>
</PropertyGroup>

<CreateWixBuildWixpack
Copy link
Member Author

Choose a reason for hiding this comment

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

@NikolaMilosavljevic I'm already moving to CreateWixBuildWixpack here

@wtgodbe
Copy link
Member Author

wtgodbe commented Aug 4, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants