search

dotnet / jitutils

MIT License
144 stars 59 forks source link

Migrate four tools to commandline v2 #362

Closed am11 closed 1 year ago

am11 commented 1 year ago

Bring them to the same plan as https://github.com/dotnet/runtime/pull/72082 and https://github.com/dotnet/runtime/pull/76467.

Remaining two are jit-diff and jit-format. I have left them (for now ™️), as it turned out to be quite a feat to migrate jit-diff to this plan. 😁

jakobbotsch commented 1 year ago

Thank you! We will probably need to do a little testing with this before we can merge it.

am11 commented 1 year ago

Bumped version (based on https://github.com/dotnet/runtime/pull/78577).

jakobbotsch commented 1 year ago

Bumped version (based on https://github.com/dotnet/runtime/pull/78577).

Thanks! I hope to take a look at and do some testing of this PR this week.

jakobbotsch commented 1 year ago

The first problem I hit is that System.CommandLine.dll published into the bin dir gets overridden by the System.CommandLine.dll, so after ./bootstrap.cmd the executables output to bin that use the old System.CommandLine.dll do not work.

I'll see if there's an easy way to rename one of the DLLs

am11 commented 1 year ago

Publishing as single-file resolved the conflicting dependencies issue when publishing to the same directory (which is generally not recommended): 95056b2.

jakobbotsch commented 1 year ago

Publishing as single-file resolved the conflicting dependencies issue when publishing to the same directory (which is generally not recommended): https://github.com/dotnet/jitutils/commit/95056b2ab08f977e7ed6e9f0ed836bcc98542cc7.

That should be fine, but I think we need to skip it for PMI that is meant to be loaded into a runtime with a checked JIT.

am11 commented 1 year ago

I've skipped PMI from PublishSingleFile.

but I think we need to skip it for PMI that is meant to be loaded into a runtime with a checked JIT.

Just curious, what prevents it from working with singlefilehost? Do we unload/reload the corelib which is not possible with single-file mode or something else?

jakobbotsch commented 1 year ago

Just curious, what prevents it from working with singlefilehost? Do we unload/reload the corelib which is not possible with single-file mode or something else?

The idea of PMI is to execute it with a custom build of the runtime/JIT. It then loads a bunch of assemblies and forcefully JITs lots of functions, which allows producing the disassembly for these functions so that it can be diffed and compared. So we generally don't want to use it with the public runtime that single file would entail.

BTW, sorry for the delay here -- I hope to test this change more asap.

(EDIT: Actually I just realized I've been using this PR for my personal jitutils install for the past 2 weeks without issues, so that's a good sign 😄)

jakobbotsch commented 1 year ago

FWIW, jit-dasm-pmi can be singlefile. Only pmi needs to be framework dependent.

am11 commented 1 year ago

Thanks for testing it @jakobbotsch. :)

I have updated the condition to only exclude src/pmi from PublishSingleFile.

am11 commented 1 year ago

Thanks for the fixes, @jakobbotsch!

If possible, I think it would be nice to move jitutils under https://github.com/dotnet/runtime/tree/main/src/tools and use live build in the CI; diffs legs, for example, can build these tools spending two (or less) additional minutes per run. That will save round-trips and keep things up-to-date with convenience.

Edit: it currently clones this repo and use the live build, so moving code to runtime repo will actually save a few seconds. 😅

jakobbotsch commented 1 year ago

Thanks for the fixes, @jakobbotsch!

No problem, thanks for the help! Sorry for the slowness. I think this is good to go now, I did some last tests and things look ok for me locally now.

If possible, I think it would be nice to move jitutils under https://github.com/dotnet/runtime/tree/main/src/tools and use live build in the CI; diffs legs, for example, can build these tools spending two (or less) additional minutes per run. That will save round-trips and keep things up-to-date with convenience.

Edit: it currently clones this repo and use the live build, so moving code to runtime repo will actually save a few seconds. 😅

It has been discussed quite a few times, but so far it has not amounted to something we want to spend time on. There are both good arguments for and against, and if we did it we would probably only move some of the tools since this repo serves as kind of a kitchen sink for a lot of very unpolished JIT tools that we don't want to bloat dotnet/runtime with.