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

transitive ProjectReferences not considered during compilation #49731

Closed
ltcmelo opened this issue Dec 2, 2020 · 5 comments
Closed

transitive ProjectReferences not considered during compilation #49731

ltcmelo opened this issue Dec 2, 2020 · 5 comments
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@ltcmelo
Copy link

ltcmelo commented Dec 2, 2020

NOTE: Related to #3249.

Version Used: 3.8.0

Steps to Reproduce:

Consider libA.csproj, libB.csproj, and libC.csproj; all within proj.sln.

# libB.csproj file
    …
    <ProjectReference Include="..\libA\libA.csproj" />
    …
# libC.csproj file
    …
    <ProjectReference Include="..\libB\libB.csproj" />
    …

Project libC doesn't directly reference libA, but it references libB, which, in turn, references libA. Now, consider this snippet in ClassC.cs, inside libC.

using libA;
…

If I have 3 Microsoft.CodeAnalysis.Project objects, whose ProjectReferences mirror that setup I described above for the .csproj files, and compute the Project.GetCompilation for the Project object corresponding to libC, I get a diagnostic error for the use of libA within it.

This related issue, from a few years ago, still is open and marked as a Question. In a strictly technical (and isolated) sense, I can understand that position. However — and back to this case —, given that same proj.sln/libC.csproj builds successfully, out-of-the box, from:

  • dotnet build
  • msbuild
  • inside VS

I'd consider this issue to be a bug.

@jaredpar
Copy link
Member

jaredpar commented Dec 7, 2020

This behavior is "By Design". The reason the solution builds in the setup you described is because of a feature in MSBuild where they pass the transitive set of references to the compiler. The compiler by explicit design does consider transitive references when building the compilation.

@jaredpar jaredpar closed this as completed Dec 7, 2020
@ltcmelo
Copy link
Author

ltcmelo commented Dec 9, 2020

@jaredpar I understand the design, and, as I mentioned in my original post, I'd say that — looked in isolation — it makes sense. However, this difference in behaviour poses a usability issue in the Roslyn API.

The compiler by explicit design does consider transitive references when building the compilation.

The compiler, itself, doesn't need to be changed; only the logic within a Solution/Project would account for this matter.

It's frustrating to see issues such as this one, whose compatibility use-case argument is obvious, being closed without a satisfactory explanation. In particular, in the area related to building, which is lacking in several aspects (e.g., MSBuildWorkspace and related); searching the web, one finds multiple hacks to deal with a variety of those.

@jaredpar
Copy link
Member

jaredpar commented Dec 9, 2020

The compiler, itself, doesn't need to be changed; only the logic within a Solution/Project would account for this matter

I understand but the Solution / Project model here have the same model as the compiler: all references must be explicit. Part of the reason for this is that our Solution / Project model is designed to represent all the different types of solutions that we can encounter. That includes cases where we do not have the full set of transitive references. It's actually not uncommon for customers to not have the complete set when building. Making references transitive at this level would break our ability to represent these solutions.

The appropriate layering of transitive references is on top of this model. Any component which wanted to apply transitive reference behavior could do a simple transform on the Solution to produce the behavior. Much in the same way that MSBuild applies the logic when building up the reference set passed to the compiler.

It's frustrating to see issues such as this one, whose compatibility use-case argument is obvious, being closed without a satisfactory explanation.

I'm sorry you feel this is frustrating. But our model must support the full set of solutions that our customers have and many of those solutions include non-transitive references and all the quirks that come with it.

@ltcmelo
Copy link
Author

ltcmelo commented Dec 9, 2020

First of all, thanks for the explanation @jaredpar . Yet, from all the technicality I could absorb — and based on the discussion in gitter (thanks @jnm2 and @CyrusNajmabadi too) —, I still see a possibility to deal with this issue. This is my suggestion:

  • Microsoft.CodeAnalysis.Project already has properties ProjectReferences and AllProjectReferences; why not adding one called TransitiveProjectReferences?

@jaredpar , you say "… That includes cases where we do not have the full set of transitive references… ", but note that, the case in question, we're talking about a "project reference", whatever that is (.csproj, vbproj, etc), so the API modelling would still be consistent I believe — i.e., for any Solution / Project kind for which the notion of a ProjectReference exist, so does that of TransitiveProjectReference.

NOTE

I have implemented a custom solution for this issue by manually collecting the transitive references, but the problem with this kind of approach is that, due to a subtle matter, it might break "tomorrow"; therefore, the need for an official solution. I have ~ a dozen workarounds in the area of project building (with/without MSBuildWorkspace, in .NET framework/core… see other bugs I've created in the past). In my experience — from building an industrial static analysis product that's been running for a couple of years —, the building/compilation support/API is the most fragile area of the entire Roslyn "ecosystem"; ironically, this is a quite important one, since a failure in the building/compilation compromises the whole semantic model.

@jaredpar
Copy link
Member

jaredpar commented Dec 9, 2020

Microsoft.CodeAnalysis.Project already has properties ProjectReferences and AllProjectReferences; why not adding one called TransitiveProjectReferences?

I still retain some skepticism of whether this is the right layering. Have to deal with a lot of questions like do we respect the DisableTransitiveProjectReferences MSBuild property, do we include ones where we just use the output but not actually reference (ordering dependencies), etc ...

My biggest issue remains that at the core transitive references as most users understand them are an MSBuild concept while Solution / Project is a compilation concept. Adding transitive references here is nothing more than a layering violation. Hence it really doesn't seem appropriate to put them at this layer. Instead I believe the right layering is to change the code which maps MSBuild projects to Solution to make a decision on whether or not to have transitive reference support evaluated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

3 participants