[Feature Request]: ability to create target maps when circular dependency exists instead of exiting with error

assarbad commented 9 months ago

Summary

Using the -graph (target graph) feature to build a graph involving a circular dependency would be tremendously helpful to debug and get rid of the circular dependency. The circular dependency could be pointed out by way of outlining it in a different color or otherwise.

Background and Motivation

Please consider ClVersionRetrieve.proj as of dda59c6.

The project mimics a Visual C++ project, but sidesteps the actual Build target.

When I originally built it prior to the revision mentioned above, I was only able to build it directly like so:

msbuild -p:Configuration=Release -t:ClVersionRetrieve -verbosity:normal ClVersionRetrieve.proj

however, using the Rebuild target instead (or using it from within the solution):

msbuild -p:Configuration=Release -t:Rebuild -verbosity:normal ClVersionRetrieve.proj

caused an error:

  C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Current\Bin\amd64\Microsoft.Common.CurrentVersion.targets(5587,7): error MSB4006: There is a circular dependency in the
target dependency graph involving target "CleanReferencedProjects". [D:\source\repos\msbuild-batching\ClVersionRetrieve.proj]

The error refers to the DependsOnTargets attribute in the following snippet:

  <Target
      Name="Clean"
      Condition=" '$(_InvalidConfigurationWarning)' != 'true' "
      DependsOnTargets="$(CleanDependsOn)" />

With the aforementioned revision I emptied out the Clean target (and removed all its dependencies and dependents) to resolve the issue.

However, even after doing that I get an error when attempting to create the target graph, while the build itself works fine. The error I get when attempting the above is:

$ msbuild -p:Configuration=Release -t:Rebuild -verbosity:normal ClVersionRetrieve.proj -graph
MSBuild version 17.7.2+d6990bcfa for .NET Framework
Build started 2023-09-26 22:20:03.

MSBUILD : error : MSB4251: There is a circular dependency involving the following projects:
MSBUILD : error : D:\source\repos\msbuild-batching\ClVersionRetrieve.proj ->
MSBUILD : error : D:\source\repos\msbuild-batching\ClVersionRetrieve.proj

Build FAILED.

  MSBUILD : error : MSB4251: There is a circular dependency involving the following projects:
MSBUILD : error : D:\source\repos\msbuild-batching\ClVersionRetrieve.proj ->
MSBUILD : error : D:\source\repos\msbuild-batching\ClVersionRetrieve.proj

    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:00.15

Proposed Feature

Extend -graph such that it doesn't issue MSB4251 and instead -- at most -- outputs a warning, otherwise proceeding to output a graph that can be used to investigate the circular dependency issue.

Alternative Designs

No response

JanKrivanek commented 9 months ago

@assarbad - The Static Graph aims for increased build performance for larger scale builds via frontloading the dependencies discovery and performing their topologic sort traversal for deterministic and 'each project just once' scheduling. More details in the linked doc.

Producing non-schedulable output (not traversable via topological sort) is a non-goal for the feature.

assarbad commented 9 months ago

@JanKrivanek understood. It's a shame, though. I wish it could also be used to determine those cyclic dependencies in order to address those issues when they're encountered.

I spent several hours on similar issues to the ones I commented on in the vscode-dotnettools ticket (linked above) and I think -- but I am not a 100% sure -- that a look at the dependency graph with target names could have shrunk that time spent considerably.

JanKrivanek commented 9 months ago

Understood and thanks for the shared feedback and suggestion. I'm marking this with 'gathering-feedback' so that it can be considered and prioritized during next cycle planning.

rainersigwald commented 9 months ago

that a look at the dependency graph with target names

It's important to understand that no such thing exists today. Dependencies between targets are processed with a stack-based mechanism and most dependencies are discovered "just in time", which is part of why the cycle detection mechanism is not good today.

dotnet / msbuild