Enable implicit framework references#26200
Conversation
Implicit framework references were disabled at the time we moved over to the new SDK due to some remanants of project.json in our build. Those have all been cleaned up now and hence moving us over to using implicit framework references. Not having this enabled meant the API surface we were targeting in our projects was not actually what was available in the project target framework. Instead it was the intersection of the target framework and the set of NuGet packages we reference. This meant that code which should work simply wasn't due to the API surface not being correct.
|
CC @dotnet/roslyn-infrastructure, @tmat for review |
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.CSharp" Version="$(MicrosoftCSharpVersion)" /> | ||
| <PackageReference Include="System.Dynamic.Runtime" Version="$(SystemDynamicRuntimeVersion)" /> |
There was a problem hiding this comment.
SystemDynamicRuntimeVersion
❓ Can you remove any of these from packages.props?
There was a problem hiding this comment.
Eventually yes. I'm trying to get BuildBoss back to the state where it will gripe if you have Packages.props entries that aren't valid. I'm still unwinding the string here though on our core package references though. Have to track @terrajobst down to get some more detail before I can squash the rest of this.
There was a problem hiding this comment.
There are also other implications like: what if you use the wrong version of Sysetm.Dynamic.Runtime? We are kind of taking it on faith that we're doing it correctly today vs. using the correct versions.
There was a problem hiding this comment.
Have to track @terrajobst down to get some more detail before I can squash the rest of this.
What concerns do you have around this?
|
@jaredpar Although I don't object to the change itself, does this open us up to surprises where somebody accidentally depends on a new assembly that we're not deploying to VS? Or does your change to PortableFacades.swr now include the entire set so we're protected? |
This is the case and bonus is that it's automatic. I didn't make this change, the tooling noticed the diff, updated the file and gave me an error message that it had changed. In talking with @tmat I'm worried because this tool is using a hueristic vs. firm algorithm. I'm noodling on ways to make this more definitive. But this has always been the case though, it's not a new problem. |
Implicit framework references were disabled at the time we moved over to
the new SDK due to some remanants of project.json in our build. Those
have all been cleaned up now and hence moving us over to using implicit
framework references.
Not having this enabled meant the API surface we were targeting in our
projects was not actually what was available in the project target
framework. Instead it was the intersection of the target framework and
the set of NuGet packages we reference. This meant that code which
should work simply wasn't due to the API surface not being correct.
Ask Mode template not completed
Customer scenario
What does the customer do to get into this situation, and why do we think this
is common enough to address for this release. (Granted, sometimes this will be
obvious "Open project, VS crashes" but in general, I need to understand how
common a scenario is)
Bugs this fixes
(either VSO or GitHub links)
Workarounds, if any
Also, why we think they are insufficient for RC vs. RC2, RC3, or RTW
Risk
This is generally a measure our how central the affected code is to adjacent
scenarios and thus how likely your fix is to destabilize a broader area of code
Performance impact
(with a brief justification for that assessment (e.g. "Low perf impact because no extra allocations/no complexity changes" vs. "Low")
Is this a regression from a previous update?
Root cause analysis
How did we miss it? What tests are we adding to guard against it in the future?
How was the bug found?
(E.g. customer reported it vs. ad hoc testing)
Test documentation updated?
If this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting.