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

Migrate to vanilla 0.10 cecil #236

Open
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
@Alexx999
Copy link
Contributor

commented Aug 31, 2018

Here's my take on the topic.
In order to use completely vanilla cecil I've moved Win32 resource handling into a separate step that includes minimal PE reader/writer based on cecil implementation.

@Alexx999 Alexx999 force-pushed the Alexx999:cecil-unfork branch 2 times, most recently from a684454 to 3c7a72c Sep 1, 2018

@Alexx999 Alexx999 force-pushed the Alexx999:cecil-unfork branch 2 times, most recently from dae7986 to b73ff07 Sep 4, 2018

@epeshk

This comment has been minimized.

Copy link
Contributor

commented Oct 10, 2018

When it will be merged? I want to build ILRepack for .NET Core, which allows run ILRepack on Linux without Mono. It requires updated Cecil.
@gluck

@Alexx999

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2018

@epeshk if you wish you can use avostres.ILRepack package as a stopgap solution - it includes this PR

@epeshk

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2018

But it's so cool to have latest features and improvements in the mainline, right?

@Alexx999

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2018

Yup, that's why this pull request exists in the first place :)

@epeshk epeshk referenced this pull request Nov 12, 2018

Open

CoreCLR support #238

@iskiselev

This comment has been minimized.

Copy link

commented Dec 1, 2018

With this PR ILRepack fails if output directory was not created with:
System.IO.DirectoryNotFoundException: Could not find a part of the path.
at System.IO.FileSystem.MoveFile(String sourceFullPath, String destFullPath)
at System.IO.File.Move(String sourceFileName, String destFileName)
at ILRepacking.ILRepack.MoveTempFile(String tempFile, String outFile)
at ILRepacking.ILRepack.Repack()
at ILRepacking.Application.Main(String[] args)

@Alexx999 Alexx999 force-pushed the Alexx999:cecil-unfork branch 3 times, most recently from 6f81173 to 38432ba Jan 7, 2019

@Alexx999

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

AppVeyor builds are now failing even with Previous 2017 version - there's problem with ProjectFileParser component

@peters

This comment has been minimized.

Copy link

commented Jan 10, 2019

@Alexx999 Great work! 👍 I have a working .netcoreapp 2.1, netstandard2.0, net45 build based of https://github.com/peters/il-repack/tree/netcoreapp2.2 and published at https://www.nuget.org/packages/ILRepack.MSBuild.Task/2.0.0

@timotei

This comment has been minimized.

Copy link
Collaborator

commented Jan 13, 2019

@Alexx999 Yes, there is a bug that happens with latest VS, there is a fix in progress: Itiviti/gradle-msbuild-plugin#105

@epeshk

This comment has been minimized.

Copy link
Contributor

commented Jan 13, 2019

Maybe it's possible to just drop gradle? Looks like it used only for nuspec generation

@peters

This comment has been minimized.

Copy link

commented Jan 13, 2019

A better solution would just to use the new dotnet tooling and drop gradle. I've already done this here:
https://github.com/peters/ILRepack.MSBuild.Task/blob/master/build.cmd

Most of the "build system" has been moved to ILRepack.csproj
https://github.com/peters/ILRepack.MSBuild.Task/blob/master/src/ILRepack.MSBuild.Task/ILRepack.MSBuild.Task.csproj.

@Alexx999 Alexx999 force-pushed the Alexx999:cecil-unfork branch 2 times, most recently from 7d822c6 to 9aa5ad4 Jan 21, 2019

@caozhiyuan

This comment has been minimized.

Copy link

commented Mar 31, 2019

@Alexx999 strongname has this problem peters/ILRepack.MSBuild.Task#37 (comment) ,
and when cecil 0.11.0 release will supoort linux strongname sign.

@odhanson

This comment has been minimized.

Copy link

commented Apr 3, 2019

A better solution would just to use the new dotnet tooling and drop gradle. I've already done this here:
https://github.com/peters/ILRepack.MSBuild.Task/blob/master/build.cmd

Most of the "build system" has been moved to ILRepack.csproj
https://github.com/peters/ILRepack.MSBuild.Task/blob/master/src/ILRepack.MSBuild.Task/ILRepack.MSBuild.Task.csproj.

@peters do you have any plan to make a PR for your change ?

@peters

This comment has been minimized.

Copy link

commented Apr 3, 2019

I'm sorry guys but I'm caught up in work stuff so unfortunately I will not have the time to upstream any of my changes.

@timotei

This comment has been minimized.

Copy link
Collaborator

commented May 4, 2019

@Alexx999 Can you rebase/merge with latest master, so that the build works properly?

gluck and others added some commits Aug 1, 2016

Migrate to cecil 0.10
- fixed handle leaks
- removed duplicate assembly definition resolving (perf++)
- moved debug information to new API
- pretty-much untested yet, but UTs look ok

@Alexx999 Alexx999 force-pushed the Alexx999:cecil-unfork branch from 25a8ae9 to f5831d9 May 5, 2019

@Alexx999

This comment has been minimized.

Copy link
Contributor Author

commented May 5, 2019

@timotei that's done, I've removed all build-related changes from this PR (except for removed submodule init step).

@kzu

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

@Alexx999

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

@kzu that tool is very weird - it looks like it patches some ancient ILRepack version with fixes included in newer one (and "newer" is quite old, mind you).
It even references commits in comments, see here and here.
The last spot patches some Mono.Unix stuff - most likely this isn't fixed here yet. I guess when (if?) this repo goes alive and starts actually accepting PRs quicker than "once in few years" it should be properly migrated to .NET Core. I'd probably do it myself but not before this PR is accepted :)

@richard-fine

This comment has been minimized.

Copy link

commented May 31, 2019

Just to confirm my understanding, will this PR mean that ILRepack supports portable PDB fully on all platforms?

@Alexx999

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2019

@richard-fine cecil 0.10 does include support for portable PDBs and it's enabled in this PR

@tephe

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

@Alexx999 I'm taking a look at this and have found one regression for ikvm'ed dlls and another issue with the memory consumption when we merge 70 dlls. They are both linked here.
Otherwise it looks good IMO.

@kkwpsv

This comment has been minimized.

Copy link

commented Jun 20, 2019

There're some problems with breakpoints in asynchronous code when debugging in Visual Studio with portable pdb created by Mono.Cecil 0.10.1. I tried upgrade Mono.Cecil 0.10.4, it seems to be fixed. So, I think Mono.Cecil should be upgraded.

But there're still problems with local variables when debugging.

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.