Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
baronfel opened this issue Feb 28, 2024 · 3 comments · May be fixed by #9805
Open

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

baronfel opened this issue Feb 28, 2024 · 3 comments · May be fixed by #9805
Assignees
@baronfel
Labels
Area: Terminal Logger Problems with the livelogger/fancylogger/terminallogger -tl functionality. Feature Request triaged

Comments

@baronfel
Copy link
Member

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
@baronfel baronfel added Feature Request needs-triage Have yet to determine what bucket this goes in. Area: Terminal Logger Problems with the livelogger/fancylogger/terminallogger -tl functionality. labels Feb 28, 2024
@baronfel baronfel linked a pull request Feb 29, 2024 that will close this issue
Open
@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Mar 2, 2024 •
edited

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
Copy link
Member Author

baronfel commented Mar 2, 2024 •
edited

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!

@AR-May AR-May added triaged and removed needs-triage Have yet to determine what bucket this goes in. labels Mar 14, 2024
@baronfel baronfel mentioned this issue Mar 14, 2024
Open
@baronfel
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@baronfel baronfel

Labels
Area: Terminal Logger Problems with the livelogger/fancylogger/terminallogger -tl functionality. Feature Request triaged
Projects
None yet
Milestone
No milestone
3 participants
@baronfel @KalleOlaviNiemitalo @AR-May