-
Notifications
You must be signed in to change notification settings - Fork 38
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
WiX: Improvements to Windows packaging #139
Open
stevapple
wants to merge
3
commits into
swiftlang:main
Choose a base branch
from
stevapple:improve-windows
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ | |
</Directory> | ||
</Directory> | ||
|
||
<SetDirectory Id="INSTALLDIR" Value="[ProgramFiles64Folder]swift"> | ||
<SetDirectory Id="INSTALLDIR" Value="[ProgramFiles64Folder]Swift"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar |
||
NOT INSTALLDIR | ||
</SetDirectory> | ||
|
||
|
@@ -57,6 +57,9 @@ | |
<Component Id="swift_StringProcessing.dll" Guid="0e0dc47f-1e80-4ed1-8ee9-7de8139086ed"> | ||
<File Id="swift_StringProcessing.dll" Source="$(var.SDK_ROOT)\usr\bin\swift_StringProcessing.dll" Checksum="yes" /> | ||
</Component> | ||
<Component Id="swiftCxx.dll" Guid="d21a84bc-0dc3-4cf0-ad73-c5adb869e426"> | ||
<File Id="swiftCxx.dll" Source="$(var.SDK_ROOT)\usr\bin\swiftCxx.dll" Checksum="yes" /> | ||
</Component> | ||
<Component Id="swiftCore.dll" Guid="b11c37b0-b6ad-4e55-b4e7-0517ff7a161f"> | ||
<File Id="swiftCore.dll" Source="$(var.SDK_ROOT)\usr\bin\swiftCore.dll" Checksum="yes" /> | ||
</Component> | ||
|
@@ -120,6 +123,9 @@ | |
<Component Id="swift_StringProcessing.pdb" Guid="6fd2d711-4452-4aca-91c0-34e240bcc2d0"> | ||
<File Id="swift_StringProcessing.pdb" Source="$(var.SDK_ROOT)\usr\bin\swift_StringProcessing.pdb" Checksum="yes" DiskId="2" /> | ||
</Component> | ||
<Component Id="swiftCxx.pdb" Guid="36a3f6f8-c21c-4e4c-96f8-c71c122f19f5"> | ||
<File Id="swiftCxx.pdb" Source="$(var.SDK_ROOT)\usr\bin\swiftCxx.pdb" Checksum="yes" /> | ||
</Component> | ||
<Component Id="swiftCore.pdb" Guid="ba858ada-58f6-499d-a9f3-fdad7e858e15"> | ||
<File Id="swiftCore.pdb" Source="$(var.SDK_ROOT)\usr\bin\swiftCore.pdb" Checksum="yes" DiskId="2" /> | ||
</Component> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intentionally
swift
. I don't think that there is any strict suggestion/requirement/pattern that says that this should beSwift
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSDN suggests the installation path to be
[Program64FilesFolder]\ISV Name\Application Name\x64
(take x86_64 as example), and it’s clear to use title case with respect to blank spaces.In practice,
ISV Name
is sometimes ignored, arch directory is ignored if it doesn’t support multi-arch installation, version number is appended afterApplication Name
if it supports multi-version installation.This means if possible we should use
[Program64FilesFolder]\Swift\5.8-dev\runtime\x64
or[Program64FilesFolder]\Swift\5.8-dev\runtime
.cr. https://docs.microsoft.com/en-us/windows/win32/msi/single-package-authoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my machine this rule gets fully addressed except for
dotnet
andnodejs
. I don’t think we should be the rule breaker if we can avoid that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suggested layout is being followed -
swift
is the ISV,runtime-...
is the versioned product directory.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ISV Name" uses title case, and I believe you’re confusing the "Application Name" here. An application on Windows can contain several components.
For Swift, "ISV Name" should be
Swift
orSwift.org
, and "Application Name" should beSwift
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is often in title case, but I think that the lower case is acceptable as well. I intentionally left the
.org
off to make it easier to work with.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swift
looks more fit and I don’t think we have reason to refuse that🤷♂️ Other OSS projects like Git, CMake and Vim also respect title case on Windows — when in Rome, do as Romans do.Also, according to Swift editorial guidelines:
We should prefer
Swift
toswift
where title case is allowed.