dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.22k stars 1.35k forks source link

[Feature Request]: Terminal Logger output display should use relative paths for directories in the same 'repo root' #9800

Open baronfel opened 7 months ago

baronfel commented 7 months ago

Summary

I have a repo where I'm using the SDK Artifacts layout, which puts outputs in a layout like

<root>
- artifacts
  - publish
    - <project>
      - <target>
- src
  - <project>

When I publish one of these projects from its src directory, the output path link is fully-qualified:

image

Since this location is in the same 'repo' I'd prefer to see this path using relative pathing: ../../artifacts/publish/msbuild-versioning-sample/release_osx-x64

Background and Motivation

We already use relative pathing for directories under the current working directory, and it seems natural to me to use relative paths for all repo-local locations. The hard part might be deciding what is repo-local, since MSBuild doesn't really have a core concept of a repo root. Maybe Sourcebuild can be of use here, since it's properties can tell us a repo root?

Proposed Feature

If the SourceRoot Item is present (which it should be after the InitializeSourceControlInformation Target has finished) then use it as a basis for computing relative paths if the project isn't a direct ancestor of the path being made relative to display the outputs.

Alternative Designs

No response

KalleOlaviNiemitalo commented 7 months ago

There can be more than one SourceRoot item:

I think what it should do is check whether there is at least one SourceRoot item such that both the current working directory and the referenced file are within it. If so, convert the path to relative; else keep it absolute.

Roslyn recognizes the GenerateFullPaths property; Microsoft.CSharp.Core.targets passes it to the Csc task, and if the value is true, then the diagnostic messages from Csc include the full paths of files. Perhaps Terminal Logger should also recognize this property.

baronfel commented 7 months ago

Yeah, there can be - that's why in this PR I've tried to find the best one using a heuristic of "the SourceRoot item that the sourcelink targets have added source control provider metadata to". I do like your proposed enhancements of a) keep all sourceRoots and determine the best one at rendering time using comparisons, and b) enable/disable the functionality based on the GenerateFullPaths property - that's really useful!

baronfel commented 7 months ago

I've attempted to use GenerateFullPaths, but for some reason setting that to false (the default is true, set by the Common props) completely destroys the TL output, so I'm going to punt on that feature in the linked PR.