@heaths Please look CI fail.

@heaths take a look at https://dev.azure.com/powershell/PowerShell/_build/results?buildId=53706&view=logs&j=1057fe42-c88b-5deb-88ad-59245c714665&t=1888043b-254d-5761-80ec-47d0d8adc59a, the test detected some file changes, if this is expected, just follow the instructions in the output

I updated the wxs for you and filed: #12799

@TravisEz13, thanks. I was going to get to this tonight.

Were these actually removed, though? I'm not clear on whether their presence or not is actually a bug. They aren't needed except for ARPPRODUCTICON which must reference a row in the Icon table but, unless you're advertising your product (almost no one does that anymore - Office stopped over a decade ago), shouldn't be used for shortcuts.

Were these actually removed, though?

I wonder too how WiX works - wxs file still contains references to the icons.

@iSazonov, what exactly do you mean? As a long-time developer on WiX (I designed and implemented many features in it, most notably package ref-counting that other installers use as well), I could probably explain if you can clarify what you meant.

When the <Icon> is used, the binding (during linking, which is light.exe) knows to stream in a file reference; otherwise, when just a normal <File> is process the binder puts it in a cabinet that is later left external or streamed into the MSI based on <Media> entries (<MediaTemplate> provides default behavior and is often easier for most packages).

My main change here was to remove most GUIDs. They aren't needed, except for the UpgradeCode property (you normally want that fixed) and shared components without a single key path resource. In fact, just being a shared component - if it always resolves to the same key path - will get the same GUID. This change is mainly about letting WiX create a durable GUID based on the key path at build time, which is far more reliable and easier than people having to maintain it, which wasn't being done here and the main problem with files and registry values not getting cleaned up (they were ref-counted). See https://heaths.dev/setup/2020/05/13/simple-wix-project.html for more explanation and links to many of my old blog posts on what is now the VS Setup team blog (used to be my professional blog - which I used mostly for team purposes - up until a year or so ago).

Heath Stewart’s Blog

Simple WiX Project

Globally-unique identifiers (GUIDs) are often the brunt of jokes. Windows Installer packages (.msi files) are full of them, from the required ProductCode, to highly-recommended UpgradeCode, package codes, and required component GUIDs. Authoring an MSI doesn’t require you create and manage so many GUIDs, however. In fact, Windows Installer XML (WiX) has in the last few years made great strides in making sure you don’t have to, and recommends you don’t.

what exactly do you mean?

I mean @TravisEz13 's commit and your question "Were these actually removed, though?".

Last I updated the file via git rebase they were still there.

Yes, they are on file system. My concern is that it seems wxs file contains references to the files.

They shouldn't anymore. It's possible when I generated my repo wasn't entirely clean. It was a bit behind before I rebased on upstream/master this past weekend since I've been swamped with work stuff.

@heaths

Were these actually removed, though? I'm not clear on whether their presence or not is actually a bug.

in CI we generate multiple packages at at time. There is probably a bug where one of the package generations deletes the icons. There is code in the packaging to create a staging folder. We need to make sure all types of packages that add/or remove files from the build use a staging directory.

I filed a bug to investigate.

I updated the wxs for you and filed: #12799

#12799 was an existing bug. We can merge this PR.

Thanks. If I can find time, there's some more clean-up I'd like to do (you can do almost everything in the .wxs itself, which makes it easier to maintain since it's not spread all over), but wasn't necessary for now. I also recommend passing parameters instead of setting environment variables, as the scope is smaller (though, at least you scope them to the current and child processes).

@heaths Thanks for great contribution!

Thanks for your contribution

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes