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

Rethink the Microsoft.NETCore.ILAsm NuGet package #9280

Closed
MichalStrehovsky opened this issue Nov 16, 2017 · 8 comments
Closed

Rethink the Microsoft.NETCore.ILAsm NuGet package #9280

MichalStrehovsky opened this issue Nov 16, 2017 · 8 comments
Labels
area-Infrastructure-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@MichalStrehovsky
Copy link
Member

It's really unfortunate that to consume ilasm.exe from the Microsoft.NETCore.ILAsm NuGet package, we need to download 24 MB of garbage* that is Microsoft.NETCore.Runtime.CoreCLR because ILAsm depends on MetaDataGetDispenser API exported from coreclr.dll. We should explore static linking or a different factoring of the nugets.

* If the only thing you need is a working ilasm.exe and don't care about the 20+ MB of System.Private.CoreLib, DAC, SOS, JIT, host, etc.

@MichalStrehovsky
Copy link
Member Author

Motivated by dotnet/buildtools#1806

@jaredpar
Copy link
Member

Just want to give my 👍 to this bug. This is so painful for us to maintain in Roslyn. We just want the simple ilasm -> il transformation. Keeping the runtime in sync here is a tax. I'd love it if the package and deployment was much simpler.

@jaredpar
Copy link
Member

Curious: what is the cost of doing this? Most recently I've spent two days trying to deploy ILASM. When I think back about the total time I've spent just deploying this simple native tool I just get sad. Maybe my time is better spent just fixing this.

jaredpar referenced this issue in jaredpar/roslyn Mar 27, 2019
The project which deploys ILASM tools ends up bringing two runtime
packages that have the same assets:

- runtime.win-x64.microsoft.netcore.runtime.coreclr
- runtime.win-x64.microsoft.netcore.app

Specifically assets like SOS.NetCore.dll exists at different versions in
these packages and end up getting copied twice to the output directory.
This double write ends up breaking our build (as well as basic
isolation).

The short term fix here is to no longer treat this as a code project but
instead a tools project. That eliminates the MS.NetCore.app package and
the associated double writes

Long term though the root issue needs to be addressed: making ilasm and
ildasm easier to deploy.

https://github.com/dotnet/coreclr/issues/15059
@AaronRobinsonMSFT
Copy link
Member

@jaredpar I think this should be a straight forward problem to solve. On Windows we are linking against coreclr, but not on Unix so I assume if we play with the linker flags a bit we could get this to just work.

if(CLR_CMAKE_PLATFORM_UNIX)
  target_link_libraries(ilasm
    ${ILASM_LINK_LIBRARIES}
    ceefgen
    unixcoreclrloader
    utilcodestaticnohost
    mscorrc_debug
    coreclrpal
    palrt
  )

  # FreeBSD and NetBSD implement dlopen(3) in libc
  if(NOT CMAKE_SYSTEM_NAME STREQUAL FreeBSD AND NOT CMAKE_SYSTEM_NAME STREQUAL NetBSD)
    target_link_libraries(ilasm
      dl
    )
  endif(NOT CMAKE_SYSTEM_NAME STREQUAL FreeBSD AND NOT CMAKE_SYSTEM_NAME STREQUAL NetBSD)
else()
  target_link_libraries(ilasm
    ${ILASM_LINK_LIBRARIES}
    coreclr
    ole32
    oleaut32
    shell32
  )
endif(CLR_CMAKE_PLATFORM_UNIX)

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Apr 25, 2019

Oh.. nevermind. CoreCLR is dynamically loaded instead. Boo...

@MichalStrehovsky
Copy link
Member Author

As of dotnet/coreclr#25144, xcopy ilasm.exe a:\path\where\you\want\it is enough to get a working ilasm!

@jkoritzinsky
Copy link
Member

I'll take a look at updating the ilasm and ildasm packages as well as the Microsoft.NET.Sdk.IL now that they're self-contained.

@jkoritzinsky
Copy link
Member

See dotnet/coreclr#25930 for the updates to the packages/sdk.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

5 participants