Skip to content
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

Easy Acquisition of .NET Framework Targeting Pack #33

Merged
merged 1 commit into from
Nov 26, 2019

Conversation

terrajobst
Copy link
Member

@terrajobst terrajobst commented Mar 13, 2018

We believe this is the final plan. Modulo more feedback, we'll be accepting this.

/cc @dsplaisted @marek-safar

@terrajobst terrajobst self-assigned this Mar 13, 2018
### Non-Goals

* Supporting NuGet-based acquisition of the .NET Framework runtime
* Supporting NuGet-based acquisition from non-SDK-style projects
Copy link
Member

@tannergooding tannergooding Mar 13, 2018

Choose a reason for hiding this comment

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

I understand this is a non-goal (and why), but what would prevent this from "just working"? #ByDesign

Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I think we should allow this to work if it's not difficult. It should be possible with the proof of concept packages I created. #ByDesign

Copy link
Member

@jaredpar jaredpar Mar 14, 2018

Choose a reason for hiding this comment

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

We were able to do this with our NuGet packages in old-school project files. Don't see this as too hard. There is really just one MSBuild variable you have to toggle. #ByDesign

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

I wrote this before NuGet was doubling down on deprecating and replacing packages.config. I think fully supporting this in non-SDK style projects is much more viable now. #ByDesign

Copy link
Member

Choose a reason for hiding this comment

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

I'd really like to see this supported in old style .csproj projects that are using <PackageReference>...that's where many of us in ASP.NET will still be for a while yet.

Copy link
Member

Choose a reason for hiding this comment

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

@NickCraver The current plan is that it will be supported, you'll just have to explicitly add the PackageReference

Copy link
Member

Choose a reason for hiding this comment

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

Oh that changes things a bunch and adds other issues.

conditional dependencies on each of the version-specific reference assembly
packages.
* The metapackage will automatically be referenced by the .NET SDK when required
(ie on non-Windows OS's). On Windows, we will probably make the .NET SDK
Copy link
Member

@tannergooding tannergooding Mar 13, 2018

Choose a reason for hiding this comment

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

Any particular reason why the difference on Windows? It would seem simpler overall to just make it consistent... #Resolved

Copy link
Member

@akoeplinger akoeplinger Mar 13, 2018

Choose a reason for hiding this comment

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

Agreed, not needing to install targeting pack .msi's is important for Windows based CI systems too.
#Resolved

Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I'd be happy either way, but would lean towards requiring opt-in for this on Windows, to stay consistent with existing behavior on Windows. #Resolved

Copy link
Member

@tannergooding tannergooding Mar 14, 2018

Choose a reason for hiding this comment

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

@daplaisted, what behavior difference would there be? The assemblies on the machine and those in the NuGet package should be identical. #Resolved

Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

@tannergooding If the reference assemblies are already installed on the machine, then switching to packages means an additional download and additional disk space used. The packages are about 20MB for each version of the framework.

I don't have a strong opinion here though. #Resolved

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

I agree with @dsplaisted that we should prefer the TP if its there, mostly for perf but also for compat reasons (who knows, people do all sorts of crazy things in their builds and all we can do is break them). For cases where the TP isn't installed (either non-Windows or missing TP) I like the idea of automatically using the NuGet approach. Requiring opt-in to make something build that wouldn't have build before seems silly to me.

I'll update the design section. #Resolved

Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

It would be great if we could use the machine-wide targeting pack if it's installed, and if not fall back to using the NuGet packages. Unfortunately I'm not sure whether that's technically feasible. #Resolved

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

Unfortunately I'm not sure whether that's technically feasible.

Oh, because you have to go through RAR to resolve it to the disk location? Hmm.... that would be sad. #Resolved

Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

It doesn't go through RAR, but it does go through some other tasks to locate the reference assemblies, which we may not be able to run in order to determine whether to add the implicit package reference. #Resolved

Copy link
Member Author

@terrajobst terrajobst Mar 15, 2018

Choose a reason for hiding this comment

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

Looks like we changed our mind: #33 (comment) #ByDesign

intellisense files in the `build\.NETFramework\v4.6.2` folder, as well as have
the `Facades`, `PermissionSets`, and `RedistList` folders with corresponding
files under that folder.
* The version number of each package should start at 1.0.0. When a new version
Copy link
Member

@tannergooding tannergooding Mar 13, 2018

Choose a reason for hiding this comment

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

Was any thought given to just release the net462 reference assemblies in a Microsoft.NETFramework.ReferenceAssemblies, v4.6.2 package, and to have a separate Microsoft.NETFramework.ReferenceAssemblies.All metapackage?
#ByDesign

Copy link
Member

@tannergooding tannergooding Mar 13, 2018

Choose a reason for hiding this comment

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

I think this would result in (overall) fewer names, easier to find/reason about, etc #ByDesign

Copy link
Contributor

@nguerrera nguerrera Mar 13, 2018

Choose a reason for hiding this comment

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

There have been (rare) occasions where reference assemblies had a bug that was serviced. I think that would be blocked by TFM version == package version. #ByDesign

Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

Yes, using the target framework version as the package version would prevent us from servicing the package if we needed to.

Also, we wouldn't want someone to manually reference the Microsoft.NETFramework.ReferenceAssemblies package, and end up with the wrong version of the package for the version of the framework they're targeting (which could happen if they retarget their project, or update to a newer package version via the NuGet UI, for example). #ByDesign

Copy link

@jskeet jskeet Mar 14, 2018

Choose a reason for hiding this comment

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

Yes, each package feels like it's a separate "product" to me. The package targeting .NET 4.5.1 can't be used if you're trying to target .NET 4.5, for example - it's not just "a patched version of the same thing". It's a package you'd use for a different purpose.

So +1 to "start at 1.0.0". #ByDesign

Copy link
Member

@tannergooding tannergooding Mar 14, 2018

Choose a reason for hiding this comment

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

The same could be said for .NETStandard Library and any future servicing fixes that come out for it...

I would assume that whatever plan is required there would also apply here.

I don't think that these are separate products at all, they are simply the reference assemblies for different versions of the .NET Framework #ByDesign

Copy link
Contributor

@nguerrera nguerrera Mar 14, 2018

Choose a reason for hiding this comment

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

Netstandard.Library package version is not 1:1 with .NETStandard TFM version either. #ByDesign

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

Netstandard.Library package version is not 1:1 with .NETStandard TFM version either.

Yep, and the fact that we tried to "look" close to the TFM version has caused more problem than it solved. 1.0.0 is the way to go. #ByDesign

@eerhardt
Copy link
Member

eerhardt commented Mar 13, 2018

@dsplaisted - when we do this work, make sure the <PreserveCompilationContext> and ASP.NET Razor compilation still works correctly even without the targeting pack installed in %ProgramFiles%. There is some code today that looks for the targeting pack installation location in both the SDK and in Microsoft.Extensions.DependencyModel.

https://github.com/dotnet/sdk/blob/c1a276fb456606e2b19d4646f8239d2546790d42/src/Tasks/Microsoft.NET.Build.Tasks/FrameworkReferenceResolver.cs#L14
and
https://github.com/dotnet/core-setup/blob/3c802455b2d8d01470e064e7557bec6faccf496e/src/managed/Microsoft.Extensions.DependencyModel/Resolution/ReferenceAssemblyPathResolver.cs#L103 #Resolved


* Supporting NuGet-based acquisition of the .NET Framework runtime
* Supporting NuGet-based acquisition from non-SDK-style projects
* Support pre-.NET Framework 4.5 targeting packs
Copy link
Contributor

@jnm2 jnm2 Mar 13, 2018

Choose a reason for hiding this comment

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

😭 Libraries like NUnit could really use this. This is what is preventing the dotnet CLI from being able to build net40 and earlier. What's the cost of two or three more targeting packs? #ByDesign

Copy link

@ghost ghost Mar 13, 2018

Choose a reason for hiding this comment

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

Please provide all PCL and .NET Framework 2+ packages. They are already shipped out and not going to get any updates anyway, so hopefully no additional cost than just package once and push to nuget.. #ByDesign

Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I think we should provide the .NET 4.0 targeting pack, and possibly other 4.0.x versions in reference assemblies. We already have targeting packs for these, so I don't think there are any obstacles to this.

For .NET 3.5.1 and lower, we didn't ship targeting packs in the same way. PCL reference assemblies are also a bit different. I would propose that initially we not support them, in order to get basic .NET Framework support out more quickly. #ByDesign

Copy link
Contributor

@jnm2 jnm2 Mar 14, 2018

Choose a reason for hiding this comment

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

@dsplaisted If net35 is merely deprioritized without closing the door on it, I can cheer up. 😃 (net20 would also make some folks happier, and by extension, me.) #ByDesign

Copy link
Member

@tannergooding tannergooding Mar 14, 2018

Choose a reason for hiding this comment

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

This would also fix cases, like in Roslyn, where custom packages are created #ByDesign

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

I have talked to @jaredpar and it seems you guys don't need pre-.NET Framework 4.5 support. Is that not true? #ByDesign

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

@jnm2

What's the cost of two or three more targeting packs?

It's mostly consistency in our offerings; we don't want to breath more life into frameworks that are that old. That being said, we can always ship more targeting packs in the future if that becomes necessary. Based on adoption I don't see enough evidence that this is necessary though. #ByDesign

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

@dsplaisted

I think we should provide the .NET 4.0 targeting pack, and possibly other 4.0.x versions in reference assemblies. We already have targeting packs for these, so I don't think there are any obstacles to this.

That's a fair point. How about this: I leave the wording in the spec as-is; if we can easily build the 4.0 set, great, we'll ship it. If not or we run out of time, we don't. Usage wise, I don't see compelling reasons to spend a great deal of time on 4.0. And 4.5 just makes sense from a .NET Standard support matrix stand point. #ByDesign

Copy link
Contributor

@jnm2 jnm2 Mar 15, 2018

Choose a reason for hiding this comment

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

@terrajobst

It's mostly consistency in our offerings; we don't want to breath more life into frameworks that are that old.

So long as VSTest ships a net35-compiled test execution engine, that puts pressure on NUnit to ship a net35 assembly so that we aren't the ones preventing people from testing on CLR v2 if they need to. A net35 targeting pack would be preferable to what we're doing now, using Mono's edition of net35. #WontFix

Copy link
Member Author

@terrajobst terrajobst Mar 15, 2018

Choose a reason for hiding this comment

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

Not sure I buy that: xUnit only supports .NET Standard 1.1. And the number of 3.5 installs is quite low. I think I'm fine with 4.0 but I'm definitely opposed to go lower, especially because the whole 2.0, 3.0, 3.5 TPs are much more convoluted. #Resolved

- For the version-specific packages:
`Microsoft.NETFramework.ReferencesAssemblies.net462` (where net462 is
replaced with the corresponding NuGet short framework Identifier)
* The NuGet packages will include a `.targets` file that sets the
Copy link
Member

@weshaggard weshaggard Mar 13, 2018

Choose a reason for hiding this comment

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

I think we may need to set FrameworkPathOverride as well to avoid other weird issues like the explicit mscorlib reference. #Resolved


Since .NET Framework 2.0, the .NET platform supported targeting different
versions of the platform regardless of the .NET Framework version that is
installed on the developer's machine. This allows developers to target a lower
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I think it wasn't until .NET Framework 4.0 that we got this for real. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm I thought 2.0 added TPs but it probably doesn't matter. Will remove the version number :-)


In reply to: 174328985 [](ancestors = 174328985)

### Non-Goals

* Supporting NuGet-based acquisition of the .NET Framework runtime
* Supporting NuGet-based acquisition from non-SDK-style projects
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I think we should allow this to work if it's not difficult. It should be possible with the proof of concept packages I created. #ByDesign


* Supporting NuGet-based acquisition of the .NET Framework runtime
* Supporting NuGet-based acquisition from non-SDK-style projects
* Support pre-.NET Framework 4.5 targeting packs
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I think we should provide the .NET 4.0 targeting pack, and possibly other 4.0.x versions in reference assemblies. We already have targeting packs for these, so I don't think there are any obstacles to this.

For .NET 3.5.1 and lower, we didn't ship targeting packs in the same way. PCL reference assemblies are also a bit different. I would propose that initially we not support them, in order to get basic .NET Framework support out more quickly. #ByDesign

* Supporting NuGet-based acquisition of the .NET Framework runtime
* Supporting NuGet-based acquisition from non-SDK-style projects
* Support pre-.NET Framework 4.5 targeting packs
* Support non-.NET Framework targeting packs (Xamarin, Mono, Unity)
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

Since Mono uses the same target framework identifier as .NET Framework, I think this proposal effectively does support Mono. #Resolved

Copy link
Member

@akoeplinger akoeplinger Mar 14, 2018

Choose a reason for hiding this comment

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

I suppose what was meant here is that this won't include targeting packs for the Xamarin/Mono/Unity specific APIs like Xamarin.iOS.dll, Mono.Posix.dll, etc. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that's stale text.


In reply to: 174329831 [](ancestors = 174329831)

Copy link
Member Author

@terrajobst terrajobst Mar 14, 2018

Choose a reason for hiding this comment

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

Not sure my mind was framed that well but let's go with this ;-) I'll change the wording according.


In reply to: 174331801 [](ancestors = 174331801)

- We have an identity package
- Each version has a different identity and is looped in by TFM
- Ensures only incremental download
- Open question is whether we need the aggregate package at all or just
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

The aggregate package helps enable the scenario when you don't use Microsoft.NET.Sdk, since there's just one package you reference. From the SDK, we may not end up referencing the aggregate package, as then we could just reference version 1.0.0 by default of the version-specific package unless we had to service one of the packages. #Resolved

Copy link
Member Author

@terrajobst terrajobst Mar 15, 2018

Choose a reason for hiding this comment

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

Got it. Will reword. #Resolved

conditional dependencies on each of the version-specific reference assembly
packages.
* The metapackage will automatically be referenced by the .NET SDK when required
(ie on non-Windows OS's). On Windows, we will probably make the .NET SDK
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I'd be happy either way, but would lean towards requiring opt-in for this on Windows, to stay consistent with existing behavior on Windows. #Resolved

`TargetFrameworkRootPath` if the `TargetFrameworkIdentifier` and
`TargetFrameworkVersion` properties match the reference assemblies that the
package provides. It will also only set the property if the
`UseReferenceAssembliesFromPackage` property is set to true.
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

I'm having second thoughts on requiring opt-in via a UseReferenceAssembliesFromPackage property if you've already referenced the package. I don't think it's necessary and just complicates using the packages from non-SDK projects. SDK projects would have a different property to control whether the packages are referenced at all. #Resolved

Copy link
Member

@weshaggard weshaggard Mar 14, 2018

Choose a reason for hiding this comment

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

If someone explicitly adds the package reference I agreed we don't need this opt-in. However if the package reference is implicitly added by the SDK then I do think we need some opt-in/out but that could be around the implicit package reference in the SDK though and not in the targets. #Resolved

Copy link
Member Author

@terrajobst terrajobst Mar 15, 2018

Choose a reason for hiding this comment

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

@dsplaisted, @nguerrera and myself talked yesterday:

  • We concluded that the opt-in for non-SDK-style project is the manual install of the package. If the package is present, it will prefer the reference assemblies from it even if a TP is installed.
  • For SDK-style projects we concluded that it's OK to default to use the package and use the existing opt-out mechanism via "use implicit package references" (not sure what the property is, but we have one)

I'll update the proposal to capture this later. #Resolved

Copy link
Member

@dsplaisted dsplaisted Mar 15, 2018

Choose a reason for hiding this comment

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

The property to disable implicit package references in the SDK is called DisableImplicitFrameworkReferences. #Resolved

intellisense files in the `build\.NETFramework\v4.6.2` folder, as well as have
the `Facades`, `PermissionSets`, and `RedistList` folders with corresponding
files under that folder.
* The version number of each package should start at 1.0.0. When a new version
Copy link
Member

@dsplaisted dsplaisted Mar 14, 2018

Choose a reason for hiding this comment

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

Yes, using the target framework version as the package version would prevent us from servicing the package if we needed to.

Also, we wouldn't want someone to manually reference the Microsoft.NETFramework.ReferenceAssemblies package, and end up with the wrong version of the package for the version of the framework they're targeting (which could happen if they retarget their project, or update to a newer package version via the NuGet UI, for example). #ByDesign

of the .NET Framework is released, a corresponding reference assembly package
(versioned at 1.0.0) should be released, and a new metapackage that includes
the additional dependency for the newly supported version should be released.
The new version of the metapackage should have it's minor version incremented.
Copy link

@dasMulli dasMulli Mar 14, 2018

Choose a reason for hiding this comment

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

Ideally, the metapackage would also have a targets file to check for the maximum supported version and report an error if a net* version is targeted greater than the currently supported version or else the consuming project might end up with reference assemblies for the lower version if resolution falls back to the latest dependency group of the metapackage. #Resolved

Copy link
Member Author

@terrajobst terrajobst Mar 15, 2018

Choose a reason for hiding this comment

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

Hey @dsplaisted, is this needed or is this handled by the way the targets work? #Resolved

Copy link
Member

@dsplaisted dsplaisted Mar 15, 2018

Choose a reason for hiding this comment

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

In my proof of concept, this is basically handled handled by conditioning setting the reference assembly path on the exact target framework version. The package for 4.7.1, for example, has the following in the .targets file:

  <PropertyGroup Condition=" ('$(UseReferenceAssembliesFromPackage)' == 'true') And ('$(TargetFrameworkIdentifier)' == '.NETFramework') And ('$(TargetFrameworkVersion)' == 'v4.7.1') ">
    <TargetFrameworkRootPath>$(MSBuildThisFileDirectory)</TargetFrameworkRootPath>
    <_NeedToRemoveExplicitReference>true</_NeedToRemoveExplicitReference>
  </PropertyGroup>

So you wouldn't get the wrong reference assemblies, you'd just get the same error you get today if reference assemblies aren't available. #Resolved

Copy link
Contributor

@nguerrera nguerrera Mar 15, 2018

Choose a reason for hiding this comment

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

We should improve on the error that you get today. It warns, "hey, you'll get refs from the GAC, which is totally wrong and stupid, but YOLO the build might succeed." This is especially silly in SDK projects where the GAC is excluded from search paths. #Resolved

Copy link
Contributor

@nguerrera nguerrera Mar 15, 2018

Choose a reason for hiding this comment

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

(and where there often is no GAC (.NET Core msbuild)) #Resolved

intellisense files in the `build\.NETFramework\v4.6.2` folder, as well as have
the `Facades`, `PermissionSets`, and `RedistList` folders with corresponding
files under that folder.
* The version number of each package should start at 1.0.0. When a new version
Copy link

@jskeet jskeet Mar 14, 2018

Choose a reason for hiding this comment

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

Yes, each package feels like it's a separate "product" to me. The package targeting .NET 4.5.1 can't be used if you're trying to target .NET 4.5, for example - it's not just "a patched version of the same thing". It's a package you'd use for a different purpose.

So +1 to "start at 1.0.0". #ByDesign

## Design

* Daniel has created a [prototype](https://github.com/dsplaisted/ReferenceAssemblyPackages).
- We have an identity package
Copy link

@jskeet jskeet Mar 14, 2018

Choose a reason for hiding this comment

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

It's not clear to me what this means. It may not be important for it to be clear - that depends on the audience and potential longevity of this doc.

There are a few other bits that I could suggest clarifications/fixes to, but they don't have a technical impact. Let me know if you want nit-picking on the wording, basically. Otherwise, I'll assume that as a proposal doc, this is effectively transient. #Resolved

Copy link
Member Author

@terrajobst terrajobst Mar 15, 2018

Choose a reason for hiding this comment

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

"Identity package" is a term we've used internally. The basic idea is that you have a single package ("the identity") that is empty and only depends on other packages split by TFM version. It's basically a very specific usage of a meta package that aggregates a bunch of packages under a common name. I've reworded the entire section to make it easier to understand.

#Resolved

### Goals

* Doesn't require running an installer
* Doesn't require Windows
Copy link
Member

@jaredpar jaredpar Mar 14, 2018

Choose a reason for hiding this comment

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

Consider present as a positive: Works cross platform #Resolved

### Non-Goals

* Supporting NuGet-based acquisition of the .NET Framework runtime
* Supporting NuGet-based acquisition from non-SDK-style projects
Copy link
Member

@jaredpar jaredpar Mar 14, 2018

Choose a reason for hiding this comment

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

We were able to do this with our NuGet packages in old-school project files. Don't see this as too hard. There is really just one MSBuild variable you have to toggle. #ByDesign

@terrajobst
Copy link
Member Author

@eerhardt

when we do this work, make sure the <PreserveCompilationContext> and ASP.NET Razor compilation still works correctly even without the targeting pack installed in %ProgramFiles%. There is some code today that looks for the targeting pack installation location in both the SDK and in Microsoft.Extensions.DependencyModel.

That's a great point. I'll call it out in the requirements.

@jnm2
Copy link
Contributor

jnm2 commented Mar 15, 2018

@terrajobst What does the added #ByDesign on most of our comments mean?

don't change the performance characteristic of exiting solutions but also
allows working around some issue due to build extensions by installing the
targeting pack.
* Supports `<PreserveCompilationContext>` so thart ASP.NET Razor compilation
Copy link

@nietras nietras Mar 15, 2018

Choose a reason for hiding this comment

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

nit: thart => that #Resolved

don't change the performance characteristic of exiting solutions but also
allows working around some issue due to build extensions by installing the
targeting pack.
* Supports `<PreserveCompilationContext>` so that ASP.NET Razor compilation
Copy link
Member

@dsplaisted dsplaisted Mar 16, 2018

Choose a reason for hiding this comment

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

I've filed dotnet/sdk#2054 to track fixing PreserveCompilationContext. I don't think we should block the rest of the work on this issue though. #Resolved

Copy link
Member

@eerhardt eerhardt Mar 16, 2018

Choose a reason for hiding this comment

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

I'm not sure what you mean by "block the rest of the work", but we can't make using the NuGet targeting pack the default in SDK projects without this issue being addressed. Or else you will break every ASP.NET MVC/Razor Page app running on the desktop .NET Framework. #Resolved

Copy link
Contributor

@nguerrera nguerrera Mar 16, 2018

Choose a reason for hiding this comment

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

As I understand it, the idea for the first turn of the crank is that you can build without the program files TP but razor will still need it to be there to run. It needn't break anything that already works. You can build against nuget and dynamically compile against the same content in program files at runtime.

Step 2 would be to write out the actual nuget location that build used, and plumb that through for runtime compilation consumption. #Resolved

Copy link
Member

@dsplaisted dsplaisted Mar 16, 2018

Choose a reason for hiding this comment

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

The current plan is to only reference the NuGet packages if the targeting pack isn't found in program files. So if you have the reference assemblies installed, everything would continue to work.

I think everything would still work even if we did use the reference assembly packages for compilation as long as you have the reference assemblies installed. Different parts of the build would be getting the reference assemblies from different locations, but they should be the same files. #Resolved

Copy link
Contributor

@nguerrera nguerrera Mar 16, 2018

Choose a reason for hiding this comment

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

(I tried to say the same thing, but Daniel worded it better. :)) #Resolved

Copy link
Member

@eerhardt eerhardt Mar 16, 2018

Choose a reason for hiding this comment

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

Sounds good. I just wanted to call out that this area is a bug factory, and often gets overlooked. #Resolved

@terrajobst terrajobst changed the title WIP: Inital draft of "Easy Acquisition of .NET Framework Targeting Pack" Easy Acquisition of .NET Framework Targeting Pack Mar 29, 2018
@davkean
Copy link
Member

davkean commented Mar 29, 2018

I think you are missing two things from this doc:

Visual Studio:

  • Global DTAR (IVsFrameworkMultiTargeting) and project DTAR (IVsDesignTimeAssemblyResolution) will fail to return the correct results, breaking any feature that calls into them, Application, WinForms and WPF designers are examples of this. The TFM prompt is another example when the TFM isn't installed.
  • Reference Manager dialog, what does it show I don't think it uses project context to locate the framework? Does it say that the entire framework is already referenced?
  • How do you rationalize between explicitly referencing a dll when using targeting pack , and presumably auto-referencing via the package? How do you switch "between" these modes if this is automatic - they both seem very different.
  • How do you warn/prevent legacy projects from using this package? This package is going to cause a few problems with it (if you mix explicit refs with auto-refs, they become unresolved, which cause UI and performance issues) and I'd rather see this avoided vs bugs being filed against VS.
  • ClickOnce - ClickOnce has different behavior for framework assemblies vs user reference assemblies, I don't think it uses project context
  • Windows Installer - Windows installer extensions uses the RedistList to determine whether a redist gets automatically included in the bootstrapper or whether the binary gets included. I don't think it uses project context

Performance:

  • There is a ton-load of performance shortcuts that occurs when using the targeting pack, for example, we read RedistList.xml instead of opening each individual dll. I'd rather see this addressed before we ship this feature, rather than after like what occured with .NET Core. How we make sure we don't start regressing build performance when this is opt'd in?
  • There's a performance shortcut for locating framework directory in .NETFramework.props

@davkean
Copy link
Member

davkean commented Mar 29, 2018

Oh seems like I'm missing context not called out in the doc, but you mentioned on twitter - you are just changing where TFM folder is found, and not using NuGet mechanisms to auto-reference the dlls? I still have some different concerns, but want to understand the implementation before I add them.

@clairernovotny
Copy link
Member

Does this cover all .NET Framework targets? I.e., what about WPF and WCF? Does this mean that WPF can be built on a Mac via Mono? This spec seems to imply that.

@niemyjski
Copy link

Amazing! Just so I understand this correctly as stated in the goals, I should be able to pull in a targeting pack nuget package for full framework and build cross platform, kinda like how VS Extensibility packages are on nuget but I don't have to have old sdks? From reading this sounds like I can finally build our full framework packages cross platform like a WPF library with a WPF Pack (It won't run xplat but I could build it). But the text is a bit confusing because the goals state xplat but then you have

It's not cross-platform friendly. The .NET Framework Developer Packs are
only available for Windows which makes the targeting packs indirectly Windows-
only too.

@akoeplinger
Copy link
Member

@onovotny @niemyjski if you only consume APIs from the WPF assemblies/namespaces in a library then I'd expect that to work with this spec, but you won't automatically get e.g. the XAML markup compiler and other tools.

@clairernovotny
Copy link
Member

@akoeplinger I assumed as much, but I think that needs to be explicitly called out or potentially detected and with an error generated at build time.

@thomaslevesque
Copy link
Member

@thomaslevesque No, that won't be part of the 3.0 SDK. We still plan to do it eventually, but will wait until we have non-preview packages available for the reference assemblies.

Thanks!

@markusschaber
Copy link

markusschaber commented Sep 11, 2019

@dsplaisted Is there any ETA for the reference assembly packages?

If not, what's the reason it's so hard to estimate? I mean, the APIs for existing versions are quite cast in stone... :-)

@Konard
Copy link

Konard commented Sep 11, 2019

@dsplaisted XUnit seems to fail with Microsoft.NETFramework.ReferenceAssemblies at https://travis-ci.com/linksplatform/Reflection/builds/127016434. The workaround is to use dotnet test -f netcoreapp2.1.

@dsplaisted
Copy link
Member

@markusschaber The reference assembly packages are available on NuGet: https://www.nuget.org/packages/Microsoft.NETFramework.ReferenceAssemblies/1.0.0-preview.2

@Konard The reference assemblies allow you to build projects that target .NET Framework. They don't provide a runtime to actually run the project. So I don't think dotnet test is supposed to work for .NET Framework when running on Linux.

@Konard
Copy link

Konard commented Sep 11, 2019

@dsplaisted Microsoft.NETFramework.ReferenceAssemblies also seems to work with dotnet pack, so this solves my issue of not having a Windows development environment and works fine with MonoDevelop. Thank you.

@markusschaber
Copy link

@dsplaisted It seems my question was not formulated the way I intended. :-(
I actually wanted to ask for an ETA for the non-preview packages for the reference assemblies.

@WeihanLi
Copy link
Contributor

will it be part of dot net core sdk 3.1?

@jnm2
Copy link
Contributor

jnm2 commented Oct 27, 2019

.NET Framework 2.0 is available, but .NET Framework 3.5 is missing in https://www.nuget.org/packages/Microsoft.NETFramework.ReferenceAssemblies/1.0.0-preview.2. This means that if one of my target frameworks in my csproj is net20, dotnet build can build it, but if one of my target frameworks is net35, I get "The reference assemblies for .NETFramework,Version=v3.5 were not found."

This doesn't make sense to me. .NET Framework 2.0 is not even supported by Microsoft, and .NET Framework 3.5 SP1 is going to be supported at least as long as Windows 10. https://support.microsoft.com/en-us/help/17455/lifecycle-faq-net-framework

Why provide net20 assemblies without providing the far more useful net35 reference assemblies?

@dsplaisted
Copy link
Member

@jnm2 We have this issue tracking support for .NET 3.5: dotnet/installer#2022

The next release of the reference assembly packages should address this.

Thanks!

@dsplaisted
Copy link
Member

@jnm2 It turned out I was mistaken. We are not planning to support targeting .NET 3.5 in the reference assembly NuGet packages. Sorry about this.

@AArnott
Copy link

AArnott commented Nov 20, 2019

@dsplaisted Why not? And why is dotnet/installer#2022 still active then?

@jnm2
Copy link
Contributor

jnm2 commented Nov 20, 2019

@dsplaisted Is it because of the C++/CLI System.Data dll? The net35 pack is the one that matters for those targeting .NET Framework versions that are currently supported. Can a community member fill the gap?

@terrajobst
Copy link
Member Author

terrajobst commented Nov 22, 2019

@AArnott

@dsplaisted Why not? And why is dotnet/core-sdk#2022 still active then?

Because our general policy is that we don't add new features to support older versions of the .NET platform. The reason we're supporting .NET Framework 2.0 is rooted in internal requirements driven by the compiler team. Otherwise we would have cut off at 4.5, which is what the spec originally suggested.

@jnm2
Copy link
Contributor

jnm2 commented Nov 25, 2019

Because net35 is still in support and net40 and net20 are not, and this targeting pack includes net20 and net40 but not net35, and because I'm familiar with the pain of not having the choice to stop supporting net35, I created a community package to go along with these:

https://www.nuget.org/packages/jnm2.ReferenceAssemblies.net35

There doesn't seem to be any technical limitation distinguishing this package from the Microsoft packages, and one of my integration tests shows that this works fine:

Example.csproj
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>net35;net40;netstandard2.0;netcoreapp3.0</TargetFrameworks>
  </PropertyGroup>
  
  <ItemGroup>
    <PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" />
    <PackageReference Include="jnm2.ReferenceAssemblies.net35" Version="1.0.0" />
  </ItemGroup>

</Project>
ClassUsingSystemLinq.cs
using System.Linq;

public class ClassUsingSystemLinq
{
    public ClassUsingSystemLinq()
    {
        Enumerable.Empty<int>();
    }
}

This solves the problem of having to use VSWhere to find MSBuild when compiling for net35 on Windows and the problem of having to use Mono's MSBuild when compiling for net35 on macOS and Linux. dotnet build all the way. Here's to less infrastructure fighting. Happy coding!

@slang25
Copy link

slang25 commented Nov 25, 2019

You are a true hero @jnm2 🎉

@terrajobst
Copy link
Member Author

@dsplaisted any objections to me merging this? Seems to be done.

@dsplaisted
Copy link
Member

@terrajobst Go ahead and merge

@terrajobst terrajobst merged commit 72ea79f into dotnet:master Nov 26, 2019
@terrajobst terrajobst deleted the targeting-packs branch November 26, 2019 00:25
@jnm2
Copy link
Contributor

jnm2 commented Dec 5, 2019

ℹ 1.0.0 of https://www.nuget.org/packages/jnm2.ReferenceAssemblies.net35 is up, mirroring the changes made in Microsoft.NETFramework.ReferenceAssemblies 1.0.0.

@0xced
Copy link

0xced commented Dec 5, 2019

@thomaslevesque No, that won't be part of the 3.0 SDK. We still plan to do it eventually, but will wait until we have non-preview packages available for the reference assemblies.

Now that the (non-preview) Microsoft.NETFramework.ReferenceAssemblies 1.0.0 package is available, are there plans to integrate this feature directly into the .NET Core SDK?

Is there any chance that it will make its way into a 3.x update or should we wait for .NET 5? Also, is there already a branch with a prototype of direct SDK integration on the dotnet/sdk repository?

@dsplaisted
Copy link
Member

@0xced I've filed dotnet/sdk#4009 to automatically reference these packages when necessary. We haven't yet discussed whether to put it in 3.1 or 5.0.

@yaakov-h
Copy link
Member

yaakov-h commented Dec 5, 2019

@dsplaisted well 3.1 already shipped, so I guess it's 5.0 then? Or would you ship it to sdk 3.1.101 or similar?

@dsplaisted
Copy link
Member

@yaakov-h It could conceivably go in something like 3.1.200 or 3.1.300.

@0xced
Copy link

0xced commented Apr 15, 2021

For reference: this feature was never released in the .NET Core 3.x SDK but was released in the .NET 5 SDK.

0xced added a commit to 0xced/DbContextValidation that referenced this pull request Jun 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.