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

Indexed source information could not be retrieved from Autofac.Extensions.DependencyInjection.pdb. Error: Symbol indexes could not be retrieved. #58

Closed
jeanluc33 opened this issue Oct 14, 2019 · 10 comments

Comments

@jeanluc33
Copy link

Describe the bug
This is the same as autofac/Autofac#995 but for Autofac.Extensions.DependencyInjection.

The PDB that is built is the "portable" one but we need the "Full" PDB for many TFS configurations (e.g. TFS 2012; other ticket said other TFS versions affected).

To reproduce
Build on older TFS server e.g. TFS 2012, which doesn't support portable PDBs for indexed sourcing.

Resolution
As with autofac/Autofac#995 just build the project setting PDB to full instead of portable.

@alsami
Copy link
Member

alsami commented Oct 14, 2019

Would you mind sending a pull-request for the changes required?

jeanluc33 pushed a commit to jeanluc33/Autofac.Extensions.DependencyInjection that referenced this issue Oct 14, 2019
…information to "Full" (has the effect of modying .csproj file to add <DebugType>full</DebugType> for each debug and release configurations.
@jeanluc33
Copy link
Author

jeanluc33 commented Oct 14, 2019

See pull request: #59

@tillig
Copy link
Member

tillig commented Oct 15, 2019

The issue in autofac/Autofac#995 sounded like it specifically was a problem with net45 targets, so the fix only happened with net45. PR #59 appears to modify netstandard2.0. That implies to me that one of the fixes is wrong - either the one for the core Autofac needed to target all configurations rather than just the one; or this fix isn't needed because it's not net45.

Either way, we need more research here.

What do the full Microsoft.Extensions.DependencyInjection libraries do? Let's do what they do. If they do this, great; if not, then what?

@jeanluc33
Copy link
Author

jeanluc33 commented Oct 15, 2019

Actually, it appears that Microsoft.Extensions.DependencyInjection avoids this issue by multi-targeting, i.e.:
$(DefaultNetCoreTargetFramework);net461;netstandard2.0;netstandard2.1

See: https://github.com/aspnet/Extensions/blob/master/src/DependencyInjection/DI/src/Microsoft.Extensions.DependencyInjection.csproj

Perhaps multi-targeting (e.g. including net461) would be a more appropriate solution?

@tillig
Copy link
Member

tillig commented Oct 15, 2019

Autofac core multi-targets and somehow didn't work, so I'm not sure that's the answer.

@jeanluc33
Copy link
Author

Okay, so Microsoft.Extensions.DependencyInjection works not because it multi-targets, but because the .pdb is not distributed in the nuget package., i.e. \microsoft.extensions.dependencyinjection\3.0.0\lib\net461 contains the .dll only and no .pdb.

So, it stands to reason that if a pdb is provided, a different fix is needed ("full" .pdb?).

@tillig
Copy link
Member

tillig commented Oct 15, 2019

Well, this is where it gets tricky.

I can't test any of this.

I can't verify what it is, or what it isn't. TFS 2012 mainstream support ended last year. TFS 2013 mainstream support ended this year. TFS 2015 is headed out this time next year.

I appreciate it's expensive for folks to migrate to new tools, but we also can't be pinned down to supporting issues for tools that aren't even supported by Microsoft.

Before we accept any more of these fixes, we need to 100% quantify which version(s) of TFS are affected and what the exact fix is. Distributing the PDB in the package is how SourceLink works, which helps folks step into libraries, and I'm fine with distributing the full PDB if that's necessary for the current supported set of tools. However, if it's just TFS 2012 and 2013 affected, that's not an interesting thing for us to fix.

In the meantime, I'm going to roll back the change to core Autofac since it sounds like that isn't guaranteed to fix it for everyone and we'll wait to hear more on the testing.

That may mean if you only have TFS 2012 available to you that you will have to solicit help from the community to find out if TFS 2013 is affected, or TFS 2015, or whatever. If you already know, awesome.

If you can find someone who can demonstrate this problem on TFS 2017 Update 3 then we'll fix it without question, likely by including full PDB for all target framework monikers to avoid issues with different app targets. TFS 2017 Update 3 is the lowest version of TFS at the moment with at least a year's worth of support left in it.

@tillig
Copy link
Member

tillig commented Oct 15, 2019

Oh, hang on, in the other issue, it appears it IS happening in 2017 Update 3.

Given that, it sounds like we just need to set debug full at the top level so it works for all configurations.

jeanluc33 pushed a commit to jeanluc33/Autofac.Extensions.DependencyInjection that referenced this issue Oct 15, 2019
…information to "Full" (has the effect of modying .csproj file to add <DebugType>full</DebugType> for each debug and release configurations.
@tillig
Copy link
Member

tillig commented Oct 16, 2019

We definitely need to do some more investigation here. Sorry I keep flip-flopping.

DebugType: full appears to have some impact on the JIT optimized code quality and it's recommended specifically that it's not used for release quality code.

PDB is included here to allow SourceLink to work.

Before we add this, let's see if there's some way we can follow up with the SourceLink team to see if they know what's going on here. I'm not against trying to come up with a fix, but I also want to be sure we're not adding a perf hit to everyone just for this situation.

I'm going to copy/paste this comment back to autofac/Autofac#995. Let's consolidate the discussion on researching these issues back on autofac/Autofac#995 rather than across these two issues and the PR. It's too hard to keep up.

Until we know what the actual solution is, I've closed #59. I'm leaving this issue open for follow up, but if it turns out we can't keep the discussion on research back in autofac/Autofac#995 then I'll close and lock this issue, then re-open/unlock. It'll just really facilitate getting the right thing done if we can discuss it in a single place. I appreciate everyone's help with this.

TO CONTINUE DISCUSSION ON HOW TO FIX THE ISSUE, PLEASE VISIT autofac/Autofac#995

We'll be back here when we figure out the right solution.

@tillig
Copy link
Member

tillig commented Feb 24, 2022

We haven't figured this issue out, but likely moving to .snupkg is the way to go. So that there's no confusion I'm going to close this issue and folks can continue following along on autofac/Autofac#995. Once we get it fixed up in core Autofac, it's a pattern we'll propagate through to the extension packages.

@tillig tillig closed this as completed Feb 24, 2022
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

3 participants