Skip to content

Enable sourcelink for Visual fsharp tools repo - #2675

Merged
brettfo merged 10 commits into
dotnet:masterfrom
KevinRansom:sourcelink
Mar 30, 2017
Merged

Enable sourcelink for Visual fsharp tools repo#2675
brettfo merged 10 commits into
dotnet:masterfrom
KevinRansom:sourcelink

Conversation

@KevinRansom

@KevinRansomKevinRansom commented Mar 22, 2017

Copy link
Copy Markdown
Contributor

Enable sourcelink for Visual F# tools repo.

  1. Sourcelink is now enabled by default. set an environment variable
    set usesourcelink=false to turn it off
  2. Release build now builds with portable pdb and sourcelink.
  3. Debug build builds with portable pdb and sourcelink. + optimizations

Note: In order
Kevin

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

@dotnet-bot test this please

3 similar comments
@mmitche

Copy link
Copy Markdown
Member

@dotnet-bot test this please

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

@dotnet-bot test this please

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

@dotnet-bot test this please

@ctaggart

Copy link
Copy Markdown
Contributor

@KevinRansom What are your thoughts on using a debug type of embedded for the tools? We see only about a 20% increase in dll files, but it guarantees the debug info and source link.

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

@ctaggart not embed in release builds. The working set of released product is important to minimize. Sourcelink works very well and only has a teeny tiny overhead. switching to portable PDBs is like getting free working set given how immense the old symbol files were.

Embedding symbols and not source may be interesting for nuget deployment I think we will need some experience to see what works best in that scenario.

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

There is a coreclr bug that impacts this:

https://github.com/dotnet/coreclr/issues/3541 : JITTrackingEnabled on coreclr making internals visible to not work correctly. I expect it to be fixed in .NET Standard 2.0

Mainly this affects debugging the coreclr version of the FSC compiler. I'm going to see if I can find a work around.

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

@dotnet-bot test this please

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

Good lord, what has happened to the build?

@KevinRansomKevinRansom mentioned this pull request Mar 30, 2017
</Reference>
<Reference Include="Microsoft.VisualStudio.Text.Internal, Version=$(RoslynVSBinariesVersion).0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
<HintPath>$(FSharpSourcesRoot)\..\packages\RoslynDependencies.Microsoft.VisualStudio.Text.Internal.$(RoslynVSPackagesVersion)\lib\net45\Microsoft.VisualStudio.Text.Internal.dll</HintPath>
<HintPath>$(FSharpSourcesRoot)\..\packages\Microsoft.VisualStudio.Text.Internal.15.0.26201-alpha\lib\net46\Microsoft.VisualStudio.Text.Internal.dll</HintPath>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned about adding a dependency to an alpha release.

@brettfo

Copy link
Copy Markdown
Member

As it currently stands, it doesn't look like source link will do what you think:

  1. Our builds come out of a private branch with master simply merged in so the git SHA will only be valid against our internal copy of the repo; it won't mean anything to GitHub.
  2. The build system passes the git SHA via the %BUILD_SOURCEVERSION% environment variable and I don't see that being referenced in any of our files.

@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

@brett, does that mean that the internal repo doesn't have the sha's from GitHub? I figured once this was in, we could figure out the exact magic to get the correct sha.

@ctaggart

ctaggart commented Mar 30, 2017

Copy link
Copy Markdown
Contributor

I just downloaded an fsc.pdb from this build. You can use a tool I built to test it and look at it.

C:\Users\CameronTaggart\cs\sourcelink-test\ClassLibrary1 [fsharp ≡ +0 ~2 -0 !]> dotnet sourcelink
SourceLink 2.1.0-b446
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.
C:\Users\CameronTaggart\cs\sourcelink-test\ClassLibrary1 [fsharp ≡ +0 ~2 -0 !]> dotnet sourcelink print-json C:\Users\Ca
meronTaggart\Downloads\fsc.pdb
{"documents": { "D:\\j\\workspace\\release_ci_pa---866fd2c3\\*" : "https://raw.githubusercontent.com/Microsoft/visualfsh
arp/532a5b763fc18fd9bd4795077f503a1a099dff8c/*" }}
C:\Users\CameronTaggart\cs\sourcelink-test\ClassLibrary1 [fsharp ≡ +0 ~2 -0 !]> dotnet sourcelink print-documents C:\Use
rs\CameronTaggart\Downloads\fsc.pdb
10db868b4c84ef9b97a08a4d2bc1ea94 md5 fsharp D:\j\workspace\release_ci_pa---866fd2c3\src\fsharp\fscmain.fs
C:\Users\CameronTaggart\cs\sourcelink-test\ClassLibrary1 [fsharp ≡ +0 ~2 -0 !]> dotnet sourcelink print-urls C:\Users\Ca
meronTaggart\Downloads\fsc.pdb
10db868b4c84ef9b97a08a4d2bc1ea94 md5 fsharp D:\j\workspace\release_ci_pa---866fd2c3\src\fsharp\fscmain.fs
https://raw.githubusercontent.com/Microsoft/visualfsharp/532a5b763fc18fd9bd4795077f503a1a099dff8c/src/fsharp/fscmain.fs
C:\Users\CameronTaggart\cs\sourcelink-test\ClassLibrary1 [fsharp ≡ +0 ~2 -0 !]> dotnet sourcelink test C:\Users\CameronT
aggart\Downloads\fsc.pdb
1 Documents with errors:
10db868b4c84ef9b97a08a4d2bc1ea94 md5 fsharp D:\j\workspace\release_ci_pa---866fd2c3\src\fsharp\fscmain.fs
https://raw.githubusercontent.com/Microsoft/visualfsharp/532a5b763fc18fd9bd4795077f503a1a099dff8c/src/fsharp/fscmain.fs
error: url hash does not match: 10db868b4c84ef9b97a08a4d2bc1ea94
sourcelink test failed

The commit hash works. The file hash does not. Guessing line endings.

@ctaggart

ctaggart commented Mar 30, 2017

Copy link
Copy Markdown
Contributor

Confirmed that is line endings. I downloaded the file from that URL, calculated the checksum, opened up the file in VS Code, changed the line endings to CRLF and then calculated it again. Git repos store the files as LF, so you must compile as LF to source linking to work. This should be easy.

C:\Users\CameronTaggart\Downloads> Get-FileHash .\fscmain.fs -Algorithm md5
Algorithm Hash Path
--------- ---- ----
MD5 B61F1298FB0798601F310A4BBFC51542 C:\Users\CameronTaggart\Downl...
C:\Users\CameronTaggart\Downloads> code .\fscmain.fs
C:\Users\CameronTaggart\Downloads> Get-FileHash .\fscmain.fs -Algorithm md5
Algorithm Hash Path
--------- ---- ----
MD5 10DB868B4C84EF9B97A08A4D2BC1EA94 C:\Users\CameronTaggart\Downl...

Hopefully adding something like

git config --global core.autocrlf input

somewhere in the netci.groovy file is simple. @mmitche ?

@mmitche

Copy link
Copy Markdown
Member

@ctaggart All the git installations are on auto (not specified in the groovy file, though theoretically you could set the repo's git config before running any commands). This should be fixed on the repo side if possible since auto is what almost every developer is using too.

@brettfo
brettfo merged commit 4a4cc1e into dotnet:masterMar 30, 2017
@KevinRansom

Copy link
Copy Markdown
ContributorAuthor

@ctaggart, apparently VSCode hashes with both line endings in-order to figure things out, and VS needs a similar change.
I chatted with Tomas and he thinks that there is not really much the compiler can do to solve this. Picking hashing with line endings different than the local file, would break VS locally. Forcing the repo to a particular set of line endings, doesn't really solve the problem, it just makes our distribution work.

Also, we are still building our distributions on a VSO repo, which merges localization files each night and so the hash we use is not available on GitHub. There are hacks we can do to fix that, however, they give @brettfo nightmares, even to think about them. In order to allow him to be awake in the day, I'm going to not push for the hacks, just yet. He is confident that we will not have the closed source loc step for much longer, so the issue will go away.

@ctaggart

Copy link
Copy Markdown
Contributor

Forcing the repo to a particular set of line endings, doesn't really solve the problem, it just makes our distribution work.

@KevinRansom You don't force the repo to a set of line endings. You simply have the git client on the build server not change the line endings. The setting that tells git to clone on Windows without changing the files is core.autocrlf input.

@KevinRansom
KevinRansom deleted the sourcelink branch March 31, 2017 18:53
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
* Enable sourcelink for repo
* Make it turn offable
* Somehow I deleted microbuild=true
* remove specific symbol type settings from a couple of projects
* Enable sourcelink for build
* Immutable fix
* More tweaks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@KevinRansom@mmitche@ctaggart@brettfo@msftclas