Skip to content

Fix MSI upgrade and shortcut issues - #12792

Merged
Travis Plunk (TravisEz13) merged 3 commits into
PowerShell:masterfrom
heaths:issue11995
May 27, 2020
Merged

Fix MSI upgrade and shortcut issues#12792
Travis Plunk (TravisEz13) merged 3 commits into
PowerShell:masterfrom
heaths:issue11995

Conversation

@heaths

Copy link
Copy Markdown
Contributor

Fixes #11995 and fixes #12011 based on changes described in #12011 (comment).

@iSazonov

Copy link
Copy Markdown
Collaborator

Heath Stewart (@heaths) Please look CI fail.

@iSazonov Ilya (iSazonov) added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label May 26, 2020
@iSazonov Ilya (iSazonov) added this to the 7.1.0-preview.4 milestone May 26, 2020
@SteveL-MSFT

Copy link
Copy Markdown
Member

Heath Stewart (@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

@TravisEz13

Copy link
Copy Markdown
Member

I updated the wxs for you and filed: #12799

@heaths

Copy link
Copy Markdown
Contributor Author

Travis Plunk (@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.

@iSazonov

Copy link
Copy Markdown
Collaborator

Were these actually removed, though?

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

@heaths

Heath Stewart (heaths) commented May 27, 2020

Copy link
Copy Markdown
Contributor Author

Ilya (@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
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.

@iSazonov

Copy link
Copy Markdown
Collaborator

what exactly do you mean?

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

@heaths

Copy link
Copy Markdown
Contributor Author

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

@iSazonov

Copy link
Copy Markdown
Collaborator

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

@heaths

Copy link
Copy Markdown
Contributor Author

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.

@TravisEz13

Copy link
Copy Markdown
Member

Heath Stewart (@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

@TravisEz13

Copy link
Copy Markdown
Member

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

@heaths

Copy link
Copy Markdown
Contributor Author

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).

@iSazonov

Copy link
Copy Markdown
Collaborator

Heath Stewart (@heaths) Thanks for great contribution!

@TravisEz13

Copy link
Copy Markdown
Member

Thanks for your contribution

@ghost

Copy link
Copy Markdown

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

Handy links:

Comment thread assets/Product.wxs
<?define UpgradeCode = "$(env.UpgradeCodeX86)" ?>
<?define ExplorerContextMenuDialogText = "&$(var.ProductName) $(env.SimpleProductVersion) ($(sys.BUILDARCH))"?>
<?define UpgradeCodePreview = "1d00683b-0f84-4db8-a64f-2f98ad42fe06"?>
<?define UpgradeCodeRelease = "86abcfbd-1ccc-4a88-b8b2-0facfde29094"?>

@TravisEz13 Travis Plunk (TravisEz13) Jun 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

preview upgrade codes

x64 - 39243d76-adaf-42b1-94fb-16ecf83237c8	
x86 - 86abcfbd-1ccc-4a88-b8b2-0facfde29094

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

release upgrade codes

"UpgradeCodeX64", '31ab5147-9a97-4452-8443-d9709f0516e1', "Process")	
"UpgradeCodeX86", '1d00683b-0f84-4db8-a64f-2f98ad42fe06', "Process")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Created #13036 to fix this

Comment on lines -3012 to -3025
[Environment]::SetEnvironmentVariable("PwshPath", "[$productDirectoryName]", "Process")
[Environment]::SetEnvironmentVariable("UpgradeCodeX64", '31ab5147-9a97-4452-8443-d9709f0516e1', "Process")
[Environment]::SetEnvironmentVariable("UpgradeCodeX86", '1d00683b-0f84-4db8-a64f-2f98ad42fe06', "Process")
[Environment]::SetEnvironmentVariable("IconPath", 'assets\Powershell_black.ico', "Process")
# The ApplicationProgramsMenuShortcut GUID should be changed when bumping the major version because the installation directory changes
[Environment]::SetEnvironmentVariable("ApplicationProgramsMenuShortcut", '6a69de6c-183d-4bf4-a40e-83007d6293bf', "Process")
}
else
{
[Environment]::SetEnvironmentVariable("PwshPath", "[$productDirectoryName]preview", "Process")
[Environment]::SetEnvironmentVariable("UpgradeCodeX64", '39243d76-adaf-42b1-94fb-16ecf83237c8', "Process")
[Environment]::SetEnvironmentVariable("UpgradeCodeX86", '86abcfbd-1ccc-4a88-b8b2-0facfde29094', "Process")
[Environment]::SetEnvironmentVariable("IconPath", 'assets\Powershell_av_colors.ico', "Process")
[Environment]::SetEnvironmentVariable("ApplicationProgramsMenuShortcut", 'ab727c4f-2311-474c-9ade-f2c6fd7f7322', "Process")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment to Hightlight change

}
else
{
[Environment]::SetEnvironmentVariable("PwshPath", "[$productDirectoryName]preview", "Process")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment to highlight change

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
Co-authored-by: Travis Plunk <travis.plunk@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Older/other versions of pwsh are not cleaned up during upgrade to 7.0.0 or manual uninstallation. Pinned shortcut icon disappears on upgrade

5 participants