Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Create targeting pack MSI installers #5114

Merged
merged 7 commits into from
Feb 14, 2019
Merged

Conversation

dagood
Copy link
Member

@dagood dagood commented Feb 8, 2019

This change has the targeting pack pkgproj create an install layout based on the nupkg's data/ dir. That's picked up by the targeting pack installer project the same way it is for hostfxr and shared framework. In general, the targeting pack installer project is a copy of existing installers with very little changed.

In official builds, build the full platform manifest on win-x86, not just win-x64. Both platforms' installers need the full manifest.


A wrinkle about building on x64 and x86: the Microsoft.NETCore.App.Ref nupkg is built once on each, so there's a chance of differences. I diffed the installed bits, and everything's identical except ordering of the lines in the platform manifest.

Ultimately it would be safest to feed one targeting pack nupkg to all the legs to package up. For now I could sort the platform manifest if order affects how the file's used at all.

For https://github.com/dotnet/core-setup/issues/4772


The three installer projects in core-setup were very copy-paste heavy, and this PR adds a fourth. I plan to refactor this to remove a lot of the duplication, and I'm waiting to do that to get the MSI validated in the SDK ASAP. I mentioned this in a mail thread but missed it in the original PR description.

List of things I plan to do before I make another installer:

  • Remove generatemsi.ps1 powershell scripts, use msbuild projects with shared targets.
    • Peek at wixproj, but likely stick with Execs for now.
  • Consider unifying targeting pack installer creation with targeting pack pkgproj. This could make the targeting pack creation entry point cleaner.
    • I also want to look into merging Microsoft.NETCore.App.Ref.pkgproj into Microsoft.NETCore.App.pkgproj, and using it to drive other packs (Runtime, AppHost) as well. Same goal: cleaner entry point.
  • Share WiX project files to avoid boilerplate.

This change has the targeting pack pkgproj create an install layout based on the nupkg's data/ dir. That's picked up by the targeting pack installer project the same way it is for hostfxr and shared framework. In general, the targeting pack installer project is a copy of existing installers with very little changed.

In official builds, build the full platform manifest on win-x86, not just win-x64. Both platforms' installers need the full manifest.
@dagood dagood self-assigned this Feb 8, 2019
@nguerrera nguerrera requested a review from johnbeisner February 8, 2019 19:00
The name "dotnet-targeting-pack" brings it in line with installer names like "dotnet-runtime" and "dotnet-host".
@johnbeisner
Copy link

@dagood
Is this MSI a stand-alone MSI or is/will it included in a bundle installation of some sort?

@nguerrera
Copy link

This will be included in the sdk installer. We would also like for it to be installable on its own so that you can have say 3.1 sdk + 3.0 runtime + 3.0 targeting pack as a possible deployment. (I.e. only one set of tools but bits needed to target old tfm with new tools.

See dotnet/designs#50

@dagood
Copy link
Member Author

dagood commented Feb 8, 2019

My understanding is that this will be similar to the runtime installers: Core-Setup builds runtime MSIs that Core-SDK chains, but Core-Setup also produces a runtime bundle installer.

However, a targeting pack bundle would only chain a single MSI (I think). The runtime bundle chains 3 MSIs. Maybe we can theme the MSI and use that as the user-facing targeting pack installer? I'm not sure, though, if there's something special the bundle does I don't know about yet, if theming somehow only works on bundle installers, or if it's as simple as wanting an EXE rather than MSI.

@dsplaisted
Copy link
Member

Do we need separate x64 and x86 MSIs, or can we bundle the same MSI into the different architecture .NET Core SDKs, and just pass in a different value for DOTNET_ROOT?

@dagood
Copy link
Member Author

dagood commented Feb 9, 2019

In the mail thread we're kind of using DOTNETHOME and DOTNET_ROOT interchangeably. Going with that, using the x64 installer as it is now, we can't install to two locations at once just using DOTNETHOME. If I do this:

.\dotnet-targeting-pack-3.0.0-preview-27408-0-win-x64.msi DOTNETHOME=C:\install-1
.\dotnet-targeting-pack-3.0.0-preview-27408-0-win-x64.msi DOTNETHOME=C:\install-2

The first installs into C:\install-1, which works. The second command gives me the change/repair/remove options. This seems like reasonable behavior--it's the same product, we're only telling it where to install.

Poking around, instance transforms seem to address this. (Doc, tutorial-ish post.) Adding this:

<Property Id="PACKINSTANCE" Value="DefaultValue" />
<InstanceTransforms Property="PACKINSTANCE">
    <Instance ProductCode="ec60557f-0793-4b6c-b181-56017b9d8d3e" ProductName="$(var.ProductName) (x64)" UpgradeCode="57f5e45d-4ab9-43b5-a0f4-4d94577c4ee7" Id="x64"/>
    <Instance ProductCode="2c7c073c-cf5d-4a90-8f53-f204029590d9" ProductName="$(var.ProductName) (x86)" UpgradeCode="c7ea6c78-442f-44fb-87ec-07b0399c5954" Id="x86"/>
</InstanceTransforms>

lets me install two instances with:

.\dotnet-targeting-pack-3.0.0-preview-27408-0-win.msi TRANSFORMS=:x86 DOTNETHOME=C:\x86 MSINEWINSTANCE=1
.\dotnet-targeting-pack-3.0.0-preview-27408-0-win.msi TRANSFORMS=:x64 DOTNETHOME=C:\x64 MSINEWINSTANCE=1

The SDK bundle would select one when chaining, and I suppose the user-facing targeting pack installer would have to let the user pick.


Stepping back, I'm not convinced there's a business need to do this. The only scenario I can think of is letting a developer skip downloading a targeting pack twice if they want to install it for x86 and x64 SDKs on the same machine. It even seems limited to manual targeting pack installs, because the x86 and x64 SDK bundles would already each have a copy of the targeting pack MSI.

Also, will it be confusing to devs to have x64/x86-specific downloads for everything else, but have an x64/x86 option in the targeting pack installer itself?

Condition="$([System.String]::new('%(TargetingPackNupkgEntries.Identity)').StartsWith('data/'))" />
</ItemGroup>

<ZipFileExtractToDirectory
Copy link
Member

Choose a reason for hiding this comment

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

Why do you pull them from the package? Just a couple targets before this you have the entire contents in items (@(File) if I remember correctly).

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a bit of reasoning in the target comment:

This extracts from the nupkg (zip) directly. An alternative would be using the PackageFile items, but there's some risk of handling TargetPath metadata differently than NuGet does. Using the nupkg directly ensures results are identical.

It is slower to decompress it, I'm just coding a little defensively here.

Copy link
Member

Choose a reason for hiding this comment

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

I don't find that defensive, it's just adding complexity. We define TargetPath and consume it, you can rely on it, of if you can't that's a bug that we fix.

Another thing to consider is inserting a layout phase between collecting assets and pack. That way the content of the pack can be defined by the layout instead of vice-versa.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's true. I did add a file in #5168 that has a non-folder TargetPath, which is the kind of thing I wanted to avoid handling--but it looks like that's actually the only one, and I don't need to do it that way. I'll get rid of this.

A layout-first approach sounds much better. I'll shoot for that.

@@ -0,0 +1,36 @@
<?xml version="1.0" encoding="utf-8"?>
<Wix xmlns="http://schemas.microsoft.com/wix/2006/wi">
Copy link
Member

Choose a reason for hiding this comment

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

All of this appears to be boilerplate yet it is still copy/pasted in every installer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah... I'm definitely planning to look into using some common WiX files as part of refactoring.

I plan to remove this script entirely, but it might as well be a little cleaner while development is ongoing.
Copy link
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

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

LGTM - +1 on the proposed enhancements to make before making additional installers.

@joeloff
Copy link
Member

joeloff commented Feb 13, 2019

Is the plan to apply MSTs to a baseline MSI? Bundles don't support this scenario AFAIK. Properties like the ProductCode/UpgradeCode are usually extracted into the metadata of the bundle. From what I've been able to find, the only way to support transforms in an MSI inside a bundle is to implement a custom bootstrapper.

@dagood
Copy link
Member Author

dagood commented Feb 13, 2019

I hadn't heard of MSTs ("MSI transform files" for my own reference) before. It looks like I created a embedded transform based on the command I ran it with. I was expecting that the bundle could use the properties I used (TRANSFORMS=:x86 DOTNETHOME=C:\x86 MSINEWINSTANCE=1) but maybe this is special territory?

I stopped short of prototyping something when I realized there might not be a point in doing it. But it does seems worth understanding this a little more to eliminate the option on a technical note rather than just lack of need.

Bundles don't support this scenario AFAIK. Properties like the ProductCode/UpgradeCode are usually extracted into the metadata of the bundle.

I see... I've got to wonder why they're extracted, but I do see that in the bundle now. Definitely raises the difficulty.

From what I've been able to find, the only way to support transforms in an MSI inside a bundle is to implement a custom bootstrapper.

I saw hints of this searching around online, but couldn't tell if they were in the same situation. I'm guessing we strongly want to avoid making a custom bootstrapper if possible?

@dagood
Copy link
Member Author

dagood commented Feb 14, 2019

Merging to get these in official builds.

Now we know more problems to work through to get a single targeting pack installer for x86/x64, but still no need. Still interested to know more, but moving on for now.

@dagood dagood merged commit 0f10c29 into dotnet:master Feb 14, 2019
@dagood dagood deleted the targeting-pack-win branch February 14, 2019 00:35
@joeloff
Copy link
Member

joeloff commented Feb 14, 2019

@dagood if we have two separate MSIs there shouldn't be a problem adding them to the bundles. The problem with transforms using only a single MSI is that the bundle would only be aware of the one MSI's metadata. So if you install it using the transform properties, the bundle won't be aware that it installed a different product and won't be able to track it.

@dagood
Copy link
Member Author

dagood commented Feb 14, 2019

Yeah, I'd say two separate MSIs is a clear default plan. Simple and in line with what we've already been doing everywhere else, just a tiny bit potentially wasteful in terms of file size.

The problem with transforms using only a single MSI is that the bundle would only be aware of the one MSI's metadata. So if you install it using the transform properties, the bundle won't be aware that it installed a different product and won't be able to track it.

Hmm, so basically the bundle must use those extracted properties, and since we'd only want to bundle one copy of the targeting pack MSI (the entire point!), it can only extract one set of properties to track with? Or maybe bundling can't extract properties at all for the transforms? (Unless we make a custom engine, which I'm still assuming we strongly don't want to do?)

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* Create targeting pack MSI

This change has the targeting pack pkgproj create an install layout based on the nupkg's data/ dir. That's picked up by the targeting pack installer project the same way it is for hostfxr and shared framework. In general, the targeting pack installer project is a copy of existing installers with very little changed.

In official builds, build the full platform manifest on win-x86, not just win-x64. Both platforms' installers need the full manifest.

* Change install dir "ref" => "packs"

* "Microsoft .NET Core Targeting Pack" branding

* Remove "netcoreapp" from MSI filename

The name "dotnet-targeting-pack" brings it in line with installer names like "dotnet-runtime" and "dotnet-host".

* Fix targeting pack powershell script: PR comments

I plan to remove this script entirely, but it might as well be a little cleaner while development is ongoing.

* Fix DependencyKey: Dotnet_CLI => Dotnet_Core

* Fix typo: GeneratedGui => GeneratedGuid


Commit migrated from dotnet/core-setup@0f10c29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants