InternalizeExclude implemented incorrectly

[pinkfloydx33]

Internalize Exclude expects a file that has a list (one per line) of Regular expressions. These patterns are used to match namespaces/classes in the combined assemblies which should not be internalized.

At the moment, ILRepack.cs attempts to correlate the InternalizeExcludes with the Assemblies. The assembly information should have nothing to do with these. For example, I may be packing 3 DLLs but I may be excluding 10 different regex/namespaces. At the moment, the loop over InternalizeExclude attempts to access the array of assemblies using the same index. This will crash the build if we're on exclude number 4 with 3 assemblies.

https://github.com/ravibpatel/ILRepack.Lib.MSBuild.Task/blob/45fbbda7b24643cdbd7e7e5d996a71c25958f954/ILRepack.Lib.MSBuild.Task/ILRepack.cs#L311

That if statement (line 311-314) should be removed. The Log message at line 315 should be updated to indicate that "pattern ^XYZ$ is being excluded.". The exception message at 309 should probably be updated to say something like: "Invalid Exclude Pattern at Index {i}; Pattern cannot be blank"

[ravibpatel]

@pinkfloydx33 Thanks for reporting it. You are right. If you want to open a pull request with the fix then please do it. I will merge it.

[pinkfloydx33]

I'm ashamed to admit that I do not know how to use git. Otherwise I would be happy to make the fix.

[ravibpatel]

@pinkfloydx33 I can do it but if you do it, it will make you a contributor in the repo. If you like to know how to use it, you can follow this tutorial from here. You just need to fork the repo to your account and then open it in Visual Studio change it and open pull request by logging into GitHub.

[pinkfloydx33]

@ravibpatel Ok, I think I did it correctly. Originally I had just cloned the repository and then branched it but I wasn't sure if it was "proper" to commit my own branch into your repo. I ended up Forking like you suggested and I think I've done it right.

Once the PR is complete/merged, what do I do with the fork? Can I remove it? Or do I need to keep it around for all eternity? (Sorry, I know I sound like such a novice...)

[pinkfloydx33]

@ravibpatel Actually -- the CI build has failed; Looks like an issue with the secret key. What did I do wrong?

[ravibpatel]

@pinkfloydx33 There is nothing wrong on your part. Don't worry it only succeeds when I do the commit. I didn't fix it and yeah, I think you can remove fork after I merge it. I keep release name parallel with gluck/il-repack repo. Can you change it to 2.0.15.5?

[pinkfloydx33]

Yeah sure. Do I have to create a whole new pull request or is it possible for me to modify the existing one after I commit the changes into my fork? I'll see if I can figure it out :)

On Tue, Jun 26, 2018, 6:04 AM Ravi Patel notifications@github.com wrote:

@pinkfloydx33 https://github.com/pinkfloydx33 There is nothing wrong on your part. Don't worry it only succeeds when I do the commit. I didn't fix it and yeah, I think you can remove fork after I merge it. I keep release name parallel with gluck/il-repack https://github.com/gluck/il-repack/releases repo. Can you change it to 2.0.15.5?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ravibpatel/ILRepack.Lib.MSBuild.Task/issues/9#issuecomment-400253534, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYJX2WfZ7AAo1lccFk3gqpvommqNJ5mks5uAgcygaJpZM4U2ECl .

[ravibpatel]

I think you can commit it to your fork and it should be visible in the pull request.

[pinkfloydx33]

Ok. Done!

[ravibpatel]

@pinkfloydx33 Thanks. I merged it. I will tag it so it will be available on NuGet soon.

[ravibpatel]

@pinkfloydx33 I pushed it to NuGet. Thanks again for the fix.