NuGet / Home

Repo for NuGet Client issues
Other
1.49k stars 250 forks source link

Make a public extensibility point corresponding to the _LoadRestoreGraphEntryPoints target #7288

Open binki opened 5 years ago

binki commented 5 years ago

Details about Problem

NuGet product used (NuGet.exe | VS UI | Package Manager Console | dotnet.exe): VS UI, msbuild/r

NuGet version (x.x.x.xxx): 4.8.0.5385

dotnet.exe --version (if appropriate): inappropriate

VS version (if appropriate): 15.8.3

OS version (i.e. win10 v1607 (14393.321)): win10 v1809 (17755.1)

Worked before? If so, with which NuGet version: No. There was not a need for this until msbuild/r.

Detailed repro steps so we can see the same problem

  1. Develop a system where a .csproj uses MSBuild targets to dynamically add ProjectReference items based on configuration or filename patterns by hooking to the BeforeResolveReferences target. This sort of configuration and build, without using any nuget packages, is fully supported by msbuild. There is an example of such a project on my without-msbuild-restore-support branch. This example project shows how one might desire this setup when using a DI system and optional dependencies which are configured at build time. To build such a project yourself,

    1. Create a new solution.
    2. Create an Interfaces project and define some interfaces for use by a DI system.
    3. Create a Runner project which references Interfaces and imports interface implementations using a DI system.
    4. Create an Injectable project which references Interfaces and exports interface implementations using a DI system.
    5. Define targets imported by Runner which use some arbitrary dynamic logic in a target to dynamically add a <ProjectReference/> prior to BeforeResolveReferences running. This results in Runner automatically referencing and copying Injectable to its output directory in such a way as to be discoverable by the DI system.
  2. Try to figure out how to get Runner building via msbuild/r to perform nuget package restore on these dynamic <ProjectReference/>. You will find that it is possible, but you must use an API marked private: _LoadRestoreGraphEntryPoints. This API is defined in NuGet.targets.

My problem is that I must use a private <Target/> to do this. I would like NuGet.targets to provide and document a <Target/> which is public and intended to be used for this use case, similar to how it provides the CollectPackageReferences target added by #4967. This way, I can rest assured that my code will continue to be buildable in future versions of MSBuild+NuGet as packaged with VS.

Sample Project

My sample project demonstrating the msbuild/r failure is at https://github.com/binki/NuGetRestoreDynamicProjectReference/tree/without-msbuild-restore-support . Usage instructions:

  1. git clone -b without-msbuild-restore-support https://github.com/binki/NuGetRestoreDynamicProjectReference NuGetRestoreDynamicProjectReference-without-msbuild-restore-support
  2. CD NuGetRestoreDynamicProjectReference-without-msbuild-restore-support\Runner
  3. msbuild/r (you may have to specify the full path to MSBuild.exe—I normally run "\Program Files (x86)\Microsoft Visual Studio\2017\Professional\MSBuild\15.0\Bin\amd64\MSBuild.exe"/r)

You should get a build failure due to type System.Runtime.CompilerServices.TupleElementNamesAttribute being unfound.

My solution which uses the private API is at https://github.com/binki/NuGetRestoreDynamicProjectReference . You can follow the same steps but checkout the master branch instead to get a successful build—at least in this version of NuGet. But it will probably fail someday in the future when the private _LoadRestoreGraphEntryPoints target either fails to exist or runs at a different type.

binki commented 5 years ago

Even when I use this hack, I don’t think that _LoadRestoreGraphEntryPoints is doing the right thing. Its treating everything as entry points, including the project being built. So my Runner project’s project.assets.json includes an entry for my Interfaces project but doesn’t include an entry for Injectable. I need a way to build the dependency graph in MSBuild and pass it to NuGet because it isn’t seeing my dynamically added <ProjectReference/> as sufficiently real dependencies of Runner.

binki commented 5 years ago

I have updated my demo project to use the CollectPackageReferences option so that NuGet would see the transitive dependencies from Injectable in the Runner project. That is enough to make project.assets.json look right.

There is no observable effect on my demo project because it is using simple NuGets which only introduce assemblies. I will see if it helps in my more complex project which actually requires assets from project.assets.json to be propagated.