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

Support for portable PDBs #11

Open
jnm2 opened this issue May 4, 2017 · 17 comments

Comments

Projects
None yet
7 participants
@jnm2
Copy link
Contributor

commented May 4, 2017

With the new csproj format, the default is the superior portable PDB format for all .NET projects, including traditional .NET Framework apps.

ILMerge will crash with

An exception occurred during merging:
ILMerge.Merge:        There were errors reported in ReferencedProject's metadata.
      Array dimensions exceeded supported range.
   at ILMerging.ILMerge.Merge()
   at ILMerging.ILMerge.Main(String[] args)

as demonstrated by this simple test:
master...jnm2:portable_pdb_bug

This is somewhat of a integral issue. Where should this capability be on the roadmap?

@mike-barnett

This comment has been minimized.

Copy link
Collaborator

commented May 4, 2017

Hmm, I'm guessing that a different PDB reader would have to be used instead of the one that ILMerge/CCI currently uses, which is some COM object of type CorSymBinder2. Would you know if the new reader can handle both the portable PDB format and the old format? I.e., can we just switch to the new one and use that alone?

@jnm2

This comment has been minimized.

Copy link
Contributor Author

commented May 5, 2017

I wish I knew. Would it be possible for you to cc your colleagues for a recommendation?

@pharring

This comment has been minimized.

Copy link

commented May 5, 2017

@tmat for comment. I don't think there's a "universal" reader. You'd have two code-paths. However, there are tools that can convert between the formats so, in theory, one could build a universal reader.

@mike-barnett

This comment has been minimized.

Copy link
Collaborator

commented May 5, 2017

I'll also take a look at how CCI2 handles this. As a more general comment, I believe the direction ILMerge should take is to remove its dependence on the System.Compiler project (which is CCI1) and instead get rewritten against CCI2. Or we just move. I've already created an initial version . But it doesn't have any of the many options that this version has.

@tmat

This comment has been minimized.

Copy link
Member

commented May 5, 2017

What are the features ILMerge provides compared to Mono ILLinker?

@mike-barnett

This comment has been minimized.

Copy link
Collaborator

commented May 5, 2017

I don't know. I'd love it if there were a better tool so we could put ILMerge down for good. But I've had at least one person say they had tried some other Mono utility (could be that one) and that they preferred to stay with ILMerge. No accounting for taste... Why do you ask? Do you think ILLinker is as good or better than ILMerge? It looks from a cursory glance that they also do tree shaking, which is a nice thing.

@erozenfeld

This comment has been minimized.

Copy link
Member

commented May 5, 2017

ILLink does tree shaking but doesn't merge assemblies (yet) so the two tools do different things at the moment. Another difference is that ILLink uses Cecil, while ILMerge uses CCI for reading and writing assemblies.

@tmat

This comment has been minimized.

Copy link
Member

commented May 5, 2017

@mike-barnett I am asking since the tools are similar and I don't think we need both.

@tmat

This comment has been minimized.

Copy link
Member

commented May 5, 2017

@erozenfeld I'm aware of the difference in implementation (Cecil vs CCI). That's kind of my point. Cecil supports Portable PDBs already.

@erozenfeld

This comment has been minimized.

Copy link
Member

commented May 5, 2017

@tmat There are plans to support assembly merging in ILLink but it's not clear how soon that will happen.
/cc @russellhadley @swaroop-sridhar

@mike-barnett

This comment has been minimized.

Copy link
Collaborator

commented May 5, 2017

@tmat Totally agree! Hope my question did not sound confrontational! Getting the merging to work in ILLink sounds like a great thing, but I'm afraid I don't have the time to commit to doing that. But if there are people there that are working on it, then I will definitely drop the idea of moving ILMerge onto CCI2.

@tmat

This comment has been minimized.

Copy link
Member

commented May 5, 2017

@mike-barnett No worries. I just wanted to point out that rather then investing into ILMerge we could spend time on making ILLink better.

@jnm2

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2017

So in the meantime...

@tmat

This comment has been minimized.

Copy link
Member

commented May 6, 2017

@poizan42

This comment has been minimized.

Copy link

commented Sep 4, 2018

I'm surprised no one has mentioned this - an alternative to ILMerge is ILRepack. Once it has been migrated to use Mono.Cecil 0.10 it should work with portable pdbs (see gluck/il-repack#182, gluck/il-repack#230, gluck/il-repack#236)

@walterlv

This comment has been minimized.

Copy link

commented Jun 17, 2019

@poizan42 I've tested ILRepack from the master branch but the same exception happens.

image

The ILRepack has a 0.10 branch but the critical commits have never been merged to the master. So the main tool still does not support the portable pdbs. But if I checkout to the 0.10 branch, I get another exception of type InvalidOperationException.

@poizan42

This comment has been minimized.

Copy link

commented Jun 17, 2019

@walterlv I think we are still waiting for gluck/il-repack#236 to be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.