Those users can clearly still use the client without shading though, they'd just have to copy more files. Not quite as trivial, certainly, but hardly an insurmountable challenge either. If really against having multiple files like many many things do, they could always produce an uber jar in e.g a CI job, perhaps along with their application bits bundled in too since liking having one file.
Having everyone who gets the assembly download need to grab what is 17MB (out of 50MB total, so 33MB without them) of essentially triple-duplicated dependencies to satisfy one somewhat-corner use case that has other simple options, doesnt seem needed for me. Having us all [try to] maintain these essentially untested set of uber jars for that use case also seems off. Especially when for many people I'd say they shouldnt even consider using these artifacts. Shading also introduces various issues for applications, particularly those ones not using build tooling. It makes it difficult to update transitive dependencies (say, prior to a release) without rebuilding the shaded artifact, cumbersome for security updates. It makes it hard to even see what is being used for identifying if such updates are needed, since everythings all hidden in the shaded artifact. It can even make it impossible to use other components in the same application if it depends on (or even similarly uber-bundled) a subtly different version of the same dependency that isnt shaded, e.g it appears Netty is not being shaded in the -all client jar, so anything else using Netty in an app could easily introduce potentially incompatible netty classes to the classpath when combined with the client uber jar. The latter is why netty themselves did away with their own uber jar and kept only the individual ones, since that kept happening, one component used the uber, another component used individual bits, and then apps using both components then had subtle (or nt) clashes from mixing them since at that point even the build tools such as Maven couldnt them keep things aligned on one version because the components have deps using the different GAVs. As an aside, I dont think the issue is really that Log4j bits directly, but probably some stuff I did around commons-logging->SLF4J bridging. Either way its likely the shading stuff needs adjusted since nothings been done it for with the logging bits yet and it seemed to be working, it was only on doing a re-build clebert noticed it fails. On Wed, 27 Jul 2022 at 15:57, Michael André Pearce <michael.andre.pea...@me.com.invalid> wrote: > > So as the person who added the shaded jars, this was a requirement to make it > far easier for those who DO NOT use tools like maven to be able to build an > application without having to copy a large dependency tree of libs, which yes > for maven or gradle is fine as they handle the dependency tree, as those > tools do this. But unfortunately we don't live in a perfect world, and > providing the shaded allows those users to use the client. > > Im happy to look at why moving logging to slf4j brings an issue, im unsure > why it does tbh, its just another lib, and as long as you shade correctly it > should be fine, ive seen it shaded in many other projects. > > With this i am going on leave for a bit, so unlikely i will get to have a > look at it for a week or two. > > Best > Mike > > On 26 Jul 2022, at 21:44, Justin Bertram <jbert...@apache.org> wrote: > > > To be clear, the documentation already exists [1]. It just needs to be > updated with the aforementioned details when the uber jars are removed. > > > Justin > > [1] > https://activemq.apache.org/components/artemis/documentation/latest/client-classpath.html > > On Tue, Jul 26, 2022 at 3:39 PM Clebert Suconic <clebert.suco...@gmail.com> > wrote: > > sure, of course we need to update the docs in relation to anything > these removed jars. What I meant was we need to document the jars that > are required independently of removing the jars.. if someone wants to > use the client jars the client dependency should be documented anyway. > that's what I meant > > On Tue, Jul 26, 2022 at 3:25 PM Justin Bertram <jbert...@apache.org> > wrote: > > > > I'm not sure what you mean by "independent issue." If we remove the uber > > jars then the docs have to be updated. The two things are directly > related, > > right? > > > > > > Justin > > > > On Tue, Jul 26, 2022 at 2:13 PM Clebert Suconic < > clebert.suco...@gmail.com> > > wrote: > > > > > I think that’s an independent issue. The doc would need to be updated > > > anyways. > > > > > > On Tue, Jul 26, 2022 at 2:40 PM Justin Bertram <jbert...@apache.org> > > > wrote: > > > > > > > Yes, the documentation needs to be clear. This is a usability issue. > > > > > > > > Even if you did a "mvn dependency:list" you'd get a list including > > > optional > > > > and test dependencies. Also, there would be potentially unnecessary > > > > dependencies (e.g. netty-transport-native-kqueue even if you aren't > on a > > > > Mac). > > > > > > > > > > > > Justin > > > > > > > > On Tue, Jul 26, 2022 at 1:30 PM Clebert Suconic < > > > clebert.suco...@gmail.com > > > > > > > > > wrote: > > > > > > > > > the pom on artemis-core-client, artemis-jms-client, and > > > > > artemis-jakarta-client... They will include all the needed > > > > > dependencies, right? > > > > > > > > > > > > > > > what is the issue? to have a clear text on the docs? > > > > > > > > > > On Tue, Jul 26, 2022 at 2:01 PM Justin Bertram < > jbert...@apache.org> > > > > > wrote: > > > > > > > > > > > > The original impetus for the uber jar was ARTEMIS-1129. The issue > > > there > > > > > was > > > > > > that it wasn't clear what jars were needed on the client. If we > > > remove > > > > > the > > > > > > uber jars then we need to update the documentation to make > crystal > > > > clear > > > > > > what jars are needed on the client, including details about what > jars > > > > may > > > > > > be optional depending on which functionality the client uses. > > > > > > > > > > > > I'm not necessarily against it, but removing the uber jar is > probably > > > > > going > > > > > > to sting for a handful of users. Anything we can do to alleviate > that > > > > > will > > > > > > help. Maybe we could generate a text file in lib/client instead > of > > > the > > > > > uber > > > > > > jars to help users who expect them to be there. The text could > list > > > the > > > > > > jars required for the client's classpath. > > > > > > > > > > > > > > > > > > Justin > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/ARTEMIS-1129 > > > > > > > > > > > > On Tue, Jul 26, 2022 at 8:40 AM Clebert Suconic < > > > > > clebert.suco...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > We currently deploy these following shaded uber jars with > ActiveMQ > > > > > Artemis. > > > > > > > > > > > > > > artemis-jms-client-all > > > > > > > artemis-core-client-all > > > > > > > artemis-jakarta-client-all > > > > > > > > > > > > > > We are in the process of removing jboss-logging, and replacing > it > > > by > > > > > > > SLF4j /LOG4J on a separate branch, and we will probably make a > > > switch > > > > > > > on the branch as 3.0. > > > > > > > > > > > > > > I never really liked these shaded jars as part of the > > > distribution. I > > > > > > > would be inclined to remove them on a switch for 3.0 anyways, > and > > > now > > > > > > > we are having a build issue, > > > > > > > as they will fail (on a second build) shading > > > apache-commons-logging: > > > > > > > > > > > > > > ERROR] Failed to execute goal > > > > > > > org.apache.maven.plugins:maven-shade-plugin:3.3.0:shade > (default) > > > on > > > > > > > project artemis-core-client-all: Error creating shaded jar: > > > duplicate > > > > > > > entry: META-INF/services/ > org.apache.activemq.artemis.shaded.org > > > > > > > .apache.commons.logging.LogFactory > > > > > > > -> [Help 1] [ERROR] [ERROR] To see the full stack trace of the > > > > > > > errors, re-run Maven with the -e switch. [ERROR] Re-run Maven > using > > > > > > > the -X switch to enable full debug logging. [ERROR] [ERROR] > For > > > more > > > > > > > information about the errors and possible solutions, please > read > > > the > > > > > > > following articles: [ERROR] [Help 1] > > > > > > > > > > > > > > > > http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException > > > > > > > [ERROR] [ERROR] After correcting the problems, you can resume > the > > > > > > > build with the command [ERROR] mvn <args> -rf > > > > > > > :artemis-core-client-all > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also, they add about 20MB to our distribution, and more 10MB > for > > > the > > > > > > > core-client-all that's not on the distro but it is on maven > repo. > > > > > > > > > > > > > > This is a common trend with other projects. Netty stopped > > > producing a > > > > > > > netty-all and is offering a pom. Jetty did the same thing.. and > > > There > > > > > > > are a lot of issues introduced by an "all client". > > > > > > > > > > > > > > > > > > > > > So, even though we could fix the build, these JARs are never > tested > > > > as > > > > > > > part of the testsuite or anything.... It's like playing with > the > > > > > > > odds... and they are huge on the distribution as they will all > > > > > > > include copies of Netty. > > > > > > > > > > > > > > > > > > > > > I would really like to remove these JARs and I think it would > be a > > > > > > > great improvement to do so. > > > > > > > > > > > > > > These POMS are already defining all the dependencies anyway. > Any > > > user > > > > > > > who wants to have a shaded jar would just be able to shade it > > > > > > > themselves as part of their project. > > > > > > > > > > > > > > > > > > > > > If anyone have a strong feeling about keeping them we would > need: > > > > > > > > > > > > > > - your opinion (why we keep them on 3.0) > > > > > > > - Help fixing the build on new-logging > > > > > > > - Help with adding smoke tests for these jars. > > > > > > > > > > > > > > > > > > > > > anyone? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Clebert Suconic > > > > > > > > > > > > > > > > > -- > > > Clebert Suconic > > > > > > > -- > Clebert Suconic > >
