NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

[Bug]: Incorrect restore in Visual Studio when a dependency to a C# project is flowed through a C++/CLI project #12170

Closed ocallesp closed 1 year ago

ocallesp commented 2 years ago

NuGet Product Used

MSBuild.exe

Product Version

msbuild -v:m -t:Restore .\ProjectA\ProjectA.csproj

Worked before?

No response

Impact

No response

Repro Steps & Context

The original bug was reported in https://github.com/dotnet/project-system/issues/8381


Visual Studio 2022 17.3 Preview

Summary

The project.assets.json file is incomplete for a C# project when it transitively depends on another C# project through a C++/CLI project. This problem seems to only occur in Visual Studio 2022 17.3 (tested with Preview 4.0 to 6.0) and not previous versions.

Lets say we have three projects: ProjectA (C#), ProjectB (C++/CLI) and ProjectC (C#); so that ProjectA references ProjectB which references ProjectC:

ProjectA (C#) --(depends on)--> ProjectB (C++/CLI) --> ProjectC (C#)

If the ProjectReference from ProjectB to ProjectC does not contain the metadata Project (e.g. <Project>{e5599aee-f39e-4613-b48d-7824eca526e5}</Project>), MSBuild (through command line) and Visual Studio restore won't behave the same way. Typically, the transitive reference to ProjectC is missing from ProjectA's asset file when performing NuGet restore in Visual Studio:

  (...)
  "targets": {
    ".NETFramework,Version=v4.8": {
      "ProjectB/1.0.0": {
        "type": "project",
-       "dependencies": {
-         "ProjectC": "1.0.0"
-       },
        "compile": {
          "bin/placeholder/ProjectB.dll": {}
        },
        "runtime": {
          "bin/placeholder/ProjectB.dll": {}
        }
      },
-     "ProjectC/1.0.0": {
-       "type": "project",
-       "framework": ".NETFramework,Version=v4.8",
-       "compile": {
-         "bin/placeholder/ProjectC.dll": {}
-       },
-       "runtime": {
-         "bin/placeholder/ProjectC.dll": {}
-       }
-     }
    }
  }
  (...)

Steps to Reproduce

  1. Create three projects as described in the summary

  2. Remove <Project> metadata in the reference from ProjectB to ProjectC:

    -    <ProjectReference Include="..\ProjectC\ProjectC.csproj">
    -      <Project>{e5599aee-f39e-4613-b48d-7824eca526e5}</Project>
    -    </ProjectReference>
    +    <ProjectReference Include="..\ProjectC\ProjectC.csproj" />
  3. Perform a restore on ProjectA using MSBuild and check the contain of .\ProjectA\obj\project.assets.json: msbuild -v:m -t:Restore .\ProjectA\ProjectA.csproj

  4. Open the solution in Visual Studio 2022 17.3 Preview, force a restore if needed, and check the content of .\ProjectA\obj\project.assets.json again; you will notice the changes mentioned in the summary

See this repository: joeltankam/restore-cli-sample

Expected Behavior

MSBuild CLI and Visual Studio should have the same behavior, which means the restore should not be performed when opening the solution in Visual Studio. Package Manage log should be:

All packages are already installed and there is nothing to restore.

Actual Behavior

The restore is performed again in Visual Studio for ProjectA:

Restored (...)\ProjectA\ProjectA.csproj (in 0.9 ms). NuGet package restore finished.

Changing .\ProjectA\obj\project.assets.json as described in the summary; creating incremental compilation issues when switching from command-line to VS (e.g. some projects are unnecessarily re-restored and recompiled).

User Impact

We have a 950+ projects repository (~85% C#, ~10% C++, ~5% C++/CLI), with an average solution size of around 280 projects.

When switching the .NET SDK project file format, we removed <Project> metadata on all project references, including C++ projects. This used to work perfectly until Visual Studio 2022 17.3. Now that it does not, it has some significant impacts considering the size of our repository and solutions (it's standard practice for us first to compile the whole repository through command-line, and then open a specific solution in Visual Studio, thus the issue).

On a side note, the Project metadata is supposed to be the ProjectGuid property of the referenced project; where does that value come from in this scenario since C# projects don't have that property anymore?

Verbose Logs

No response

nkolev92 commented 2 years ago

Hey @joeltankam,

While we work on figuring out our action here, I'd recommend keeping the workaround for the time being. You should usually be able to find the IDs in the solution file.

Thank you for the repo link! I am able to repro your problem.

Why did this break?

In 17.3, NuGet switched from one set of project-system APIs to a different one. The original APIs being used in 16.x, 17.0-17.2 were causing performance problems. In 17.3 we started using vcproject APIs. https://github.com/NuGet/NuGet.Client/commit/beb3bd51b1d4af1a1c17cbed5a9cc4b190a10a30

Turns out these APIs need the Project attribute to exist on a project reference.

Hey @olgaark,

As you're aware starting 17.3, we use VCProjectReference to read the project references. https://github.com/NuGet/NuGet.Client/blob/5d2c71b138e6a70f7843f58799a110c94a984990/src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/ProjectServices/NativeProjectSystemReferencesReader.cs#L59-L90

When declaring a project reference in a .NET Framework C++/CLI project,

The below reports a project reference,

  <ItemGroup>
    <ProjectReference Include="..\ProjectC\ProjectC.csproj">
      <Project>{e5599aee-f39e-4613-b48d-7824eca526e5}</Project>
    </ProjectReference>
  </ItemGroup>

however, if you remove the project element it stops reporting.

  <ItemGroup>
    <ProjectReference Include="..\ProjectC\ProjectC.csproj">
    </ProjectReference>
  </ItemGroup>
joeltankam commented 1 year ago

Hello @nkolev92 @olgaark

Any news on a definitive fix for this issue ?

olgaark commented 1 year ago

@nkolev92 Yes, referenced project guid is required in VC projects and this has always been the case, no change on VC side.

joeltankam commented 1 year ago

@nkolev92 Yes, referenced project guid is required in VC projects and this has always been the case, no change on VC side.

Hello, therefore, how is the change of behavior during restore explained? I think we can agree there's a regression here, any fix intended?

nkolev92 commented 1 year ago

@olgaark If that's the case, any idea why the builds ever worked? NuGet restore is only one component to care for the project references, the build would do the same thing. Any idea why we didn't any issues there?

olgaark commented 1 year ago

There was probably the same issue, just not visible if csproj happened to be built before the c++ one

joeltankam commented 1 year ago

Any update on this issue ?

jeffkl commented 1 year ago

Team Triage: We are closing this as by design, C++ projects require the project GUID to be part of the project reference.