Project-system: Transitive references

[BillHiebert]

◦xproj you only need to declare top level dependencies and others are pulled in automatically
◦This also has an impact on the publish artifacts. csproj only pulls from referenced assemblies and has special logic to exclude assemblies which are not actually used in code.

[davkean]

We think the cost here is going to be in two places:

• MSBuild work to actually flow the references
• UI work in the ReferencesSubTreeProvider to show inherited dependencies (xproj shows them under the project that it was inherited from)

We think that the language service will basically get this for free if MSBuild raises them as normal <Reference/> and <ProjectReference>.

[brthor]

Any way we could schedule this for RC? We're running into issues migrating projects with nested project dependencies, including cli repo itself.

[srivatsn]

Unlikely. This is a fairly large work item that has compat implications. Can the migration tool flatten the hierarchy?

[joelverhagen]

@brthor, @srivatsn, running into the same issue. Could the whole closure of framework references be include when migrate writes the csproj? Similar to project references.

[joelverhagen]

Lacking transitivity for projects is a problem for dotnet pack3 as well.

For example, in NuGet, one of our "entry point" packages is NuGet.CommandLine.XPlat (on NuGet.org). The project.json for this package has one project reference (NuGet.Commands) and several package references. dotnet pack on the project.json results in a .nupkg with symmetrical package dependencies.

After migrating this project to .csproj and using pack3, the resulting .nupkg has these _additional_ package dependencies:

NuGet.Client
NuGet.Common
NuGet.Configuration
NuGet.ContentModel
NuGet.DependencyResolver
NuGet.DependencyResolver.Core
NuGet.Frameworks
NuGet.LibraryModel
NuGet.Packaging
NuGet.Packaging.Core
NuGet.Packaging.Core.Types
NuGet.ProjectModel
NuGet.Protocol.Core.Types
NuGet.Protocol.Core.v3
NuGet.Repositories
NuGet.RuntimeModel
NuGet.Versioning

[davkean]

I don't think that's related - this seems like a pack issue. Whether a project reference is transitive or not shouldn't affect when it ends up in a dependency in the nupkg.

[davkean]

Actually, I think it's a conversion work item we should mark the references so that they don't turn up as refs during the conversion.

[davkean]

@brthor ^

[brthor]

So is migration supposed to promote the refs or not promote them for pack?

We have this issue open for the migration work to promote refs and project refs,

https://github.com/dotnet/cli/issues/4414

Should the promoted refs just be marked PrivateAssets All? will that fix the pack scenario? @joelverhagen

[davkean]

Still promote the refs - should mark them with the metadata to opt them out of refs for pack. Work with NuGet.

[brthor]

Talked with @joelverhagen offline, PrivateAssets should do the trick

[srivatsn]

This is the current proposal for how we could implement this:

• References to Projects that produce packages can be represented in the project as <PackageReference Include="PathToProject.csproj">. Lack of the version attribute could be used to statically determine the distinction between a package referenced by name\version and a package referenced by path (i.e a Project reference to a package).
• NuGet will write to the assets file all the projects that are resolved this way.
• The targets in the SDK should read the lock file and raise the closure of P2Ps as ResolvedProjectReferences so that they can be passed to the compiler.
• Adding a Reference to a project from the UI should write a PackageReference to the project file if the target project can produce a package - this will be determined by checking if the target project has a PackageId.
• Pack needs to understand this new pattern so that it can write the proper dependencies in the project. Currently pack reads attributes on a ProjectReference and that will have to be switched to read from the PackageReference instead.
• The solution build manager will not see the transitive P2Ps but that should be fine since it builds the solution in dependency order. However solution build manager needs to be told about the first level PackageReference that's a projectref.

Tagging @kieranmo @davidfowl @davkean @yigalataz @rrelyea. Let me know if I missed anything from yesterday's discussion.

[srivatsn]

Distilling the workitems from the above proposal:

• https://github.com/dotnet/roslyn-project-system/pull/898 ProjectSystem: Nominate restore needs to pass PackageRefs with paths as ProjectReferences to NuGet so that it can write them (and their closures) to the lock file.
• https://github.com/dotnet/sdk/issues/200 SDK: Needs to raise the project refs from the lock file and ensure they are resolved by the ResolveProjectReference target.
• NuGet: Write p2ps in the lock file. This looks to be done already but probably needs to be verified.
• https://github.com/dotnet/roslyn-project-system/issues/900 ProjectSystem: Tell solution build manager about the PackageRefs which are projects.
• https://github.com/dotnet/roslyn-project-system/issues/899 ProjectSystem: Add ref to a project should write a PackageRef with path.
• NuGet: Pack needs to understand this new pattern.
• https://github.com/dotnet/cli/issues/4959 CLI : Migration should stop raising the P2Ps to the project file.

[nguerrera]

Proposal looks good. I've reopened dotnet/sdk#200 to track the SDK work. I can confirm that we already have project reference information in the lock file.

[nguerrera]

There should also be a work item for:

• CLI: Stop migrating extra P2P refs

[nguerrera]

Actually, why do we need <PackageReference Include="Foo.csproj"> instead of <ProjectReference Include="Bar.csproj">? That feels wrong and unnecessary to me. The lock file already has info about P2P refs. Can't we do everything else without requiring a change to the first-level project ref item?

[dsplaisted]

I think we should make it optional for projects references to be transitive. Today they aren't and people can use that to help avoid adding accidental dependencies. So whether it's via PackageReference or ProjectReference, I think there should be a metadata item on it to control whether you get transitive dependencies. For example, <ProjectReference Include="..\Foo\Foo.csproj" Transitive="True" />.

[nguerrera]

If it's optional, I'd prefer transitive=true. I'd prefer opt out.

[davidfowl]

The reason it needs to be on PackageReference instead of project reference is because we want to enable swapping the package with a project without changing the references in the project file.

Imagine we started with this:

<PackageReference Include="Microsoft.AspNetCore.Mvc" Version="1.0.0" />

Then I want to swap the package for a project.

<PackageReference Include="Microsoft.AspNetCore.Mvc" Version="1.0.0" Project="$(Microsoft.AspNetCore.Mvc)" />

I would import a file in all of my projects that set the $(Microsoft.AspNetCore.Mvc) property appropriately when I did a specific gesture... It could even be inferred by name or something...

Now my packages automagically flip to projects and we can step in and debug 😄 . We do this trick today with ASP.NET Core to seamlessly work across solutions (we have like ~50 repositories).

[NTaylorMullen]

If i'm understanding this fix correctly we'll also need to stop migrating transitive framework assemblies in the CLI.

[dsplaisted]

@davidfowl You could also use a target to replace a PackageReference with a ProjectReference. So they don't necessarily need to be the same tag in order to make the scenario work.

[davidfowl]

@dsplaisted no, the point is I don't want to replace anything in the project at all. So explain to me how I would do the exact same thing? Where the presence of a property in an import automagically changes all of my references from package to project in a way the IDE is ok with?

[nguerrera]

Ok, that's a good scenario. I like packagereference with project metadata for that. It is different from Sri's proposal, though. I'm pushing back against a package reference that has only a project path.

[srivatsn]

We discussed both those syntaxes and the concern with the <PackageReference Include="Id of package" Project="Path to project" /> was that the PackageId is redundant information since the source of truth for that is in the target project. If this name differs and we ignore it that causes confusion.

Referencing something by name or path is also the existing pattern for <Reference> items

[nguerrera]

I'm missing how that addresses @davidfowl's scenario. If the projects are spread across repos and I want to be able to build from packages sometimes and projects other times, the projects might not even be on my machine. So the ID is not redundant when I don't have the project in hand.

Or would he instead be expected to do <PackageReference Include="$(Microsoft.AspNetCore.Mvc) Version="$(Microsoft.AspNetCore.Mvc.Version)" /> and toggle $(Microsoft.AspNetCore.Mvc) between a csproj path and ID and toggle $(Microsoft.AspNetCore.Mvc.Version) between 1.0.0 and empty?

[dsplaisted]

@davidfowl

no, the point is I don't want to replace anything in the project at all. So explain to me how I would do the exact same thing? Where the presence of a property in an import automagically changes all of my references from package to project in a way the IDE is ok with?

You don't modify the project file itself, the import has a target that modifies the set of ProjectReference and PackageReference items. A very simple one would look something like this:

<Target Name="ReplacePackageReferences" BeforeTargets="_LoadRestoreGraphEntryPoints;PrepareProjectReferences">
    <PackageReference Remove="Microsoft.AspNetCore.Mvc" />
    <ProjectReference Include="$(Microsoft.AspNetCore.Mvc)" />
</Target>

This removes any PackageReference to Microsoft.AspNetCore.Mvc and adds a ProjectReference before the rest of the targets consume those items. For a real solution, you would probably want to write a task that would help you only add the ProjectReference if the PackageReference for Microsoft.AspNetCore.Mvc existed, and perhaps other conditions.

[srivatsn]

@nguerrera we can do what @dsplaisted suggests for PackageReference as well to switch. As for PackageReference vs ProjectReference the argument that was made in favour of PackageReference was that ProjectReferences are references to projects producing assemblies and that's an existing concept and those are not transitive. Referencing a package is a new concept and packages are transitive regardless of whether they are referenced by name\version or a path to a project that produces one

[nguerrera]

I don't think everyone who wants transitive P2P refs would think of their projects as "producing nuget packages". I can imagine lots of customers with big solutions that never package their stuff, but still want the convenience of transitive P2P.

[romerod]

@davidfowl when swapping a package with a project should the project be open in the solution or do you work in another visualstudio instance? And will the UI take care not removing the Project="$(Microsoft.AspNetCore.Mvc)" part when updating a project?

[srivatsn]

This is now done.