P2P references should be transitive

[Sridhar-MS]

Example: Consider the below dependency chain
ProjectA-> ProjectB->ProjectC

Here ProjectA depends on ProjectC but build fails with error

C:\Users\sridhper\.nuget\packages\microsoft.netcore.sdk\1.0.0-alpha-20160923-4\build\netstandard1.0\Microsoft.PackageDependencyResolution.targets(142,5): error : Unexpected Dependency D:\github\tryout\TestAppDependencyGraph \ProjectC\ProjectC.csproj with no version number [D:\github\tryout\TestAppDependencyGraph\ProjectA\ProjectA.csproj]

Point of failure is here

cc @eerhardt

[natidea]

The project system and sdk are in an indeterminate state making it hard for me to repro this consistently. But when I have been able to repro this, I see a project.assets.json that has target libraries like:

      "ProjectB/1.0.0": {
        "type": "project",
        "framework": ".NETStandard,Version=v1.4",
        "dependencies": {
          "ProjectC": "1.0.0",
          "Microsoft.NETCore.Sdk": "1.0.0-alpha-20160928-1",
          "NETStandard.Library": "1.6.0",
          "c:\\Projects\\TransitiveRepro\\ProjectC\\ProjectC.csproj": "1.0.0"
        }
      },
      "ProjectC/1.0.0": {
        "type": "project",
        "framework": ".NETStandard,Version=v1.4",
        "dependencies": {
          "Microsoft.NETCore.Sdk": "1.0.0-alpha-20160928-1",
          "NETStandard.Library": "1.6.0"
        }
      }

There is an entry for ProjectC/1.0.0 under the target libraries, (and also in the top-level libraries section), but there isn't one for c:\\Projects\\TransitiveRepro\\ProjectC\\ProjectC.csproj. This is what is producing the "Unexpected Dependency" error.

But sometimes I don't get any entry in either library section for the project dependencies. I'm also seeing a possibly related error:

Unable to resolve 'c:\Projects\TransitiveRepro\ProjectB\ProjectB.csproj' for '.NETStandard,Version=v1.4'.

@emgarten @joelverhagen what is the expected output of project.assets.json for Project Dependencies, especially in this transitive case?

[emgarten]

Having ProjectC and the path to ProjectC looks incorrect for the ProjectB entry. I'll take a look at how restore is handling these.

[natidea]

This error no longer occurs, presumably resolved by NuGet/NuGet.Client#924
Still see GenerateDepsFile error in #355

[davidfowl]

I just ran into this with VS2017 RC:

Severity    Code    Description Project File    Line    Suppression State
Error   MSB4018 The "GenerateDepsFile" task failed unexpectedly.
System.Exception: Could not find valid a SingleProjectInfo for project 'c:\users\dfowler\documents\visual studio 2017\Projects\WebApplication1\ClassLibrary1\ClassLibrary1.csproj'
   at Microsoft.NET.Build.Tasks.DependencyContextBuilder.GetProjectInfo(LockFileLibrary library)
   at Microsoft.NET.Build.Tasks.DependencyContextBuilder.GetLibrary(LockFileTargetLibrary export, LockFileLookup libraryLookup, IDictionary`2 dependencyLookup, Boolean runtime)
   at Microsoft.NET.Build.Tasks.DependencyContextBuilder.<>c__DisplayClass19_0.<GetLibraries>b__0(LockFileTargetLibrary export)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Enumerable.<CastIterator>d__94`1.MoveNext()
   at System.Linq.Enumerable.<ConcatIterator>d__58`1.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at Microsoft.Extensions.DependencyModel.DependencyContext..ctor(TargetInfo target, CompilationOptions compilationOptions, IEnumerable`1 compileLibraries, IEnumerable`1 runtimeLibraries, IEnumerable`1 runtimeGraph)
   at Microsoft.NET.Build.Tasks.DependencyContextBuilder.Build()
   at Microsoft.NET.Build.Tasks.GenerateDepsFile.ExecuteCore()
   at Microsoft.NET.Build.Tasks.TaskBase.Execute()
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() WebApplication1 C:\Users\dfowler\.nuget\packages\microsoft.net.sdk\1.0.0-alpha-20161104-2\build\Microsoft.NET.Sdk.targets   77  

Here's the solution https://github.com/davidfowl/VS2017TransitiveBug.

[natidea]

@davidfowl The error you are seeing is Bug #355

[nguerrera]

#355 tracks the bad failure mode seen above, but transitive P2P ref support is not supported yet. dotnet/project-system#199 tracks the project-system side. I am reopening this to track any SDK work.

[nguerrera]

Note that migration from project.json will lift the transitive closure for you with metadata on extra P2P refs so they can be removed once the feature work is done.

For now, with the preview project system/CLI/SDK bits in VS 2017 RC, you must add explicit project references as needed (ProjectA -> ProjectC in the example).

[nguerrera]

See dotnet/project-system#199 (comment). We need to raise the project reference information in lock file to ProjectReference items in ResolveProjectReferences.

[Messilimeng]

严重性 代码 说明 项目 文件 行 Suppression State
错误 MSB4018 “GenerateDepsFile”任务意外失败。
System.Exception: Could not find valid a SingleProjectInfo for project 'D:\REM_CORE\C_ERM\DBaseLibrary\DBaseLibrary.csproj'
在 Microsoft.NET.Build.Tasks.DependencyContextBuilder.GetProjectInfo(LockFileLibrary library)
在 Microsoft.NET.Build.Tasks.DependencyContextBuilder.GetLibrary(LockFileTargetLibrary export, LockFileLookup libraryLookup, IDictionary2 dependencyLookup, Boolean runtime) 在 Microsoft.NET.Build.Tasks.DependencyContextBuilder.<>c__DisplayClass19_0.<GetLibraries>b__0(LockFileTargetLibrary export) 在 System.Linq.Enumerable.WhereSelectArrayIterator2.MoveNext()
在 System.Linq.Enumerable.d__941.MoveNext() 在 System.Linq.Enumerable.<ConcatIterator>d__581.MoveNext()
在 System.Linq.Buffer1..ctor(IEnumerable1 source)
在 System.Linq.Enumerable.ToArray[TSource](IEnumerable1 source) 在 Microsoft.Extensions.DependencyModel.DependencyContext..ctor(TargetInfo target, CompilationOptions compilationOptions, IEnumerable1 compileLibraries, IEnumerable1 runtimeLibraries, IEnumerable1 runtimeGraph)
在 Microsoft.NET.Build.Tasks.DependencyContextBuilder.Build()
在 Microsoft.NET.Build.Tasks.GenerateDepsFile.ExecuteCore()
在 Microsoft.NET.Build.Tasks.TaskBase.Execute()
在 Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
在 Microsoft.Build.BackEnd.TaskBuilder.d__26.MoveNext() C_ERM C:\Users\pcdalao.nuget\packages\microsoft.net.sdk\1.0.0-alpha-20161104-2\build\Microsoft.NET.Sdk.targets 77

[jez9999]

@davidfowl I know this is a very old issue now, but I've been considering this recently and I'd like to ask: was any consideration given to how this "feature" affects separation of concerns? Transitive references allow a presentation layer, by default, to directly access classes in a DAL if you have n-tier architecture presentation -> BLL -> DAL, and there are probably a bunch of other scenarios where transitive references expose classes that should be hidden for proper separation of concerns.

Whilst it's possible to edit the .csproj file and use PrivateAssets="All", this is specified in a few blog posts and SO answers and does not appear to be the "default" way of doing things in ASP.NET Core tutorials, or in Visual Studio in general, suggesting to me that Microsoft's developers don't really think it's something that should generally be done, or that it's some kind of edge case.

Am I missing something here? Is it somehow not a problem that transitive references expose a bunch of stuff to layers that shouldn't have access to that stuff?

[davidfowl]

The thing we were trying to solve with flowing transitive deps by default was forcing consumers to reference the transitive closure manually. That is A -> B -> C and now A has to add a reference to C because it used something from B that exposed C. This is a bad default that makes things unusable. This became apparent in early .NET Core because we had something on the order of ~100 packages that needed to be manually referenced transitively.

The perfect implementation of this would use the compiler to figure out what references are exposed but not declared as transitive force you to mark those.

[jez9999]

@davidfowl So what is the recommendation for people who want the old behaviour, when A shouldn't be able to reference C? Is it just always to manually edit the .csproj file? Given that this is a somewhat common requirement, should something be added to the VS GUI to do it for you?

[davidfowl]

Yes manually edit the csproj. I don't think something should be added to the VS UI no. I'd much rather prefer a build step that told you what you needed to expose based on "public API" or something to that effect.

[dasMulli]

Manually editing the csproj file doesn't work that well.. you'd need to remove the reference from the compiler right before csc is called, most other workarounds that were tried in the past also lead to the dll file missing in the output.
This has been discussed times and times again on slack when 1.0/1.1 was adopted.
Personally I suggested writing analyzers to enforce deliberatre architecture constraints (like there NetArchTest or arch.js for tests) but I don't know if there are any good ones out there. (This could potentially have the benefit of being applicable to both assemblies and namespaces / individual types).

[davidfowl]

@dasMulli why doesn't manually editing the csproj work?

[dasMulli]

Ah looks doable now by removing the correspending ReferencePathWithRefAssemblies item in e.g. before BeforeCompile. I may have mixed it up with transitive nuget references for which there initially was no ""easy"" way to remove the reference but keep the copy-local behavior.

[jez9999]

Yes manually edit the csproj. I don't think something should be added to the VS UI no. I'd much rather prefer a build step that told you what you needed to expose based on "public API" or something to that effect.

So, to design a solution with proper separation of concerns, manually edit a file you don't normally have to touch in a way that none of our tutorials will tell you to do.

Just doesn't sit right with me. It should be more default, more obvious, more clearly recommended.

[dasMulli]

Tbh the concept is not unreasonable to ask for but it boils down to basing enforcing of architectural decisions on side effects of the build system.. (-> new issue for an opt out feature? Maybe loop in msbuild people since i don't know what the side effects of assembly / conflict resolution of various possible solutions could be)
Personally I strongly recommend using something like NetArchTest to properly express and validate architectural rules (which can be more flexible than just assembly level) or writing custom roslyn analyzers.

[jez9999]

@dasMulli Still, using something like that at unit test time doesn't stop undesirable interfaces from showing up in IntelliSense at design time. I know I might seem a bit fussy, but if I'm working in my presentation layer, VS shouldn't be offering me an autocomplete of IFooRepo when I only want my presentation layer to access that repository through IFooService, at least in an ideal world.