Packaging: simplify building an installable package for Windows #5871

Merged
merged 4 commits into from Jan 17, 2018

Conversation

Projects
None yet
3 participants
Contributor

bergmeister commented Jan 11, 2018

PR Summary

This is the continuation of PR 5499 that had to be abandoned due to a fatal merge conflict and I did not want to risk accidentally reverting recent fixes.

  • Remove unnecessary/unused default for productGuid because it always gets a new Guid when being called from Start-PSPackage
  • Add defaults for required files but also add extra path validation attribute
  • Rename ProductGuid to ProductCode

PR Checklist

Note: Please mark anything not applicable to this PR NA.

- Remove unnecessary/unused default for productGuid because it always…
… gets a new Guid when being called from `Start-PSPackage`

- Add defaults for required files but also add extra path validation attribute
- Rename ProductGuid to ProductCode

@bergmeister bergmeister changed the title from Packaging: simply building an installable package for Windows to Packaging: simplify building an installable package for Windows Jan 11, 2018

@iSazonov iSazonov requested a review from SteveL-MSFT Jan 12, 2018

build.psm1
+ This only works on a Windows machine due to the usage of WiX.
+ .EXAMPLE
+ # This example shows how to produce a Debug-x64 installer for development purposes.
+ cd $RootPathOfPowerShellCheckout
@SteveL-MSFT

SteveL-MSFT Jan 12, 2018

Owner

Is it better to use the term Repo instead of Checkout?

build.psm1
@@ -2070,8 +2080,9 @@ function New-MSIPackage
[string] $ProductVersion,
# Product Guid needs to change for every version to support SxS install
@SteveL-MSFT

SteveL-MSFT Jan 12, 2018

Owner

This comment should probably be changed to: Product Code needs to be different for every version

tools/packaging/packaging.psm1
@@ -243,7 +243,7 @@ function Start-PSPackage {
AssetsPath = "$PSScriptRoot\..\..\assets"
LicenseFilePath = "$PSScriptRoot\..\..\assets\license.rtf"
# Product Guid needs to be unique for every PowerShell version to allow SxS install
@SteveL-MSFT

SteveL-MSFT Jan 12, 2018

Owner

This comment should probably be changed to: Product Code needs to be different for every version

address PR comments (missing "product guid" -> "product code" renamin…
…g in comments) and use the word repo instead of checkout in comment to be more clear.
Contributor

bergmeister commented Jan 12, 2018

@SteveL-MSFT OK, I addressed your comments.

build.psm1
@@ -2069,9 +2079,10 @@ function New-MSIPackage
[ValidateNotNullOrEmpty()]
[string] $ProductVersion,
- # Product Guid needs to change for every version to support SxS install
+ # Product Code needs to change for every version to support SxS install
@TravisEz13

TravisEz13 Jan 13, 2018

Member

Does this comment make sense any more?

The ProductCode property is a unique identifier for the particular product release

from https://msdn.microsoft.com/en-us/library/windows/desktop/aa370854(v=vs.85).aspx

tools/packaging/packaging.psm1
@@ -242,8 +242,8 @@ function Start-PSPackage {
ProductVersion = $Version
AssetsPath = "$PSScriptRoot\..\..\assets"
LicenseFilePath = "$PSScriptRoot\..\..\assets\license.rtf"
- # Product Guid needs to be unique for every PowerShell version to allow SxS install
- ProductGuid = New-Guid
+ # Product Code needs to be unique for every PowerShell version to allow SxS install
@TravisEz13

TravisEz13 Jan 13, 2018

Member

This is not correct either... but here just remove the to allow SxS install part.

@@ -5,7 +5,7 @@
<!-- TBD:Point to the actual release -->
<?define InfoURL="https://github.com/PowerShell/PowerShell" ?>
<?define ProductName = "$(env.ProductName)" ?>
- <?define ProductGuid = "$(env.ProductGuid)" ?>
+ <?define ProductCode = "$(env.ProductCode)" ?>
<!-- UpgradeCode GUID MUST REMAIN SAME THROUGHOUT ALL VERSIONS, otherwise, updates won't occur. -->
@TravisEz13

TravisEz13 Jan 13, 2018

Member

This is actually where the comment about SxS should be (in the comment about the UpgradeCode.)

I have some comments about the comments

@TravisEz13 TravisEz13 merged commit d3a775d into PowerShell:master Jan 17, 2018

4 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment