Open dfederm opened 2 years ago
Note that standard restore doesn't really support SetTargetFramework either.
NuGet chooses the best framework of the available ones.
NuGet chooses the best framework of the available ones.
What does that mean? NuGet doesn't choose the target framework at all across a P2P that explicitly sets SetTargetFramework, I thought. We set it when NuGet isn't present to make the decision, at least. I expected (perhaps in ignorance) that the metadata would carry weight even on nuget-aware projects as well.
My initial investigation found some interesting results. As David noted, removing SetTargetFramework works and migrating to SDK-style projects works.
This is because SDK-style projects are treated as having an inner and outer node:
For SDK-style projects, the SetTargetFramework applies but later on the TargetFramework global property is removed because project graph knows that this project is SDK-style. If the project is legacy, project graph treats its references as legacy and will only do a single evaluation of them. This results in the follow evaluations:
Legacy with SetTargetFramework -> SDK-style (net472, net5.0) | Project | Global Properties |
---|---|---|
Legacy | ||
referenced SDK-style | TargetFramework=net472 |
Legacy without SetTargetFramework -> SDK-style | Project | Global Properties |
---|---|---|
Legacy | ||
referenced SDK-style | ||
referenced SDK-style | TargetFramework=net472 | |
referenced SDK-style | TargetFramework=net5.0 |
SDK-style -> SDK-style | Project | Global Properties |
---|---|---|
SDK-style | ||
referenced SDK-style | ||
referenced SDK-style | TargetFramework=net472 | |
referenced SDK-style | TargetFramework=net5.0 |
For builds, its okay if the graph is incomplete since all of the projects that will actually be built are there. However, for restore we need every project and all inner nodes representing their target frameworks.
We could fix the exception here but we'd probably need to fail restore saying that the project graph is incomplete. For now the workaround of removing SetTargetFramework
seems like the best option.
What does that mean? NuGet doesn't choose the target framework at all across a P2P that explicitly sets SetTargetFramework, I thought. We set it when NuGet isn't present to make the decision, at least. I expected (perhaps in ignorance) that the metadata would carry weight even on nuget-aware projects as well.
During restore, SetTargetFramework is completely disregarded because NuGet needs a full representation of all projects and all target frameworks, not just the ones being built. It then does a complete restore for each project while the build might be more scoped and only build certain target frameworks. So at evaluation time, NuGet gets all projects, and at build time MSBuild only builds projects based on stuff like SetTargetFramework.
So at evaluation time, NuGet gets all projects, and at build time MSBuild only builds projects based on stuff like SetTargetFramework.
How can we accommodate both? Is there some Condition we should add to the SetTargetFramework
metadata so that nuget restore ignores it but build uses it? Or if as it sounds, NuGet already ignores it during restore, why is restore failing?
How can we accommodate both? Is there some Condition we should add to the SetTargetFramework metadata so that nuget restore ignores it but build uses it? Or if as it sounds, NuGet already ignores it during restore, why is restore failing?
Static graph-based restore is expecting to get a graph back from MSBuild that represents every outer and inner evaluation. However, you've discovered a bug/feature where if you use SetTargetFramework
with a legacy project referencing an SDK-style project, the graph is incomplete. NuGet then reads the graph and can't find the outer project evaluation representing the project and its target frameworks and unfortunately at the moment it throws an exception rather than logging an error.
We think the solution is to have two modes for static graph. One mode tells MSBuild to get all project edges that represent the complete graph and another mode (which is the current one) that only gets project edges that are to be built. This second mode leaves out some outer project evaluations that NuGet needs but would potentially cause MSBuild to build more than it needed to. But a complete graph is needed for restore.
Makes sense.
Would that perchance also fix this bug?
This doesn't work: dotnet build /r /p:TargetFramework=net472
Note that I want the build of the project to only build net472. But adding the /r
switch, which should restore the project before the build, also gets infected by the TargetFramework global property and (IIRC) propagates to P2Ps and infects them, leading to restore failure. Perhaps I should file a separate bug to track that. What do you think?
@AArnott That sounds like a dup of https://github.com/NuGet/Home/issues/11653.
NuGet Product Used
MSBuild.exe
Product Version
msbuild 17
Worked before?
No
Impact
It's more difficult to complete my work
Repro Steps & Context
Dep\Dep.csproj
:Broken\Broken.csproj
:Do a static graph restore on
Broken\Broken.csproj
. The output looks like:The NRE specifically is in
MSBuildStaticGraphRestore.GetPackageSpec
. The caller iterates eachProjectWithInnerNodes
and callsGetPackageSpec(project.OuterProject)
, butproject.OuterProject
is null for theDep\Dep.csproj
entry.If any of the following is changed, it works:
SetTargetFramework
metadata fromBroken\Broken.csproj
Broken\Broken.csproj
to SDK-styleVerbose Logs
No response