You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "jbertram (via GitHub)" <gi...@apache.org> on 2024/10/10 17:19:23 UTC
[PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
jbertram opened a new pull request, #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289
(no comment)
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289#discussion_r1801559040
##########
artemis-distribution/src/main/assembly/dep.xml:
##########
@@ -72,9 +72,18 @@
<includes>
<include>org.apache.activemq:artemis-console:war</include>
</includes>
- <outputDirectory>web</outputDirectory>
- <unpack>false</unpack>
- <outputFileNameMapping>console.war</outputFileNameMapping>
+ <outputDirectory>web/console.war</outputDirectory>
+ <unpack>true</unpack>
+ <!-- Exclude these jar files from console since they are already provided in the broker distribution.
+ If any of these jars is dropped from the broker distribution they should be added back to the console. -->
+ <unpackOptions>
+ <excludes>
+ <exclude>**/guava*.jar</exclude>
+ <exclude>**/commons-io*.jar</exclude>
+ <exclude>**/commons-logging*.jar</exclude>
+ <exclude>**/failureaccess*.jar</exclude>
+ </excludes>
+ </unpackOptions>
Review Comment:
I'm testing it locally to check what is the impact for users that want to replace console.war with the hawtio.war.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289#discussion_r1801594707
##########
artemis-distribution/src/main/assembly/dep.xml:
##########
@@ -72,9 +72,18 @@
<includes>
<include>org.apache.activemq:artemis-console:war</include>
</includes>
- <outputDirectory>web</outputDirectory>
- <unpack>false</unpack>
- <outputFileNameMapping>console.war</outputFileNameMapping>
+ <outputDirectory>web/console.war</outputDirectory>
+ <unpack>true</unpack>
+ <!-- Exclude these jar files from console since they are already provided in the broker distribution.
+ If any of these jars is dropped from the broker distribution they should be added back to the console. -->
+ <unpackOptions>
+ <excludes>
+ <exclude>**/guava*.jar</exclude>
+ <exclude>**/commons-io*.jar</exclude>
+ <exclude>**/commons-logging*.jar</exclude>
+ <exclude>**/failureaccess*.jar</exclude>
+ </excludes>
+ </unpackOptions>
Review Comment:
It LGTM and the exploded war provides an easy way to customize files like web.xml
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr merged PR #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289#discussion_r1799957257
##########
artemis-hawtio/artemis-console/pom.xml:
##########
@@ -76,9 +76,14 @@
<artifactId>maven-war-plugin</artifactId>
<configuration>
<useCache>true</useCache>
+ <!-- exclude these jars as they are already shipped with the broker -->
<packagingExcludes>
**/lib/slf4j*.jar,
- **/lib/log4j*.jar
+ **/lib/log4j*.jar,
+ **/lib/guava*.jar,
+ **/lib/commons-io*.jar,
+ **/lib/commons-logging*.jar,
+ **/lib/failureaccess*.jar
Review Comment:
I changed up the strategy. Instead of dealing with this when building `console.war` I am dealing with it when creating the broker distribution. That should eliminate any problems with non-assembly use-cases.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289#discussion_r1801539531
##########
artemis-distribution/src/main/assembly/dep.xml:
##########
@@ -72,9 +72,18 @@
<includes>
<include>org.apache.activemq:artemis-console:war</include>
</includes>
- <outputDirectory>web</outputDirectory>
- <unpack>false</unpack>
- <outputFileNameMapping>console.war</outputFileNameMapping>
+ <outputDirectory>web/console.war</outputDirectory>
+ <unpack>true</unpack>
+ <!-- Exclude these jar files from console since they are already provided in the broker distribution.
+ If any of these jars is dropped from the broker distribution they should be added back to the console. -->
+ <unpackOptions>
+ <excludes>
+ <exclude>**/guava*.jar</exclude>
+ <exclude>**/commons-io*.jar</exclude>
+ <exclude>**/commons-logging*.jar</exclude>
+ <exclude>**/failureaccess*.jar</exclude>
+ </excludes>
+ </unpackOptions>
Review Comment:
To be clear, there are some minor differences under the covers, but unexploded and exploded war files are functionally equivalent in Jetty.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289#discussion_r1799337518
##########
artemis-hawtio/artemis-console/pom.xml:
##########
@@ -76,9 +76,14 @@
<artifactId>maven-war-plugin</artifactId>
<configuration>
<useCache>true</useCache>
+ <!-- exclude these jars as they are already shipped with the broker -->
<packagingExcludes>
**/lib/slf4j*.jar,
- **/lib/log4j*.jar
+ **/lib/log4j*.jar,
+ **/lib/guava*.jar,
+ **/lib/commons-io*.jar,
+ **/lib/commons-logging*.jar,
+ **/lib/failureaccess*.jar
Review Comment:
The comment added above this is "\<!-- exclude these jars as they are already shipped with the broker --\>"
I think we need to be very clear what is meant by 'shipped with the broker' here, and in turn what it is is being expected/required that a user might need to provide to make the resulting war usable in their situation.
The broker `artemis-server` module does not depend on or provide guava or commons-io, and neither does the `artemis-web` module this war is typically used via. The assembly distribution does include those things, but only due to _other_ included bits bringing them in, not the broker. So if we completely remove those deps, they wont in fact be provided necessarily, i.e this actually is likely to actually break non-assembly use cases.
Previously we have entirely stripped some logging deps, but the _required_ API dep was certain to provided, either by the modules the war is most likely to be used with, or by the application itself (as is a logging implementation unless they have none configured). That isnt necessarily true for the other deps like these.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5096 exclude unnecessary jars from console war [activemq-artemis]
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #5289:
URL: https://github.com/apache/activemq-artemis/pull/5289#discussion_r1801527985
##########
artemis-distribution/src/main/assembly/dep.xml:
##########
@@ -72,9 +72,18 @@
<includes>
<include>org.apache.activemq:artemis-console:war</include>
</includes>
- <outputDirectory>web</outputDirectory>
- <unpack>false</unpack>
- <outputFileNameMapping>console.war</outputFileNameMapping>
+ <outputDirectory>web/console.war</outputDirectory>
+ <unpack>true</unpack>
+ <!-- Exclude these jar files from console since they are already provided in the broker distribution.
+ If any of these jars is dropped from the broker distribution they should be added back to the console. -->
+ <unpackOptions>
+ <excludes>
+ <exclude>**/guava*.jar</exclude>
+ <exclude>**/commons-io*.jar</exclude>
+ <exclude>**/commons-logging*.jar</exclude>
+ <exclude>**/failureaccess*.jar</exclude>
+ </excludes>
+ </unpackOptions>
Review Comment:
So if I understand, this effectively removes the console.war file from the assembly, and extracts its [trimmed] contents instead, i.e puts a directory called console.war in its place? (EDIT: Or I could have read further before commenting, and saw the doc explaining that is what it does, hehe :D)
Do we need to do anything with the config for that changing? Its geared around war files from memory?
@brusdev any thoughts around the switch?
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact