I already responded once with a short response saying I don't think it's a good idea to remove the parser but I will elaborate more. Reading over this thread again I think there are really two independent things being discussed here and should be considered separately.
1) Removing javacc as a build time dependency I'm a -1 on this because I still do not understand the motivation. I 100% agree with Justin's response. It's just a build time dependency and we've had it forever and it's causing zero problems. What if we need to change the generated parser? Just because we haven't for a long time doesn't mean we won't need to. Removing it means you'd have to manually change the output or switch to something else. If the dependency is old then we handle it like every other dependency, we upgrade. I see mention of a maintenance burden but I don't see how there really is one. Keeping the dependency up to date shouldn't be any different than updating any of the other tons of dependencies ActiveMQ has. I'd argue it's a lot less of a burden considering we've not updated in a very long time and it's not shipped with the broker. We should just update to the latest version just like Artemis did. 2) Commiting the generated source code to the repo I'm -0 on this as I don't think it is necessary because the generated source is fast and only a few files but I guess it could be discussed more if there was some benefit. In this case we would just set up the parser to generate only if there's a change (so maybe move it into an optional profile). Many projects do this where you only run the generator when there's a change and then commit the output to avoid having to run it each time. This is pretty common when generating serializers for frameworks like protobuf or Thrift. This is also how we handle OpenWire, we have (well had, the current openwire generator is broken with new JDKs which is why we are working on the new one) the ability to generate a new version when we need to with a maven profile and just commit the generated source. On Sat, Jan 18, 2025 at 10:39 AM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > Hi > > You are right Art: there’s no issue for now. > We are not in the rush to update. I would just create a Jira to track it > for later purpose. > > Regards > JB > > > Le sam. 18 janv. 2025 à 14:24, Arthur Naseef <artnas...@apache.org> a > écrit : > > > Is the generated code problematic, or are we talking hypotheticals here? > > > > Not trying to discourage updating - but instead trying to understand need > > here: > > why does javacc need to be updated? > > > > Ther javacc grammar file is the original code, and the generated java is > an > > output of that. If we want to hand-maintain a parser, we probably want > > something more readable/maintainable than javacc output. > > > > Art > > > > On Sat, Jan 18, 2025, 12:38 AM Jean-Baptiste Onofré <j...@nanthrax.net> > > wrote: > > > > > Do we really need to generate this code ? > > > > > > It's a dependency of the build (not the packages dependency), but it > > > could be problematic regarding the code generated for updated JDK > > > versions (JDK11 and beyond, especially JDK23 at some point). > > > > > > As we don't generate the code often, I don't see the benefit compared > > > to just having the source. I don't think it's an urgent matter, but > > > it's worth keeping this in mind and I think it would be a good idea to > > > remove this build dep. > > > > > > Regards > > > JB > > > > > > On Sat, Jan 18, 2025 at 12:15 AM Christopher Shannon > > > <christopher.l.shan...@gmail.com> wrote: > > > > > > > > I don't think there is a good reason to get rid of dependency either, > > we > > > > absolutely need to be able to build a new version going forward. Just > > > > because it hasn't changed doesn't mean it won't. > > > > > > > > On Fri, Jan 17, 2025 at 3:20 PM Arthur Naseef <artnas...@apache.org> > > > wrote: > > > > > > > > > I agree with Justin here. What is the need to remove the > dependency > > on > > > > > javacc - especially since it is build-time only? > > > > > > > > > > Art > > > > > > > > > > > > > > > On Fri, Jan 17, 2025 at 1:11 PM Justin Bertram < > jbert...@apache.org> > > > > > wrote: > > > > > > > > > > > In what sense is JavaCC a "dependency of the activemq-client > > > package"? > > > > > It's > > > > > > not a Maven dependency, and it's not shipped with the broker. > It's > > > simply > > > > > > part of the build process and represents a near-zero maintenance > > > burden. > > > > > > > > > > > > I'm against checking in the generated source and removing the > > > integration > > > > > > with JavaCC for the following reasons: > > > > > > > > > > > > - You never know what changes will be required in the future. > > > Generally > > > > > > speaking, you'd want to modify the JavaCC input rather than the > > > JavaCC > > > > > > output in that case. > > > > > > - If there is ever any improvement to JavaCC we won't benefit > > from > > > it. > > > > > > - There is no real downside to keeping the existing structure > in > > > place. > > > > > > > > > > > > Artemis uses the same basic process to generate the selector > > parser, > > > and > > > > > it > > > > > > uses JavaCC 7.0.13 without issue. > > > > > > > > > > > > What is the benefit of removing the integration and checking in > the > > > > > > generated code? > > > > > > > > > > > > > > > > > > Justin > > > > > > > > > > > > On Fri, Jan 17, 2025 at 3:49 AM Ken Liao <kenlia...@gmail.com> > > > wrote: > > > > > > > > > > > > > Hi folks, > > > > > > > > > > > > > > Recently, I am diving into the SelectorParser.java generated by > > > > > javacc. I > > > > > > > am wondering, do we want to keep maintaining javacc as a > > > dependency of > > > > > > the > > > > > > > activemq-client package? > > > > > > > > > > > > > > In another word, the grammar of the JMS selector hasn't changed > > > (last > > > > > > time > > > > > > > the change made to the grammar definition file > SelectorParser.jj > > is > > > > > > > changing the namespace to jakarta in the main branch). Would it > > be > > > > > easier > > > > > > > to just commit the generated java file as source and remove the > > > javacc > > > > > > > dependency? > > > > > > > > > > > > > > If we do want to keep it as a dependency, the latest stable > > > release of > > > > > > > javacc is version 7. I can upgrade javacc to version 7 to check > > if > > > it > > > > > > > breaks the build and tests. I will create a PR on it soon if > > > there's no > > > > > > > objection. > > > > > > > > > > > > > > Thanks, > > > > > > > Ken > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@activemq.apache.org > > > For additional commands, e-mail: dev-h...@activemq.apache.org > > > For further information, visit: https://activemq.apache.org/contact > > > > > > > > > > > >
