GitTools / GitLink

Making .NET open source accessible!
MIT License
555 stars 86 forks source link

Controlling the task output #165

Open jaredpar opened 7 years ago

jaredpar commented 7 years ago

Last night I was looking into the possibility of moving dotnet/roslyn to using the GitLink MSBuild task. Previously we were using the command line tool as a post build step. That works but had a couple of issues we were hoping the task would resolve. Most notable: performance and completeness.

Using the build task appears to fix the performance issues we were seeing with the command line tool. However it has another problem that is blocking adoption. Whenever it finds a file in the PDB that it can't find in the Git tree it will issue an MSBuild warning and dump output to the console.

This blocks adoption in roslyn because we build with /TreatWarningsAsErrors and have a couple of patterns that GitLink can't handle. The patterns are:

The XAML problem may be solvable but likely not the #line directive. The PDB is deliberately lying to GitLink here so not much for GitLink to do.

Been thinking about this over the evening. Wondering if we could get an option to tell GitLink to not warn on missing files. That should be off by default but for cases like Roslyn, where we know we're going to lie a bit, we could turn it on for the projects where it happens. Something along the lines of

<GitLinkWarnOnMissingFiles>false</GitLinkWarnOnMissingFiles>

Example of the XAML missing file warning:

  VisualStudioDiagnosticsWindow -> E:\code\roslyn\Binaries\Debug\Vsix\VisualStudioDiagnosticsWindow\Roslyn.VisualStudio.DiagnosticsWindow.vsix
E:\code\gitlink\output\debug\GitLinkTask\GitLink.targets(9,5): error : Unable to find file in git: "e:\code\roslyn\binaries\obj\csharpvisualstudio\debug\options\formatting\options\formatting\formattingoptionpagecontrol.xaml". [E:\code\roslyn\src\VisualStudio\CSharp\Impl\CSharpVisualStudio.csproj]
E:\code\gitlink\output\debug\GitLinkTask\GitLink.targets(9,5): error : Unable to find file in git: "e:\code\roslyn\binaries\obj\csharpvisualstudio\debug\options\options\advancedoptionpagecontrol.xaml". [E:\code\roslyn\src\VisualStudio\CSharp\Impl\CSharpVisualStudio.csproj]
jaredpar commented 7 years ago

Here is a quick code up of the idea of adding GitLinkWarnOnMissingFiles element

https://github.com/jaredpar/GitLink/commit/b5af8e83dbebddc4f1703284357c0f4f30fd9623

When going through this though I wonder if GitLinkSkipVerify should be used instead. The logic being that if you need GitLinkWarnOnMissingFiles to be off then you will always also need GitLinkSkipVerify to be true. Made me think that GitLinkSkipVerify should cover both cases.

Thoughts?

GeertvanHorrik commented 7 years ago

We had the same issues for our WPF libraries. I think SkipVerify is much more than just skipping missing files verification, so I definitely think we should introduce a new option for this. Or... maybe we could just auto-skip .xaml files (but then, what's next?).

jaredpar commented 7 years ago

Another idea I had was essentially adding glob support for files to ignore when linking. This would allow a very granular approach to filtering out problematic files. The default could simply be *.xaml.

GeertvanHorrik commented 7 years ago

Yes, but that wouldn't solve your #line directive issue, would it?

jaredpar commented 7 years ago

I think it would solve that. For that project we'd just override it to ignore both *.xaml plus the invalid file name we were specifying.