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

S.R.TypeExtensions not deployed for net461 application #2035

Closed
jaredpar opened this issue Mar 10, 2018 · 10 comments
Closed

S.R.TypeExtensions not deployed for net461 application #2035

jaredpar opened this issue Mar 10, 2018 · 10 comments
Assignees
Milestone

Comments

@jaredpar
Copy link
Member

Have a situation where System.Reflection.TypeExtensions is not being deployed. Here is the project setup that we have:

  • CSharpCompilerTestUtilites: targets netstandard1.3 and uses System.Reflection.TypeExtensions
  • CSharpCompilerEmitTests: targets net461 + netcoreapp2.0, references CSharpCompilerTestUtilities

The net461 output of CSharpCompilerEmitTests does not contain System.Reflection.TypeExtensions. That is necessary to run and hence some of our unit tests fail.

To repro:

Note: In our build DisableImplicitFrameworkReferences is false by default and we have a tendancy to use explicit package references (hold over from the project.json days). It's possible this is contributing here but I'm unable to track this down.

@nguerrera nguerrera added this to the 2.1.3xx milestone Mar 10, 2018
jaredpar added a commit to jaredpar/roslyn that referenced this issue Mar 10, 2018
This is referenced in the TestUtilities.csproj but for some
reason not explicitly copied to the output folder here. Manually adding
for now.

dotnet/sdk#2035
@nguerrera
Copy link
Contributor

@jaredpar, this fell off my radar. Are you still seeing this? Did you workaround it?

@nguerrera
Copy link
Contributor

Looking now.

@nguerrera
Copy link
Contributor

I see the linked commit with the direct ref workaround now.

@nguerrera
Copy link
Contributor

nguerrera commented Apr 2, 2018

OK, here's what's happening:

  1. There are 2 levels of indirection from CSharpCompilerEmitTest to the PackageReference to S.R.TypeExtensions:
    i. CSharpCompilerEmitTest (net461;netcoreapp2.0) -> CSharpCompilerTestUtilities (netstandard1.3)
    ii. CSharpCompilerTestUtilities (netstandard1.3) -> TestUtilities (netstandard1.3;net46;net461;netcoreapp2.0)

  2. The package reference to S.R.TypeExtensions (two levels away) is conditioned on TargFramework=netstandard1.3

This condition effectively flows through the P2P graph as packages flow transitively. So CSharpCompilerEmitTest (net461) gets TestUtilities (net461) package references, and not TestUtilities (netstandard1.3) package references.

It's confusing in this edge case, but the system is behaving as designed and I think it is the right design. As I try to think about how to specify different behavior that would make this case work, my brain is exploding and I think we'd only end up making things more complicated.

IMHO, the best fix on your side would be to remove that condition from from the package reference. I don't see why it's necessary and it is what is preventing the indirect net461 consumers from getting the package reference to flow as needed.

@jaredpar
Copy link
Member Author

jaredpar commented Apr 3, 2018

@nguerrera in your explanation you list CSHarpCompilerEmit as net46 when it is actually net461. Does that impact your analysis?

@nguerrera
Copy link
Contributor

It does not.

@nguerrera
Copy link
Contributor

I've edited my analysis. I misread $(RoslynPortableTargetFrameworks)

@jaredpar
Copy link
Member Author

jaredpar commented Apr 3, 2018

I don't see why it's necessary and it is what is preventing the indirect net46 consumers from getting the package reference to flow as needed.

This is part of me trying to undo the sins of the past. Roslyn compiles with $(DisableImplicitFrameworkReferences) == 'true' by default. This is due to our history with project.json and building for CoreClr before even CLI existed. The result is we have a bunch of project which to the casual observer just reference a random jumble of NuGet packages.

I've been trying to flip us back to the default of $(DisableImplicitFrameworkReferences) == 'false' by flipping a couple of core projects then trimming off the unneeded NuGet packages.

In this case I think I got tripped up because I had the following mental model:

The TF of the final unit test project will ultimately determine whether or not S.R.TypeExtensions needs to be deployed. Anything targeting net46 will get TestUtilites@netstandard1.3 and hence get the PackageReference through TestUtilities. Anything targeting net461 will get TestUtilities@netstandard2.0 and hence this PackageReference isn't needed.

I didn't realize how the intermediate project threw a bit of a wrench into this problem.

@nguerrera
Copy link
Contributor

The intermediate project tripped me up too. It's very confusing.

@jaredpar
Copy link
Member Author

jaredpar commented Apr 3, 2018

Thanks for the explanation!

JL03-Yue pushed a commit that referenced this issue Mar 19, 2024
…120.1 (#2035)

[main] Update dependencies from dotnet/arcade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants