Automate validating sourcelink information - for Windows PDBs about arcade-services HOT 23 CLOSED

@tmat - any thoughts on this?

from arcade-services.

My manual step was pretty dumb, along the lines of

c:\temp>\t\strings System.Text.Encoding.pdb | \t\grep documents | \t\grep -v devdiv
{"documents":{"/home/dan/git/corefx/*":"https://raw.githubusercontent.com/danmosemsft/corefx/ded9b8da999dbabb24abb697b429de73229a16bd/*"}}

Perhaps there's some proper API for reading portable PDB's? (We're only talking about managed PDB's here, right?)

Also it would be helpful to try a GET on the link (appending something to make it valid eg /README.md which should be a safe bet -- making something like https://raw.githubusercontent.com/danmosemsft/corefx/ded9b8da999dbabb24abb697b429de73229a16bd/README.md )

from arcade-services.

You can install the sourcelink tool via dotnet tool install --global sourcelink which has some nice helpers:

alexander:~$ sourcelink --help
Source Code On Demand

Usage:  [options] [command]

Options:
  -h|--help  Show help information

Commands:
  print-documents  print the documents stored in the pdb or dll
  print-json       print the Source Link JSON stored in the pdb or dll
  print-urls       print the URLs for each document based on the Source Link JSON
  test             test each URL and verify that the checksums match

Use " [command] --help" for more information about a command.

from arcade-services.

@akoeplinger that is much better, thank you. Clearly the way to go.

from arcade-services.

C++ PDBs should also use Source Link.

from arcade-services.

Note that @Anipik is about to merge his change to fix sourcelink for the facades coming out of CoreFX (which was the original problem that spurred this conversation). So if we wanted to add a validation step, it need not wait on that.

from arcade-services.

AFAIU right now sourcelink cli tool can only handle Portable PDBs and the PDBs that we have during publish/release are Windows PDBs. I'm still investigating.

from arcade-services.

You could update the cli tool to support Windows PDBs. Although dealing with Windows PDBs is never much fun - you'd need to add a package reference to Microsoft.DiaSymReader and Microsoft.DiaSymReader.Native packages.

I don't think we should be building Windows PDBs directly though. We should always build Portable PDBs and convert them to Windows PDBs. The converter preserves SourceLink, so validating the Portable PDBs is sufficient.

from arcade-services.

Thanks @tmat . One question, the symbol uploader supports converting portable pdbs to windows pdbs, can't we disable the pdb conversion during build and only do the conversion during publishing?

from arcade-services.

We can't since it doesn't support embedded PDBs yet. Even if it did, I don't think that's the right place to convert actually. We should be converting during build.

from arcade-services.

The converter preserves SourceLink, so validating the Portable PDBs is sufficient.

@tmat so do you think doing this validation during the build is the way to go? I.e., do the validation during the build (before conversion to win pdb) instead of during publishing ?

from arcade-services.

No, we should do the validation on Portable PDBs during publishing. We should also check during publishing that all managed binaries that are being published have Portable or Embedded PDBs.

from arcade-services.

C++ PDBs should also use Source Link

@Anipik were you going to check whether they do - for the native binaries from core repos at least? Wondering whether more work is needed for them.

from arcade-services.

Yes, we should definitely do that. I guess we won't get around reading Windows PDBs in SourceLink validation.

from arcade-services.

Now that I'm thinking about it more I don't think we have the necessary APIs in Microsoft.DiaSymReader/Native to read native source link data (they are stored in a different stream within the PDB than managed ones). No problem adding those APIs it's just work.

So I suggest we separate validation of Source Link Portable PDBs and native (C++ generated) PDBs. Let's make sure our Portable PDBs are good first and later on we can add validation of native PDBs.

from arcade-services.

for the native binaries from core repos at least?

I checked sourcelink for the native dlls that we produce in corefx and it was giving me an error as "Invalid Header"

from arcade-services.

@Anipik How did you check?

from arcade-services.

so one of the native pdbs that we ship is clrcompression.pdb so I just ran

C:\git\coreclr>sourcelink test E:\sdkmastersymbols\clrcompression.pdb
Invalid COR20 header signature.

from arcade-services.

it's not portable pdb so it's not gonna work

from arcade-services.

So I suggest we separate validation of Source Link Portable PDBs and native (C++ generated) PDBs. Let's make sure our Portable PDBs are good first and later on we can add validation of native PDBs.

Created this issue to track the work on Portable PDBs: dotnet/arcade#2512

from arcade-services.

Moved this to the same release as the blocking issue. @markwilkie @riarenas

from arcade-services.

Out of scope for dotnet/arcade#6418.

from arcade-services.

Closing this as the issue is almost 4 years old and we don’t have plans to work on this in the near future. In case you believe the issue needs resolution - please reopen it and provide a reason.

from arcade-services.