haacked / feedback

Ask @haacked anything!
7 stars 1 forks source link

Issues with line endings and PDBs #152

Closed tmat closed 6 years ago

tmat commented 7 years ago

Phil, as you are surely aware the PDB files generated by various compilers and tools allow to associate source file names with links on web. The debugger can use this information to download the source code from the web when stepping thru a component that the developer doesn't have source code for on their machine. For repositories on GitHub these links would point to https://raw.githubusercontent.com.

In addition to the source file names the compilers also store a SHA checksum of each source file in the PDB. The debugger validates that the hash of the file it acquires from the server matches the one stored in the PDB. In practice, repositories might specify a line ending translation in .gitattributes file. As a result, the line endings that the compiler sees might be different from those on https://raw.githubusercontent.com, hence the SHAs are different, hence the debugger considers the files not matching.

There are several possible solutions to this problem that involve tweaking the git configuration on the build server. The hash checking can also be disabled in the debugger settings.

However, I am wondering if it would be possible/desirable for the content on https://raw.githubusercontent.com to match the settings in .gitattributes file. After all in some cases the exact form of line endings might not only affect debugging but also correctness.

What do you think?

/cc @AArnott @ctaggart @jaredpar

haacked commented 7 years ago

@tmat Let me look into it. I'm not sure if it's possible or even desirable, but I'll ask around.

AArnott commented 7 years ago

Maybe if you have concerns you can offer URLs for both scenarios.

tmat commented 7 years ago

@Haacked Thanks! We have figured out an alternative solution on the debugger side. The next version of the VS debugger is gonna try to replace the line endings if the checksum doesn't match and calculate a new checksum. So at this point there is no need for change on your side. Still would be curious what your opinion is.

ctaggart commented 7 years ago

When core.autocrlf=true is set, the git client sets crlf when on Windows and leaves them lf when not on Windows. GitHub would not know which client did the build. I don't think this request makes sense.

Source linking only needs to be done on the build server. It is usually easy to set core.autocrlf=input on the build server. For people just getting started, I've made it so SourceLink automatically adjusts the line endings if that would make the checksums match. I've updated the documentation. I think that GitHub should only serve raw files as they are stored in the repostory.

https://github.com/ctaggart/SourceLink/wiki/Line-Endings

AArnott commented 7 years ago

When core.autocrlf=true is set, the git client sets crlf when on Windows and leaves them lf when not on Windows. GitHub would not know which client did the build.

Very good point.

It is usually easy to set core.autocrlf=input on the build server

But doesn't the repo-level .gitattributes file take precedence? How would the build server override that?

ctaggart commented 7 years ago

But doesn't the repo-level .gitattributes file take precedence? How would the build server override that?

core.autocrlf=input just tells the git client not to mess with the line endings. I may not understand your question. To see how git behaves, try this with a test repository. Sadly, the default for the git client on Windows when unset used to be core.autocrlf=input. I used this script to test between different git versions:

https://gist.github.com/ctaggart/469a3b9fb0113a2cc367fa347644a3b7

BTW @AArnott, would love a review of SourceLink v2 from you. I've revised all the docs.

AArnott commented 7 years ago

@tmat How can I tell if a PDB I have is the new portable PDB or the old Windows kind? I'm using the .NET SDK project type and the PDB it builds has an header unrecogized by GitLink/PdbGit and I wonder if it's because it's a portable PDB.

Also, how do I switch between generating a portable PDB and a Windows one in a .NET SDK project? (EDIT: Answer is <DebugType>portable</DebugType> vs. full)

@ctaggart does SourceLink v2 only work when csc.exe is generating portable PDBs, or does it also work with Windows PDBs? For most of my projects, I'm hung up by the same requirement Roslyn is: I have to continue to use Windows PDBs till Watson supports portable PDBs. But I'll give it a try.

AArnott commented 7 years ago

@ctaggart So other than the fact that I had to add both a PackageReference and a DotNetCliToolReference to my project, it Just Worked, including fixing line endings (which I need to undo now since it was a local build but that's as easy as staging the files) and including the source code of two files which I had uncommitted changes in. Super cool. I'll keep it. :)

Unless I have to give it up for Windows PDB generation.

Now to break the news to the gitlink folks... :)

AArnott commented 7 years ago

OK, so for now since I think the safe position is to use Windows PDBs (I'll take my cue from @tmat when it's safe to switch), and since SourceLink v2 seems to quietly turn off when DebugType=full, I'm going back to PdbGit/GitLink for now.

Very cool though. And I look forward to switching to SourceLink. At this point I don't think I would have any motivation to update GitLink to support portable PDBs because SourceLink is already there.

haacked commented 6 years ago

The next version of the VS debugger is gonna try to replace the line endings if the checksum doesn't match and calculate a new checksum

That seems like a reasonable solution.