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.

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

Need to expose /embed compiler option through MSBuild #19127

Closed
KirillOsenkov opened this issue Apr 29, 2017 · 49 comments
Closed

Need to expose /embed compiler option through MSBuild #19127

KirillOsenkov opened this issue Apr 29, 2017 · 49 comments
Assignees
Milestone

Comments

@KirillOsenkov
Copy link
Member

csc.exe has the /embed option (embed all sources in the portable PDB). There is the EmbeddedFiles MSBuild item that lets you specify the list of files to embed.

However there is no MSBuild alternative to the /embed option and people have to resort to workarounds such as:

  <Target Name="PopulateEmbeddedFiles"
          AfterTargets="BeforeCompile"
          BeforeTargets="CoreCompile">
    <ItemGroup>
      <EmbeddedFiles Include="@(Compile)" />
    </ItemGroup>
  </Target>

It would be very useful to introduce a new Csc task parameter "EmbedAllSourcesInPdb" or similar. If set, it would automatically populate EmbeddedFiles with Sources.

Otherwise everyone will be forced to have a custom target like above.

Alternatively, we can do this entirely on the MSBuild side, and have that target added to the common targets and specify a condition to turn it on or off.

But basically if people want to embed sources in PDB they're very likely to want to embed them all anyway. So removing ceremony here would be very helpful.

@KirillOsenkov KirillOsenkov changed the title Need to expose /embed compiler option through MSBuild and Csc task Need to expose /embed compiler option through MSBuild Apr 29, 2017
@jaredpar jaredpar added this to the 15.3 milestone May 1, 2017
@jaredpar
Copy link
Member

jaredpar commented May 1, 2017

CC @tmat and @nguerrera

@tmat
Copy link
Member

tmat commented May 1, 2017

I'm not sure if embedding all sources is the primary scenario. Embedding sources that are not in source control seems more likely for projects that can use source link for files checked in a source control. Source Link already does this. See https://github.com/ctaggart/SourceLink

@KirillOsenkov
Copy link
Member Author

Having sources locally is of great benefit:

  1. For debugging performance (local sources will open faster)
  2. Avoids the security warning in the debugger
  3. For private repos (auth issues etc)
  4. For projects outside of source control, or which are not under source control at build time
  5. The PDB size doesn't grow a lot (esp. compared with Windows PDBs)

@tmat
Copy link
Member

tmat commented May 1, 2017

Re 3. Auth will be supported in future. //cc @gregg-miskelly

All these reasons are great, but I still don't think embedding all sources in PDB will be very common. We shall see. I'm not disagreeing though that having EmbedAllSourcesInPdb task parameter wouldn't be good. I'd wait to get more up votes for this proposal though. The alternative (adding a task) doesn't seem like that big of a deal.

@KirillOsenkov
Copy link
Member Author

I guess my real concern that I haven't voiced yet is the following. Our tools ecosystem is full of friction. NuGet, project.json, MSBuild, dotnet CLI, project systems, etc. they all are a 1000 papercuts, full of hard to diagnose issues. Nothing ever goes smoothly in the build tools world.

I view my personal responsibility in trying to reduce this friction as much as I can, making the .NET tools world a pleasant one to live in. When I see a potential roadblock for users, I try to remove it so that the experience is as smooth and fluent as possible.

"Just add a target" doesn't seem like a big deal to you or me, but this is a ridiculously unpleasant experience for 99% of people. And forcing each and every one of them who want to consume the feature that is already there to jump through hoops they don't understand just hurts my heart.

I do understand that it's always required work, and it always competes with a lot of other important stuff we can do. Adding a Csc task parameter is a big deal. But I always want to err on the side of less pain for the users. Please help me make our ecosystem as friction free as possible and unlock all the cool stuff we already have in Roslyn to empower our users.

@cwe1ss
Copy link

cwe1ss commented May 1, 2017

Full ack @KirillOsenkov!

Working with internal libraries and the .NET core tooling is pretty painful right now. Using project references makes debugging easy but currently is incredibly - slow and using NuGet packages is a big PITA as well because you can't easily step into them and you also get the burden of managing version numbers etc.
With project.json there at least was the compromise of being able to easily swap package references for local references but unfortunately this is a lot harder with MSBuild.

I'd love to see support for stepping into "packages" that "just works" without requiring the user to do anything.

I see 3 possibilities for this:

  • dotnet pack --include-source which puts the source into a separate folder.
  • Embedding sourcelink info.
  • Embedding the source with the option described here.

Using the dotnet pack command has the advantage that the DLL size does not increase but it probably makes it harder for tools like VS to pick up the right paths etc. (How should it know that it is a nuget package during debugging...)

Embedding sourcelink info seems good for open source and cases where you don't want the increased DLL size. It seems like VS2017 already supports this and with ctaggart/SourceLink it's quite easy to enable it.

Embedding the source directly seems like the perfect solution for internal libraries in scenarios where the DLL size does not really matter. It just sounds a lot more reliable and faster than having to authenticate and download files one by one. Also you don't have the issue with non-matching hashes due to different line endings etc.

Unfortunately, it seems like VS 2017 does not yet support stepping into embedded source files.

Given that this will happen (otherwise what would be the point of this feature?), I'd even like to see this integrated as a first-class parameter in dotnet build - e.g. with dotnet build --embed-source.

@ctaggart
Copy link
Contributor

ctaggart commented May 1, 2017

What if I made a SourceLink.EmbedAllSources nupkg that did what you are asking?

@tmat
Copy link
Member

tmat commented May 1, 2017

We are working on improving things. It goes slower than I'd like to but we are making progress.

Let me clarify a few things -- embedding sources doesn't increase DLL size unless the DLL also embeds PDB. Sources are embedded into PDB. PDB can still be a separate file. Yes, if sources are embedded and PDB is embedded then the DLL is gonna be bigger. I would not recommend that for general purpose libraries. Adding a source link also doesn't increase the size of DLL. Source link is in PDB and it's tiny. So even if PDB is embedded into DLL source link doesn't really add anything significant.

The plan is to include Portable PDB in nupkg by default at some point. The idea is to make it possible for the debugger to find the PDB in local nuget cache or on nuget server.

A few things the debugger is working on:

  1. Finish support for embedded PDBs (some features such as EnC are missing)
  2. Add support for source link authentication.
  3. Add support for extracting sources embedded into PDB
  4. Add support for line break normalization during source hashing (afaik VS Code debugger already does this).

@caslan - Please correct me if any of these items are not on your radar.

@cwe1ss
Copy link

cwe1ss commented May 1, 2017

@tmat - thanks for your response and for clarifying that sources go into the PDB. I used debugType=embedded so I didn't think about this.

The plan is to include Portable PDB in nupkg by default at some point. The idea is to make it possible for the debugger to find the PDB in local nuget cache or on nuget server.

👍 This would be amazing! Hope to see it one day!!

@jairbubbles
Copy link

@tmat Hi, I was wondering if you have plans on remote debugging. Right now if I'm not mistaken, pdb on the distant computer will be ignored. So including pdb in NuGet package is not enough in this case, you must put it on a symbol server which is a bummer as the idea behind having pdb in NuGet packages is NOT to have a symbol server. It's clearly less important but I guess you should include this scenario in the big picture!

Regarding @KirillOsenkov comment, in my company we'll clearly choose to include sources in pdb as soon as it's available. Just because it's faster and will work even if your network is down (everything else will stop working but anyway... 😄 ). We don't really care about package size as it's an internal NuGet server but we care about the debugging experience!

@tmat
Copy link
Member

tmat commented May 1, 2017

@jairbubbles The idea is that the debugger finds the PDB on the NuGet server.

@jairbubbles
Copy link

@tmat Interesting but how will it know what package to download? I've always thought that from a runtime perspective you can't really know if the assembly comes from a NuGet package or not. Do you plan to change that? If so it would be pretty cool and open opportunities.
Also, why does not the remote server serve the pdb locally ? It seems more universal. It's not like all assemblies come from a NuGet package (at least not yet!).

@gregg-miskelly
Copy link

gregg-miskelly commented May 1, 2017

@ctaggart
Copy link
Contributor

ctaggart commented May 2, 2017

I think the scope of this issue is about the /embed option to embed all source files. I created a SourceLink.Embed.AllSourceFiles nuget package as part of 2.1.1 just now. It works with both C# and F# projects which use different msbuild properties. It is documented here:
https://github.com/ctaggart/SourceLink#embedding-source-files

@cwe1ss Most of the issues you brought up are now listed in Known Issues. Source linking would be a success if the three of them were solved.
https://github.com/ctaggart/SourceLink#known-issues

@nguerrera
Copy link
Contributor

There should also be an easy way to embed only generated sources, for the scenario where you SourceLink the stuff in the repo and embed the stuff that's generated.

Here's one approach to that: consider anything in @(Compile) by the time CoreCompile runs that was not in @(Compile) statically as generated.

So we'd have

  • EmbedAllSourcesInPdb -> /embed
  • EmbedGeneratedSourcesInPdb -> /embed:(set described above)

@ctaggart
Copy link
Contributor

ctaggart commented Jun 8, 2017

There should also be an easy way to embed only generated sources, for the scenario where you SourceLink the stuff in the repo and embed the stuff that's generated.

@nguerrera That is what SourceLink.Create.GitHub will do.

I was expecting the Visual Studio 2017 debugger to be able to find embedded source files when it shipped. Is Debugger should support C# compiler '/embed' option now planned for 15.6?

@nguerrera
Copy link
Contributor

@ctaggart Cool. That's more robust as it knows what's in the repo and what's not. This would include. I withdraw the EmbedGeneratedSourceInPdb proposal.

@gregg-miskelly
Copy link

@ctaggart the scope for 15.6 hasn't been defined. So it isn't possible to say yet what will and will not make it.

@jinujoseph jinujoseph removed this from the 15.5 milestone Oct 7, 2017
@tmat tmat modified the milestones: Unknown, 15.6 Dec 8, 2017
@tmat
Copy link
Member

tmat commented Jan 23, 2018

Fixed by #23656

@jnm2
Copy link
Contributor

jnm2 commented Jan 23, 2018

@caslan

@jnm2, we have recently bumped up the priority of this work item based on user feedback. It is the next work item on our list that we are going to take on in this space (i.e. it should start in the next week or so). It is too early to share timeline / target VS release at this point. Hope this helps.

One of you had observed to me a while back that I had settled on embedding sources in the meantime. I do see that as a temporary workaround. Final nuget packages with source-embedded PDBs are three times the size of the final nuget packages with source-linked PDBs. Other than the size factor, it might be be my preference; internal URLs sometimes change and embedding the source (and putting the commit hash in the assembly info) makes a lot of sense. At 3x the size it's hard to sell though, and I need something that's easy to sell not just for my company but when I talk to third party vendors.

Q1: Is there anything that isn't already being done that could further reduce the size of embedded source in PDB blobs? I don't mind the compile taking twenty times longer if it halves the size.

Q2: With source-linked PDBs, you're planning the ability for the debugger to authenticate with the source control host. If the source control hosting base URL changes, would it make sense to be able to optionally enter a new URL at the same point where we enter authentication credentials?

@tmat
Copy link
Member

tmat commented Jan 23, 2018

Q1: No. The sources are already stored compressed.
Q2: If you change your repo URL you could set up forwarding to the new URL. The debugger will follow the forwarding.

@jnm2
Copy link
Contributor

jnm2 commented Jan 23, 2018

Q1 for example: compressing after appending all documents to a massive text buffer? Might give better results than compressing each document (or not) individually?
Q2: ✔

@tmat
Copy link
Member

tmat commented Jan 23, 2018

Q1: Perhaps marginally, but then the debugger would need to decompress the entire source in order to show a single file.

@jnm2
Copy link
Contributor

jnm2 commented Jan 23, 2018

Isn't decompression fast?

@tmat
Copy link
Member

tmat commented Jan 23, 2018

Is it?

@jnm2
Copy link
Contributor

jnm2 commented Jan 23, 2018

I've always heard that gzip is ridiculously fast, but we'd need parameters of what's acceptable to find how large the combined source can get without getting close to those parameters.

@moogle001
Copy link

Can someone please explain how to use this feature? There is no documentation on how one is supposed to turn on embedded source files through a project file. Looking through the various related issues, the work around stated in the original post seems to be the only solution, and Visual Studio debugger does not step into files embedded in that manner.

@jnm2
Copy link
Contributor

jnm2 commented Jan 31, 2018

Until VS 15.6 is released, the workaround at the top (or @ctaggart's EmbedAllSources nupkg) are the only ways to make this work. Only VS 15.5 and up understand embedded source.

@moogle001
Copy link

Thank you @jnm2; based on the close date for #23656 I thought it was already in Visual Studio. Now to figure out why VS 15.5 won't step into DLLs using the work around...

@gregg-miskelly
Copy link

@moogle001 are you getting a prompt to find source for the frame? Or does the debugger not step in at all? If the later, make sure have enabled "Suppress JIT optimization on module load", and if the dll is still optimized after that (because it loads before the debugger attached or is NGen'ed), turn off Just My Code as well. Then make sure symbols are actually loaded.

@moogle001
Copy link

@gregg-miskelly Yes, the debugger asks me to find the source file on disk. I currently have "Enable Just My Code" off, and tried turning on "Suppress JIT optimization on module load", to no avail.

This is what I have in my csproj:
<PropertyGroup> <DebugType>embedded</DebugType> <DebugSymbols>true</DebugSymbols> </PropertyGroup>

<Target Name="PopulateEmbeddedFiles" AfterTargets="BeforeCompile" BeforeTargets="CoreCompile"> <ItemGroup> <EmbeddedFiles Include="@(Compile)" /> </ItemGroup> </Target>

When trying to step into DLL, the source search information is:

Locating source for 'C:\Workspace...\JobFileConverter.cs'. (No checksum.)
The file 'C:\Workspace...\JobFileConverter.cs' does not exist.
Looking in script documents for 'C:\Workspace...\JobFileConverter.cs'...
Looking in the projects for 'C:\Workspace...\JobFileConverter.cs'.
The file was not found in a project.
Looking for source using source server...
The debugger will ask the user to find the file: C:\Workspace...\JobFileConverter.cs.
The user pressed Cancel in the Find Source dialog. The debug source files settings for the active solution have been modified so that the debugger will not ask the user to find the file: C:\Workspace...\JobFileConverter.cs.
The debugger could not locate the source file 'C:\Workspace...\JobFileConverter.cs'.

My assumption is that the source files are correctly embedded in the DLL because its size is good bit larger than when the work-around is not included. Regardless, I see nothing in the search information indicating it is even trying to find embedded source files.

@gregg-miskelly
Copy link

gregg-miskelly commented Jan 31, 2018

@moogle001 it sounds like the debugger doesn't believe your PDB has embedded sources for JobFileConvert.cs. Unfortunately I am not aware of a nice easy tool to check. You could try adding this to your project to make sure that the 'EmbeddedFiles' item group is getting populated after the 'Compile' item group.

 <Target Name="PopulateEmbeddedFiles"
          AfterTargets="BeforeCompile"
          BeforeTargets="CoreCompile">
    <ItemGroup>
      <EmbeddedFiles Include="@(Compile)" />
    </ItemGroup>
  </Target>

@gregg-miskelly
Copy link

@moogle001 actually wait, I think this is probably the problem -- (No checksum.).

Did you disable checksums in the IDE or did you turn off checksum generation in the compiler? I don't believe embedded sources are supported without checksums.

@ctaggart
Copy link
Contributor

Unfortunately I am not aware of a nice easy tool to check.

Try this:

dotnet sourcelink print-urls file.dll

@gregg-miskelly
Copy link

@ctaggart this is embedded sources, not source link. Will that print embedded sources too?

@ctaggart
Copy link
Contributor

ctaggart commented Jan 31, 2018

@gregg-miskelly Yes, it prints embedded instead of the URL.

@moogle001
Copy link

@gregg-miskelly Where would checksums be disabled in Visual Studio? I am not familiar with that aspect of the build process.

@ctaggart When I attempt to run the command you suggested I get 'No executable found matching command "dotnet-sourcelink"'; is SourceLink installed separately from VS? I have dotnet runtime and sdk installed.

@gregg-miskelly
Copy link

@moogle001 Make sure that 'Tools->Options->Debugging->Require source files to exactly match the original version' is checked.

@moogle001
Copy link

@gregg-miskelly That did it! Thank you so much. I had that off due to trying to work with symbol source servers, and had no idea it was related to embedded files.

@tmat
Copy link
Member

tmat commented Feb 1, 2018

@gregg-miskelly Why does the setting "Require source files to exactly match the original version" affect embedded sources? We need to reduce the friction.

@gregg-miskelly
Copy link

gregg-miskelly commented Feb 1, 2018

@tmat because the way we layout the embedded source file to a temp location requires checksum data. Maybe we could request checksum data even though the option is turned off, though I suspect very few people turn that option off.

@jnm2
Copy link
Contributor

jnm2 commented Feb 1, 2018

I've turned it off even just for code within the solution when a race condition happened between building and saving a new edit and I didn't feel like starting the debugging session from scratch.

(Such race conditions are more frequent than they used to be with .editorconfig settings being applied via direct save but not via the save that happens when you build.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests