NuGet w/ MSBuild doesn't provide a mechanism to stop transitively propagating references in Project-Project references

[agocke]

I would expect NuGet to either respect the ProjectReference ReferenceOutputAssembly item or provide some other mechanism for cutting the reference graph.

[agocke]

/cc @jasonmalinowski

[yishaigalatzer]

Related to NuGet/NuGet.Client#43 / #1540 where we prevent these from going into pack.

[yishaigalatzer]

We would love to take a contribution for this, from someone with strong expertise in msubild. Otherwise we have no resources to deal with this until after 3.3 / vs update I

ping me if you are interested in providing this fix.

[davidfowl]

Let's talk about this befor any action is taken.

[jasonmalinowski]

@davidfowl: indeed, there are many design decisions here.

Just to give some background to @agocke's scenario for people to think about, our immediate problem is we have a console application that we use to generate syntax tree information. Our class libraries that need to run that tool have references to that console application but doesn't reference the output. This is to ensure the build is done in the right order. Right now, any dependencies we have on the generator flow through to the class library, and the to the rest of our entire solution virally.

[davkean]

@jasonmalinowski Who's reading ProjectReference? Our build task, or NuGet itself?

This thing is practically the thing that's left that's causing us to stay on Mono at the moment which is a massive cause of our failures on Linux.

Is there a workaround that we can apply without this fix?

[yishaigalatzer]

Jason is there a different way to control the build order? Not to sound offensive but this seems like a tremendous hack to work around limitations in the build system.

Does someone has the time to dig into this for 3.3? We got a few more days left and if so we should have a design meeting asap

[jaredpar]

I agree. We've solved our build order problems before by factoring out tools to a NuGet or prebuild phase (FakeSign). Couldn't we take the same approach here?

Sure it would be nice if we could have this feature and avoid this step. Short term though it seems like a prebuild phase / NuGet package could unblock us here.

[davkean]

The project reference with ReferenceOutputAssembly isn't a hack - it's how the feature was designed. ProjectReference is the way to control build order, it's just whatever is reading this isn't respecting it.

Wonder if a project that solely exists to control order might work here, taking offline for a discussion.

[jaredpar]

To clarify I wasn't trying to comment on whether ReferenceOutputAssembly was a hack or not (I don't have the context on that). Instead agreeing that it seemed like a build order limitation that we could possibly work around.

[yishaigalatzer]

What I'm seeing are two different uses of this attribute with nuget

1. To exclude the dlls from being included in the generated package
2. To exclude the references from flowing up.

It's not clear to me if this is the right attribute to indicate don't flow or not, but regardless of the correct pattern we can't get it designed and done in time for update I unless someone from outside my group can jump in and help.

So let us put the hack argument aside, and see how we can move this forward?

[jasonmalinowski]

@davkean: it's a mix of both. The command line nuget.exe restore gets it's full set of projects to process by running a build target which lives in my build targets. The VS NuGet tooling restore walks DTE.

[jaredpar]

Wanted to get this conversation moving again. This issue is blocking us from moving significant portions of our infrastructure to CoreCLR: Here is the basic problem we have where ReferenceOutputAssembly comes into play.

1. There are tools in our repo that transform XML to source code as a part of our MSBuild process.
2. These tools exist as source in our repo and hence must themselves be built as a part of building our compiler DLLs.
3. In order to schedule the build properly our compiler projects reference the tools projects (forces tools to be built first) but uses the ReferenceOutputAssembly flag to indicate we don't actually reference the output. It's just a scheduling reference.

Here is an example of this pattern:

    <ProjectReference Include="..\..\..\Tools\Source\CompilerGeneratorTools\Source\CSharpSyntaxGenerator\CSharpSyntaxGenerator.csproj">
      <Project>{288089C5-8721-458E-BE3E-78990DAB5E2D}</Project>
      <Name>CSharpSyntaxGenerator</Name>
      <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
    </ProjectReference>

It does seem odd at a glance that P2P references are the solution to this type of scheduling problem. But according to the MSBuild team this is the recommended way of scheduling such a dependency. We've explored a number of alternatives to this approach but none have worked out for us.

Ideally from our perspective, NuGet would ignore P2P references which had ReferenceOutputAssembly = false when constructing the transitive dependency graph. MSBuild doesn't treat them as references, this feature is in wide use today, so it seems counter-intuitive for NuGet to treat them as references. Also seems like it would break a lot of users converting to NuGet v3 in the same way it broke us.

If there is a cleaner way the NuGet team would prefer to implement we're willing to adjust to it. But we do need to get a solution here, or we're essentially locked out of CoreCLR for a large portion of our build. Very open to options here.

[yishaigalatzer]

We now have a way to cut the reference graph, @emgarten will provide more info here. This feature should be done at some point, but given that there is a way to do it, I don't see a good reason to cram it into 3.4 (if it can't you are still block we will reconsider moving it back to 3.4)

[jasonmalinowski]

@jaredpar: good point on ReferenceOutputAssembly being something that should probably be special. @yishaigalatzer, is that a separate fix we should take? It would require a (fairly trivial) targets change on my side, and a probably equally small change on the NuGet extension side.

[yishaigalatzer]

I'm looking at the nuget code right now, it should change in two places. One in

src\NuGet.Clients\PackageManagement.VisualStudio\ProjectSystems\BuildIntegratedProjectSystem.cs

So walking the P2P through DTE skips these hooks.

And one in the build task in nuget.exe.

So we get the same experience in the commandline.

Why do you need a change in your task?

[jasonmalinowski]

The task won't change. The target you call to get the graph closure for command line would change, unless you've already moved away from that.

[yishaigalatzer]

I believe we have our own at this point, but I'm not sure if it calls yours the targets are here

https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Clients/NuGet.CommandLine/GetProjectsReferencingProjectJsonFiles.targets
https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Clients/NuGet.CommandLine/GetProjectsReferencingProjectJsonFilesEntryPoint.targets

[jasonmalinowski]

I guess you're no longer calling mine then? I was unaware that work happened.

[jaredpar]

We now have a way to cut the reference graph, @emgarten will provide more info here.

@emgarten can you please provide this information?

[emgarten]

@jaredpar take a look at: https://github.com/NuGet/Home/wiki/%5BSpec%5D-Managing-dependency-package-assets

For your scenario you can add the non-referenced child projects to the dependencies section and specify "exclude": "all"

The child projects will still be included as dependencies and may change the package closure if the dependencies are nearer in the graph. This can be fixed by overriding them in the parent project.json file.

The full fix for this is to ignore these dependencies by checking ReferenceOutputAssembly in the targets (as @yishaigalatzer mentioned) and then remove them from the graph completely. The expectation with include/exclude is that if a reference should be 100% ignored that the user should just remove the reference (which doesn't apply to this scenario).

One workaround which could give you the complete behavior is to add a second project in the middle and then use suppressParent there to treat the actual project as a build time dependency in the project.json way. An example of that would be.

Current:
A -(ReferenceOutputAssembly=false)-> B

Workaround:
A -(ReferenceOutputAssembly=false, exclude=all)-> X -(suppressParent=all)-> B

Project A will contain a reference to X which will be empty and B will be ignored.

[jaredpar]

@emgarten thanks. Will try that out.

[agocke]

Hey @emgarten, there were a couple suggestions here, but I tried to implement them and didn't get very far. An example of the problem we're running into is here: https://github.com/agocke/roslyn/tree/TestNuGetCut

When I make the commit to add the System.Runtime reference to the CSharpSyntaxGenerator, it starts flowing up through the CSharpCodeAnalysis assembly.

I explored setting exclude: all on a reference to the CSSyntaxGenerator, but I couldn't find a way to specify the reference in our project.json -- because the generator tool is in a different directory, project.json won't let me reference the file or specify a relative path that goes to that file.

Could you take a look and see if your suggestion will work for our use case?

[emgarten]

@agocke it looks like you'll need to add the extra project with suppressParent here.

[emgarten]

NuGet/NuGet.Client@0379122