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

Implements #215 #221

Merged
merged 7 commits into from
Mar 12, 2018
Merged
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ _NCrunch*
*.ncrunchproject
*.sln.cache
*.DotSettings
/.vs
1 change: 1 addition & 0 deletions ILRepack/Application.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ static void Usage()
Console.WriteLine(@" - /align - NOT IMPLEMENTED");
Console.WriteLine(@" - /closed - NOT IMPLEMENTED");

Console.WriteLine(@" - /repackdrop:RepackDropAttribute allows dropping members denoted by this attribute name when merging");
Console.WriteLine(@" - /allowdup:Type allows the specified type for being duplicated in input assemblies");
Console.WriteLine(@" - /allowduplicateresources allows to duplicate resources in output assembly (by default they're ignored)");
Console.WriteLine(@" - /zeropekind allows assemblies with Zero PeKind (but obviously only IL will get merged)");
Expand Down
44 changes: 29 additions & 15 deletions ILRepack/RepackImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,11 @@ public MethodReference Import(MethodReference reference, IGenericParameterProvid

public TypeDefinition Import(TypeDefinition type, Collection<TypeDefinition> col, bool internalize)
{
_logger.Verbose("- Importing " + type);
if (CanInclude(type) == false) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current code uses spaces (4 spaces) instead of tab, let's keep consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer tabs in all my projects.
Can you upload an editor config file to ensure that spacing consistency (as well as others) so I can follow it?

_logger.Log("Repack dropped " + type.FullName);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also logged in the CanInclude method, I'd say we only log once here, and rewrite it as; Type <X> dropped as it was marked with the drop attribute

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I forgot that CanInclude also logged that. Shall I simply remove the log here?

return null;
}
_logger.Verbose("- Importing " + type);

TypeDefinition nt = _repackContext.TargetAssemblyMainModule.GetType(type.FullName);
bool justCreatedType = false;
Expand Down Expand Up @@ -156,31 +160,41 @@ public TypeDefinition Import(TypeDefinition type, Collection<TypeDefinition> col
_repackContext.MappingHandler.StoreRemappedType(type, nt);

// nested types first (are never internalized)
foreach (TypeDefinition nested in type.NestedTypes)
foreach (TypeDefinition nested in type.NestedTypes.Where(CanInclude))
Import(nested, nt.NestedTypes, false);
foreach (FieldDefinition field in type.Fields)
foreach (FieldDefinition field in type.Fields.Where(CanInclude))
CloneTo(field, nt);

// methods before fields / events
foreach (MethodDefinition meth in type.Methods)
foreach (MethodDefinition meth in type.Methods.Where(CanInclude))
CloneTo(meth, nt, justCreatedType);

foreach (EventDefinition evt in type.Events)
foreach (EventDefinition evt in type.Events.Where(CanInclude))
CloneTo(evt, nt, nt.Events);
foreach (PropertyDefinition prop in type.Properties)
foreach (PropertyDefinition prop in type.Properties.Where(CanInclude))
CloneTo(prop, nt, nt.Properties);
return nt;
}

// Real stuff below //
// These methods are somehow a merge between the clone methods of Cecil 0.6 and the import ones of 0.9
// They use Cecil's MetaDataImporter to rebase imported stuff into the new assembly, but then another pass is required
// to clean the TypeRefs Cecil keeps around (although the generated IL would be kind-o valid without, whatever 'valid' means)

/// <summary>
/// Clones a field to a newly created type
/// </summary>
private void CloneTo(FieldDefinition field, TypeDefinition nt)
private bool CanInclude<TMember>(TMember member) where TMember : ICustomAttributeProvider, IMemberDefinition {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be renamed to ShouldDropForRepacking?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I renamed that method and use an if in each place to filter the result where CloneTo was called

// skip members marked with a custom attribute named "RepackDropAttribute"(by default)
var marked = member.HasCustomAttributes
&& member.CustomAttributes.FirstOrDefault(attr => attr.AttributeType.Name == _options.RepackDropAttribute) != null;
if (marked) {
_logger.Log("Repack dropped " + member.FullName);
}
return marked == false;
}

// Real stuff below //
// These methods are somehow a merge between the clone methods of Cecil 0.6 and the import ones of 0.9
// They use Cecil's MetaDataImporter to rebase imported stuff into the new assembly, but then another pass is required
// to clean the TypeRefs Cecil keeps around (although the generated IL would be kind-o valid without, whatever 'valid' means)

/// <summary>
/// Clones a field to a newly created type
/// </summary>
private void CloneTo(FieldDefinition field, TypeDefinition nt)
{
if (nt.Fields.Any(x => x.Name == field.Name))
{
Expand Down
11 changes: 9 additions & 2 deletions ILRepack/RepackOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ public List<string> AllowedDuplicateNameSpaces
get { return allowedDuplicateNameSpaces; }
}

public string RepackDropAttribute { get; set; }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent is off by 1 here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you are right. That file needs reformatting.


private readonly Hashtable allowedDuplicateTypes = new Hashtable();
private readonly List<string> allowedDuplicateNameSpaces = new List<string>();
private readonly List<Regex> excludeInternalizeMatches = new List<Regex>();
Expand Down Expand Up @@ -224,8 +226,13 @@ void Parse()
LogVerbose = cmd.Modifier("verbose");
LineIndexation = cmd.Modifier("index");

// everything that doesn't start with a '/' must be a file to merge (verify when loading the files)
InputAssemblies = cmd.OtherAguments;
RepackDropAttribute = cmd.Option("repackdrop");
if (String.IsNullOrWhiteSpace(RepackDropAttribute)) {
RepackDropAttribute = "RepackDropAttribute";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always drop the types flagged with the attribute, I think we want to only do this if the option is enabled. (/repackdrop or /repackdrop:attribute)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I agree with that.

}

// everything that doesn't start with a '/' must be a file to merge (verify when loading the files)
InputAssemblies = cmd.OtherAguments;
}

/// <summary>
Expand Down