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 2020/03/05 14:23:06 UTC
[GitHub] [maven-shade-plugin] rmannibucau opened a new pull request #39:
[MSHADE-353] adding a generic relocation friendly transformer
rmannibucau opened a new pull request #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39
The generic transformer will actually delegate to other transformers for the actual processing to avoid to reimplement again and again the same logic.
//cc @rfscholte can you review it since you worked on the manifest flavor?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] tandraschko commented on issue #39:
[MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
tandraschko commented on issue #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#issuecomment-612351549
+1
It works fine
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] tandraschko commented on issue #39:
[MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
tandraschko commented on issue #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#issuecomment-595257807
tested and works fine: https://github.com/primefaces/primefaces/commit/9e8e2c3b81005f5f8ef8cd77c5a8a42449c8a327
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on issue #39:
[MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on issue #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#issuecomment-612349935
+1 to move forward, this feature will become important with jakartaee 9
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] rfscholte commented on a change in pull
request #39: [MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#discussion_r397480229
##########
File path: src/site/apt/examples/resource-transformers.apt.vm
##########
@@ -601,6 +603,45 @@ Transformers in <<<org.apache.maven.plugins.shade.resource>>>
</project>
+-----
+* Relocating file content with {RelocationTransformer}
+
+ The <<<RelocationTransformer>>> allows to apply relocators before delegating the processing to other transformers.
+
++-----
+<project>
+ ...
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <transformers>
+ <transformer implementation="org.apache.maven.plugins.shade.resource.RelocationTransformer">
Review comment:
Maybe I'm missing an important detail, but in this example is no relocation. So why do it like this?
And I would have expected that defining both transformers after each other would work, but that requires defining the resource twice. That's probably the thing you're trying to solve.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] rfscholte commented on a change in pull
request #39: [MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#discussion_r398803274
##########
File path: src/site/apt/examples/resource-transformers.apt.vm
##########
@@ -601,6 +603,45 @@ Transformers in <<<org.apache.maven.plugins.shade.resource>>>
</project>
+-----
+* Relocating file content with {RelocationTransformer}
+
+ The <<<RelocationTransformer>>> allows to apply relocators before delegating the processing to other transformers.
+
++-----
+<project>
+ ...
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <transformers>
+ <transformer implementation="org.apache.maven.plugins.shade.resource.RelocationTransformer">
Review comment:
Nested transformers looks more like a XML representation of the Java solution, and it might also expose a design flaw in the plugin.
It probably makes more sense to start with the resources, and define its transformers:
<resources>
<resource>
<includes/>
<excludes/>
<transformers/>
</resource>
</resources>
I understand that this is quite a different approach, but this looks way more clear to what's happening for the end user. Maybe the code can stay almost the same, but I would map this configuration to the actual transformer calls.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull
request #39: [MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#discussion_r397625820
##########
File path: src/site/apt/examples/resource-transformers.apt.vm
##########
@@ -601,6 +603,45 @@ Transformers in <<<org.apache.maven.plugins.shade.resource>>>
</project>
+-----
+* Relocating file content with {RelocationTransformer}
+
+ The <<<RelocationTransformer>>> allows to apply relocators before delegating the processing to other transformers.
+
++-----
+<project>
+ ...
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <transformers>
+ <transformer implementation="org.apache.maven.plugins.shade.resource.RelocationTransformer">
Review comment:
This example is about composing transformers which is not something obvious for most people.
Defining each one after the other does not work because the relocation transformer is a delagate pattern which only works if there is another transformer saying it what it should handle. Goal is not to not define resource twice but to be able to decorate any transformer with this feature, including the ones with dynamic resource handling (pattern in canTransform for ex).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull
request #39: [MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#discussion_r398844954
##########
File path: src/site/apt/examples/resource-transformers.apt.vm
##########
@@ -601,6 +603,45 @@ Transformers in <<<org.apache.maven.plugins.shade.resource>>>
</project>
+-----
+* Relocating file content with {RelocationTransformer}
+
+ The <<<RelocationTransformer>>> allows to apply relocators before delegating the processing to other transformers.
+
++-----
+<project>
+ ...
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <transformers>
+ <transformer implementation="org.apache.maven.plugins.shade.resource.RelocationTransformer">
Review comment:
Hmm, yes and no. It is foo/bar.txt cause it is an AppendingTransformer but the RelocatingTransformer is not aware of that detail. Think to a GraalVM transformer, it will create resources depending what is in the build so the files will not be configured statically. The RelocatingTransformer still works.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] rfscholte commented on a change in pull
request #39: [MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#discussion_r398830196
##########
File path: src/site/apt/examples/resource-transformers.apt.vm
##########
@@ -601,6 +603,45 @@ Transformers in <<<org.apache.maven.plugins.shade.resource>>>
</project>
+-----
+* Relocating file content with {RelocationTransformer}
+
+ The <<<RelocationTransformer>>> allows to apply relocators before delegating the processing to other transformers.
+
++-----
+<project>
+ ...
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <transformers>
+ <transformer implementation="org.apache.maven.plugins.shade.resource.RelocationTransformer">
Review comment:
Based on your test the input is `foo/bar.txt`, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull
request #39: [MSHADE-353] adding a generic relocation friendly transformer
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #39: [MSHADE-353] adding a generic relocation friendly transformer
URL: https://github.com/apache/maven-shade-plugin/pull/39#discussion_r398817676
##########
File path: src/site/apt/examples/resource-transformers.apt.vm
##########
@@ -601,6 +603,45 @@ Transformers in <<<org.apache.maven.plugins.shade.resource>>>
</project>
+-----
+* Relocating file content with {RelocationTransformer}
+
+ The <<<RelocationTransformer>>> allows to apply relocators before delegating the processing to other transformers.
+
++-----
+<project>
+ ...
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>shade</goal>
+ </goals>
+ <configuration>
+ <transformers>
+ <transformer implementation="org.apache.maven.plugins.shade.resource.RelocationTransformer">
Review comment:
Sorry @rfscholte , I don't get this proposal at all. You assume the resource configuration is an input of a transformer but a transformer can actually compute its resources and even create them on the fly without taking any input from the project sources so I don't see how it can work :s.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services