You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/07/12 08:46:20 UTC
[GitHub] [maven-shade-plugin] kriegaex opened a new pull request #105: [MSHADE-396] Improve SourceContent Shading
kriegaex opened a new pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105
Improve search & replace heuristics without destroying previously correct replacements in my test project (AspectJ). This solution is still bound to fail in some situations, simply because it is just a heuristic approach and not a full Java parser correctly recognising package names in all possible situations in Java source code. As for matching within Java string constants, this is next to impossible to get 100% right.
But the source shading feature is not meant as a source code generator anyway, merely as a tool creating reasonably plausible source code when navigating to a relocated library class from an IDE, hopefully displaying source code which makes 95% sense - no more, no less.
Supersedes #100 and fixes regression a few bugs I found with the new approach.
---
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Reamer commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882517785
Hi @kriegaex
thanks for improving the shading of the source code.
I have tested your version and found an error.
I have corrected this error with the following change.
https://github.com/apache/maven-shade-plugin/pull/100/files#diff-293098625e786122aa8c8eff5e7910b351fe9e777c90e5bdb6dcf2942efa66a6L123-L125
```patch
// Excludes should be subpackages of the global pattern
- else if ( exclude.startsWith( pathPattern ) )
+ if ( exclude.startsWith( pathPattern ) )
{
```
To test, you need to apply the following lines.
```patch
SimpleRelocator relocatorPackage = new SimpleRelocator( "io", "shaded.io", null, null);
SimpleRelocator asmPackage = new SimpleRelocator( "org.objectweb.asm", "aj.org.objectweb.asm", null, null);
+ SimpleRelocator relocatorExlucdeSubPackage = new SimpleRelocator( "foo", "shaded.foo", null , Arrays.asList( "foo.bar"));
assertEquals(
relocatedFile,
- asmPackage.applyToSourceContent(
- relocatorPackage.applyToSourceContent(
- relocator.applyToSourceContent( sourceFile )
+ relocatorExlucdeSubPackage.applyToSourceContent(
+ asmPackage.applyToSourceContent(
+ relocatorPackage.applyToSourceContent(
+ relocator.applyToSourceContent( sourceFile )
+ )
)
)
);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] kriegaex commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882950852
@Reamer
> I have tested your version and found an error.
Good catch! Because your previous change did not have an obvious test case, I did not understand why my code was wrong in this corner case. I have fixed it and added your test case, then squashed the change into the first commit.
Barring a positive code review, I think this is ready to be merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Reamer commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-899393908
> Do you want to squash commits before we merge or not (personally I don't care at all but asking before hitting merge ;))?
I can not decide this. But I think that @kriegaex will quickly comment on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] rmannibucau merged pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
rmannibucau merged pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Tibor17 commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882895404
@rmannibucau Would you pls review this PR too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] kriegaex commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882950852
@Reamer
> I have tested your version and found an error.
Good catch! Because your previous change did not have an obvious test case, I did not understand why my code was wrong in this corner case. I have fixed it and added your test case, then squashed the change into the first commit.
Barring a positive code review, I think this is ready to be merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] kriegaex commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882950852
@Reamer
> I have tested your version and found an error.
Good catch! Because your previous change did not have an obvious test case, I did not understand why my code was wrong in this corner case. I have fixed it and added your test case, then squashed the change into the first commit.
Barring a positive code review, I think this is ready to be merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Reamer commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882517785
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] kriegaex commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-895314732
@Reamer, I am not sure, not being a Maven committer. Maybe we need more than one approved review. But no other reviewer signed up for this yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] kriegaex commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
kriegaex commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-899398962
IMO, more and smaller commits are preferable. Squashing in many cases is some kind of information hiding. But I am aware of the fact that Tibor is a decided proponent of squashing (one PR, one commit). I could not disagree more, but I guess that if he wants it squashed, he will do so anyway, like he did before with another recent PR of mine. In any case, we can all say our opinions, but in the end the person responsible for the merge has the final say and we have to respect whatever he or she does. So let's not make it a big debate. 🙂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] rmannibucau commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-899372875
Do you want to squash commits before we merge or not (personally I don't care at all but asking before hitting merge ;))?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Tibor17 commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882895404
@rmannibucau Would you pls review this PR too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Tibor17 commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882895404
@rmannibucau Would you pls review this PR too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Reamer commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-899359811
@rmannibucau and @Tibor17
Can you help merge this into Master?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Reamer commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-883162125
> Barring a positive code review, I think this is ready to be merged.
I have checked the generated source code and found no error. PR looks good to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Reamer commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-882517785
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] Reamer commented on pull request #105: [MSHADE-396] Improve SourceContent Shading
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #105:
URL: https://github.com/apache/maven-shade-plugin/pull/105#issuecomment-895151961
What is blocking a merge?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org