dotnet / linker

388 stars 127 forks source link

Add option to create self-contained trimming repro #3056

Open sbomer opened 1 year ago

sbomer commented 1 year ago

We often get issues like https://github.com/dotnet/runtime/issues/75802 which are hard to reproduce without a very specific build setup. It would be helpful if we had an officially supported option that would create a self-contained repro (with all input assemblies and the linker commandn-line options) that can then be shared easily.

@vitek-karas I know you have created a tool like this - what would it take to make that officially supported?

vitek-karas commented 1 year ago

I don't think my tool should be the final solution. Better solution would be to make this part of the product. ilc already has something like this. Basically a new command line parameter (which specifies a path) which would:

The goal would be to get the target directory onto a different machine and just run illink repro.rsp from that directory and it works and behaves exactly as the original.

Some things will be a bit tricky, but all of it is doable (think custom steps and their custom command line options, maybe we don't need to support all of that right now).

When it's part of the product it's much more clear what it means to be "supported".

sbomer commented 1 year ago

Yes, I was really asking what it would take to make it part of the product. :) I agree it would be better as a linker option (rather than hooking in at the MSBuild layer).

MichalStrehovsky commented 1 year ago

Instead of building something completely from scratch, can we reuse the logic from crossgen2/ilc/ilverify here:.

https://github.com/dotnet/runtime/blob/ad7f31dbc7150ca3e9a5a3e8707550f96b193acc/src/coreclr/tools/Common/CommandLineHelpers.cs#L110

It could potentially become a shared file once illink moves to the runtime repo. The scheme is pretty similar.

It newly depends on System.CommandLine because everyone is jumping on that bandwagon for better or worse. But we could do ifdefs. Basically, it would be nice if we didn't have a completely separate copy and different ways of doing things (this already handles situations like "need to pack a file with the same name as another file, but from a different source path", etc.).

sbomer commented 1 year ago

It's looking like this would only get us so far for cases like https://github.com/dotnet/runtime/issues/75802 - there the custom steps have more dependencies than you might initially expect.

I'll write down the difficulties I encounter while trying to use @vitek-karas's repro tool to come up with a repro for that issue:

With that I'm at least able to run the linker and get some output assemblies.

vitek-karas commented 1 year ago

Yeah - I've ran into this myself in the past as well. The repro tool could copy the custom steps as well, but if those have dependencies... well still doable but gets more tricky. But if the custom steps take file paths as custom parameters, we would have to either have an extension point for the custom step to describe this somehow, or hardcode this into the tool about the custom steps we know about (technically we should know about all of them really). The last part is the macOS dependency - I looked into that once and there's no good way around it. One of the Xamarin custom steps calls a PInvoke into macOS libraries... there's basically nothing we can do about it - the repo has to run only on macOS.

vitek-karas commented 1 year ago

Overall I think this is still worth doing - and honestly I would not be against hardcoding knowledge about Xamarin and whatever MAUI or Blazor has into it. The size of linker itself is not a consideration. And this is diagnosability, which is basically always a best effort.