-
Notifications
You must be signed in to change notification settings - Fork 446
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
Include the targeting pack MSI in the SDK bundle installation #588
Include the targeting pack MSI in the SDK bundle installation #588
Conversation
eng/Versions.props
Outdated
@@ -45,6 +45,8 @@ | |||
<CLI_NETStandardLibraryNETFrameworkVersion>2.0.1-servicing-26011-01</CLI_NETStandardLibraryNETFrameworkVersion> | |||
<SharedHostVersion>$(MicrosoftNETCoreAppPackageVersion)</SharedHostVersion> | |||
<HostFxrVersion>$(MicrosoftNETCoreAppPackageVersion)</HostFxrVersion> | |||
<!-- <TargetingPackVersion>$(MicrosoftNETCoreAppPackageVersion)</TargetingPackVersion> --> | |||
<TargetingPackVersion>3.0.0-preview4-27419-5</TargetingPackVersion> |
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.
Just making a comment here so that we remember to undo this before merging.
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.
When this problem first came up in IM, I thought it was because core-sdk didn't branch off for the next preview yet. Now I see this PR is to release/3.0.1xx
rather than master
. (My changes are in core-setup master
, not release/3.0
.)
I'm familiar with this workflow for 2.1/2.2 fixes (especially infra), didn't expect to see it for brand new stuff like this. Maybe that's an arbitrary distinction, but generally I hear about proving things out in master
before putting them in release branches. E.g. a new feature like this would be added to master
, then ported to the oldest applicable release branch, then it would merge its way through to the latest one. I don't know if anyone else has this expectation, though.
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.
@dagood - we've just gone through the branching exercise for 3.0 so where things land first is in flux.
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.
We need to include asp.net's msi on this change as well. And at some point in the future, windowsdesketop.
We have included asp.net's msi in this PR.
Include the targeting pack MSI in the SDK bundle installation.
src/redist/targets/GenerateLayout.targets
download the targeting pack MSI
src/redist/targets/GenerateMSIs.targets
src/redist/targets/packaging/windows/clisdk/bundle.wxs
src/redist/targets/packaging/windows/clisdk/generatebundle.ps1
Include the targeting pack MSI in the SDK bundle installation.
@leecow
@dagood
Testing:
Successful manual installation