gluck / il-repack

Open-source alternative to ILMerge
Apache License 2.0
1.16k stars 214 forks source link

Better support for dropping attributes #345

Closed MWstudios closed 4 months ago

MWstudios commented 7 months ago

I need to drop multiple types with different attributes and I've noticed that due to RepackImporter.ShouldDrop<TMerge>(), ILMerge supports only one attribute. Why? I suggest supporting multiple attributes separated by a semicolon (this won't collide with any naming conventions and complement MSBuild item collections).

Also, I noticed that the code checks only for fields, methods, types, events and properties. Hence I suggest including module and assembly attributes as well.

KirillOsenkov commented 5 months ago

To clarify, are you looking to drop the members marked with an attribute or the attribute itself? Ideally if you could provide a tiny C# console app that demonstrates what you want and what your desired behavior is.

MWstudios commented 4 months ago

I'm thinking of this:

[Attribute1] //drop these
class A { }
[Attribute2] //also drop these
class B { }

If I want to drop both of these attributes, I need to run ILRepack twice with each configuration set to drop one of these two attributes. The bottleneck is in ILRepack/RepackOptions.cs at line 118:

public string RepackDropAttribute { get; set; }

And in ILRepack/RepackImporter.cs at line 248:

// skip members marked with a custom attribute named as /repackdrop:RepackDropAttribute
var shouldDrop = member.HasCustomAttributes
    && member.CustomAttributes.FirstOrDefault(attr => attr.AttributeType.Name == _options.RepackDropAttribute) != null;

If there was, say, string[] instead of string, ILRepack could effectively drop multiple attributes in one go. Since the argument is passed into the program by /repackdrop, I suggested separating multiple attributes via a semicolon. So, in ILRepack/RepackOptions.cs at line 286, this:

RepackDropAttribute = cmd.Option("repackdrop");

would be changed to this:

RepackDropAttribute = cmd.Option("repackdrop").Split(';');

Since semicolons are prohibited in C# namings, there would be no name conflicts in the process. As far as I'm aware there is nothing else stopping this from being implemented.

KirillOsenkov commented 4 months ago

Thanks, your answer doesn't clearly answer my question:

To clarify, are you looking to drop the members marked with an attribute or the attribute itself?

In other words, for

[Attribute1] //drop these
class A { }

do you expect just the attribute to be deleted, or class A as well?

MWstudios commented 4 months ago

I want to drop the members marked with the attribute, which is exactly what ILRepack does. But it only drops one attribute per run, so I need to create multiple configurations and them run them all in succession before building. I'm proposing a way to drop all members marked with at least one of the attributes provided. So, to drop both classes A and B, I would be able to write:

<ILRepack Internalize="false" InputAssemblies="$(OutputPath)DLL.dll" TargetKind="Dll" DebugInfo="true" OutputFile="$(OutputPath)DLL.dll" RepackDropAttribute="Attribute1;Attribute2"/>

instead of running ILRepack twice:

<ILRepack Internalize="false" InputAssemblies="$(OutputPath)DLL.dll" TargetKind="Dll" DebugInfo="true" OutputFile="$(OutputPath)DLL.dll" RepackDropAttribute="Attribute1"/>
<ILRepack Internalize="false" InputAssemblies="$(OutputPath)DLL.dll" TargetKind="Dll" DebugInfo="true" OutputFile="$(OutputPath)DLL.dll" RepackDropAttribute="Attribute2"/>
KirillOsenkov commented 4 months ago

Here you go: https://www.nuget.org/packages/ILRepack/2.0.32