You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by Jonathan Gallimore <jo...@gmail.com> on 2018/11/02 10:53:42 UTC
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
I'm following this thread with some interest. What would be some good tests
to make sure these cases are covered off? Some suggestions in addition to
the existing test:
* Standalone client that has its own main and delegates to the CLI main
* As above, but perhaps with the standalone client creating a classloader
with the CLI jars and then launching it
* A webapp calling commands in the CLI
Any others? These are interesting use cases that I hadn't considered - this
is interesting.
Jon
On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <rm...@gmail.com>
wrote:
> Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <da...@gmail.com> a écrit
> :
>
> > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
> > jonathan.gallimore@gmail.com> escreveu:
> >
> > > We should write a test case for both of these, and agree on them. I'd
> > like
> > > to be able to merge this in with confidence, and without the tests I
> > don't
> > > see how we can do that.
> > >
> > > Jon
> > >
> > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
> > rmannibucau@gmail.com>
> > > wrote:
> > >
> > > > Hi Daniel,
> > > >
> > > > the point is that this code is reused in several places so we must be
> > as
> > > > clean as we can, here the cases I had in mind:
> > > >
> > > > 1. reused main in another main (so an app calls the main): here the
> > > pitfall
> > > > is that the property openejb.classloader.first.disallow-system-load
> is
> > > not
> > > > resetted and the TCLL neither so after the main you use a closed
> > > > classloader (so will likely easily fail)
> > >
> >
> > I really didn't get your point. Side note we are create a classloader
> only
> > for tomee cli execution, which keep alive only for a short time during
> > command execution.
> >
> > That part of code is not reused in another place, if yes, just point me
> > where, because I can't find it.
> >
> > It will close only after the program finish, since it is inside a
> try-catch
> > block, no?
> >
>
> Assuming the main is launch but this main is sometimes launched
> programmatically (like in a webapp)
> When not you are right
>
>
> >
> >
> > > > 2. if a user relies in java 8 on the classloader being the system one
> > > then
> > > > your new classloader level breaks it so we shouldnt create this new
> > > > classloader when not needed and the old ClassPath impl is working
> IMO.
> > We
> > > > can enrich it with a warn log saying "it will fail in next version"
> > (and
> > > we
> > > > drop that code after 1 or 2 releases). Idea is to not break directly
> > > users
> > > > who can not even know some of the libs they added in the classpath
> for
> > > > logging purposes rely on that classloader setup - random example
> indeed
> > > ;).
> > > >
> > >
> >
> > Yeah, I probably could use the SystemClassPath, but it is doing much more
> > than I need to CLI. I don't think that I need to change the
> java.class.path
> > property to make it works or other property like
> > openejb.classloader.first.disallow-system-load.
> >
>
> We should likely be cleaner here but not your PR, fair enough
>
>
> >
> > And to be honest, I didn't understand what is
> > openejb.classloader.first.disallow-system-load and why do we have it. :)
> >
>
> It is used in the "webapp" loader to check if we should load some well
> known classes from the system classloader or not.
> It is likely the kind of property you rely on when you want to be
> embeddable but also standalone.
>
>
> >
> >
> > > > Romain Manni-Bucau
> > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > <http://rmannibucau.wordpress.com> | Github <
> > > > https://github.com/rmannibucau> |
> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > > > <
> > > >
> > >
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > > > >
> > > >
> > > >
> > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <da...@apache.org> a
> > > > écrit :
> > > >
> > > > > Hi,
> > > > >
> > > > > I updated the PR with tests!
> > > > >
> > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> > > > > jonathan.gallimore@gmail.com> escreveu:
> > > > >
> > > > > > I saw your note on the PR regarding Romain's feedback.
> > > > > >
> > > > > > For visibility here, Romain is querying whether this patch will
> > cause
> > > > an
> > > > > > issue with folks calling the CLI with a custom classpath using
> > `java
> > > > -cp
> > > > > > ...` under Java 8. I'd suggest we add a test for it, which looks
> to
> > > be
> > > > > what
> > > > > > Daniel is doing.
> > > > > >
> > > > > > Thanks guys.
> > > > > >
> > > > > > Jon
> > > > > >
> > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
> > danielsoro@apache.org
> > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Folks,
> > > > > > >
> > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot of
> > feedback
> > > > > from
> > > > > > > Romain (thank you Romain for all comments and help)
> > > > > > >
> > > > > > > I believe which we are in good shape to merge.
> > > > > > > The changes cover from Java 8 till Java 11 which not provide
> any
> > > > impact
> > > > > > for
> > > > > > > TomEE or application deployed on it.
> > > > > > >
> > > > > > > Please, look at the PR and let me know what you think.
> > > > > > >
> > > > > > > PR: https://github.com/apache/tomee/pull/176
> > > > > > >
> > > > > > > --
> > > > > > > Daniel "soro" Cunha
> > > > > > > https://twitter.com/dvlc_
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Daniel "soro" Cunha
> > > > > https://twitter.com/dvlc_
> > > > >
> > > >
> > >
> >
> > Thank you! :)
> > --
> > Daniel "soro" Cunha
> > https://twitter.com/dvlc_
> >
>
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi Daniel,
Cause there are some commands using a webapp like classloader and it would
fail in this mode in general - we have the same hack for tomee embedded i
think, just not triggered the same way.
Feel free to revisit it and unify it with what you are doing. Likely what
was planned:
https://github.com/apache/tomee/blob/master/container/openejb-loader/src/main/java/org/apache/openejb/loader/SystemClassPath.java#L33
and
https://github.com/apache/tomee/blob/master/container/openejb-loader/src/main/java/org/apache/openejb/loader/SystemClassPath.java#L66.
This is why i mentionned you shouldnt modify the classloader by default at
the beginning of the review and just reuse the system classpath. The
intention was to make this custom loader behave the same as the child first
one but guess it was never aligned.
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Le jeu. 22 nov. 2018 à 14:24, Daniel Cunha <da...@gmail.com> a écrit :
> Hi Romain,
>
> I get your point now...
> Question: Why do we need to do it[1]?
>
> if (loader != null) {
> Thread.currentThread().setContextClassLoader(loader);
> if (loader != ClassLoader.getSystemClassLoader()) {
>
> System.setProperty("openejb.classloader.first.disallow-system-loading",
> "true");
> }
> }
>
>
> Do we really need to change it? Why?
> I believe which the CLI has you owner classpath and we don't need to change
> the default on to execute our commands. Just try understand why we are
> doing it.
>
> [1]
>
> https://github.com/apache/tomee/blob/master/container/openejb-core/src/main/java/org/apache/openejb/cli/Bootstrap.java#L151-L156
>
> Em ter, 20 de nov de 2018 às 17:32, Daniel Cunha <da...@gmail.com>
> escreveu:
>
> > Hi Romain,
> >
> > Ok, I'll create a scenario for it.
> >
> > Thank you.
> >
> > Em ter, 20 de nov de 2018 às 17:29, Romain Manni-Bucau <
> > rmannibucau@gmail.com> escreveu:
> >
> >> Hi Daniel
> >>
> >> Do you think you can assert the classloader in the execution thread
> didnt
> >> change after the call? Was one concern and i didnt find the matching
> >> assert
> >>
> >> Le mar. 20 nov. 2018 20:00, Daniel Cunha <da...@apache.org> a
> écrit
> >> :
> >>
> >> > Hi folks,
> >> >
> >> > I've added a new test with JAXRS, calling a command in my endpoint
> from
> >> > Bootstrap CLI.
> >> > Please review the changes on my PR[1] and let me know what do you
> think
> >> > about it.
> >> >
> >> > Thank you. :)
> >> >
> >> > [1] https://github.com/apache/tomee/pull/176
> >> >
> >> > Em seg, 19 de nov de 2018 às 13:57, Daniel Cunha <
> danielsoro@gmail.com>
> >> > escreveu:
> >> >
> >> > > Hi Jon,
> >> > >
> >> > > ok. I got it! :)
> >> > >
> >> > > Em seg, 19 de nov de 2018 às 13:49, Jonathan Gallimore <
> >> > > jonathan.gallimore@gmail.com> escreveu:
> >> > >
> >> > >> On Mon, Nov 19, 2018 at 4:30 PM Daniel Cunha <danielsoro@gmail.com
> >
> >> > >> wrote:
> >> > >>
> >> > >> > Hi guys,
> >> > >> >
> >> > >> > I updated the PR with some chages:
> >> > >> > 1 - Move BasicURLClassPath.CustomizableURLClassLoader to be
> public.
> >> > >> > 2 - I'm using BasicURLClassPath.CustomizableURLClassLoader
> instead
> >> of
> >> > >> > DynamicClassLoader (class that I created)
> >> > >> > 3- DynamicClassLoader is not needed anymore. Was removed from the
> >> PR.
> >> > >> > 3 - Added more tests:
> >> > >> > - Standalone client that has its own main and delegates to
> the
> >> CLI
> >> > >> main
> >> > >> > - As above, but perhaps with the standalone client creating a
> >> > >> > classloader
> >> > >> > with the CLI jars and then launching it
> >> > >> >
> >> > >> > Question:
> >> > >> >
> >> > >> > @Jon,
> >> > >> > what do you want me mean with: A webapp calling commands in the
> >> CLI?
> >> > >> >
> >> > >>
> >> > >> Hopefully I can answer that one in a straightforward way :). Try
> >> > creating
> >> > >> a
> >> > >> servlet that loads the Bootstrap CLI class, and invokes some
> methods.
> >> > >> Effectively you'd be calling the TomEE command line commands from a
> >> > >> servlet. That use case hadn't occurred to me, but it sounds like
> one
> >> > worth
> >> > >> trying.
> >> > >>
> >> > >> That would probably need to sit
> >> > >> under
> >> > arquillian/arquillian-tomee-tests/arquillian-tomee-webprofile-tests.
> >> > >>
> >> > >> Jon
> >> > >>
> >> > >>
> >> > >> >
> >> > >> > Command is a class with main method, if I've it deployed in my
> >> library
> >> > >> > folder, it should works fine, I just need to create a instance
> and
> >> > call
> >> > >> the
> >> > >> > main.
> >> > >> > Same will be if I try execute it with Process (that is exactly
> >> what we
> >> > >> are
> >> > >> > testing).
> >> > >> >
> >> > >> > Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
> >> > >> > jonathan.gallimore@gmail.com> escreveu:
> >> > >> >
> >> > >> > > I'm following this thread with some interest. What would be
> some
> >> > good
> >> > >> > tests
> >> > >> > > to make sure these cases are covered off? Some suggestions in
> >> > >> addition to
> >> > >> > > the existing test:
> >> > >> > >
> >> > >> > > * Standalone client that has its own main and delegates to the
> >> CLI
> >> > >> main
> >> > >> > > * As above, but perhaps with the standalone client creating a
> >> > >> classloader
> >> > >> > > with the CLI jars and then launching it
> >> > >> > > * A webapp calling commands in the CLI
> >> > >> > >
> >> > >> > > Any others? These are interesting use cases that I hadn't
> >> > considered -
> >> > >> > this
> >> > >> > > is interesting.
> >> > >> > >
> >> > >> > > Jon
> >> > >> > >
> >> > >> > >
> >> > >> > >
> >> > >> > > On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <
> >> > >> > rmannibucau@gmail.com>
> >> > >> > > wrote:
> >> > >> > >
> >> > >> > > > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <
> >> danielsoro@gmail.com>
> >> > a
> >> > >> > > écrit
> >> > >> > > > :
> >> > >> > > >
> >> > >> > > > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
> >> > >> > > > > jonathan.gallimore@gmail.com> escreveu:
> >> > >> > > > >
> >> > >> > > > > > We should write a test case for both of these, and agree
> on
> >> > >> them.
> >> > >> > I'd
> >> > >> > > > > like
> >> > >> > > > > > to be able to merge this in with confidence, and without
> >> the
> >> > >> tests
> >> > >> > I
> >> > >> > > > > don't
> >> > >> > > > > > see how we can do that.
> >> > >> > > > > >
> >> > >> > > > > > Jon
> >> > >> > > > > >
> >> > >> > > > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
> >> > >> > > > > rmannibucau@gmail.com>
> >> > >> > > > > > wrote:
> >> > >> > > > > >
> >> > >> > > > > > > Hi Daniel,
> >> > >> > > > > > >
> >> > >> > > > > > > the point is that this code is reused in several places
> >> so
> >> > we
> >> > >> > must
> >> > >> > > be
> >> > >> > > > > as
> >> > >> > > > > > > clean as we can, here the cases I had in mind:
> >> > >> > > > > > >
> >> > >> > > > > > > 1. reused main in another main (so an app calls the
> >> main):
> >> > >> here
> >> > >> > the
> >> > >> > > > > > pitfall
> >> > >> > > > > > > is that the property
> >> > >> > openejb.classloader.first.disallow-system-load
> >> > >> > > > is
> >> > >> > > > > > not
> >> > >> > > > > > > resetted and the TCLL neither so after the main you
> use a
> >> > >> closed
> >> > >> > > > > > > classloader (so will likely easily fail)
> >> > >> > > > > >
> >> > >> > > > >
> >> > >> > > > > I really didn't get your point. Side note we are create a
> >> > >> classloader
> >> > >> > > > only
> >> > >> > > > > for tomee cli execution, which keep alive only for a short
> >> time
> >> > >> > during
> >> > >> > > > > command execution.
> >> > >> > > > >
> >> > >> > > > > That part of code is not reused in another place, if yes,
> >> just
> >> > >> point
> >> > >> > me
> >> > >> > > > > where, because I can't find it.
> >> > >> > > > >
> >> > >> > > > > It will close only after the program finish, since it is
> >> inside
> >> > a
> >> > >> > > > try-catch
> >> > >> > > > > block, no?
> >> > >> > > > >
> >> > >> > > >
> >> > >> > > > Assuming the main is launch but this main is sometimes
> launched
> >> > >> > > > programmatically (like in a webapp)
> >> > >> > > > When not you are right
> >> > >> > > >
> >> > >> > > >
> >> > >> > > > >
> >> > >> > > > >
> >> > >> > > > > > > 2. if a user relies in java 8 on the classloader being
> >> the
> >> > >> system
> >> > >> > > one
> >> > >> > > > > > then
> >> > >> > > > > > > your new classloader level breaks it so we shouldnt
> >> create
> >> > >> this
> >> > >> > new
> >> > >> > > > > > > classloader when not needed and the old ClassPath impl
> is
> >> > >> working
> >> > >> > > > IMO.
> >> > >> > > > > We
> >> > >> > > > > > > can enrich it with a warn log saying "it will fail in
> >> next
> >> > >> > version"
> >> > >> > > > > (and
> >> > >> > > > > > we
> >> > >> > > > > > > drop that code after 1 or 2 releases). Idea is to not
> >> break
> >> > >> > > directly
> >> > >> > > > > > users
> >> > >> > > > > > > who can not even know some of the libs they added in
> the
> >> > >> > classpath
> >> > >> > > > for
> >> > >> > > > > > > logging purposes rely on that classloader setup -
> random
> >> > >> example
> >> > >> > > > indeed
> >> > >> > > > > > ;).
> >> > >> > > > > > >
> >> > >> > > > > >
> >> > >> > > > >
> >> > >> > > > > Yeah, I probably could use the SystemClassPath, but it is
> >> doing
> >> > >> much
> >> > >> > > more
> >> > >> > > > > than I need to CLI. I don't think that I need to change the
> >> > >> > > > java.class.path
> >> > >> > > > > property to make it works or other property like
> >> > >> > > > > openejb.classloader.first.disallow-system-load.
> >> > >> > > > >
> >> > >> > > >
> >> > >> > > > We should likely be cleaner here but not your PR, fair enough
> >> > >> > > >
> >> > >> > > >
> >> > >> > > > >
> >> > >> > > > > And to be honest, I didn't understand what is
> >> > >> > > > > openejb.classloader.first.disallow-system-load and why do
> we
> >> > have
> >> > >> it.
> >> > >> > > :)
> >> > >> > > > >
> >> > >> > > >
> >> > >> > > > It is used in the "webapp" loader to check if we should load
> >> some
> >> > >> well
> >> > >> > > > known classes from the system classloader or not.
> >> > >> > > > It is likely the kind of property you rely on when you want
> to
> >> be
> >> > >> > > > embeddable but also standalone.
> >> > >> > > >
> >> > >> > > >
> >> > >> > > > >
> >> > >> > > > >
> >> > >> > > > > > > Romain Manni-Bucau
> >> > >> > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >> > >> > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> >> > >> > > > > > > <http://rmannibucau.wordpress.com> | Github <
> >> > >> > > > > > > https://github.com/rmannibucau> |
> >> > >> > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> |
> >> Book
> >> > >> > > > > > > <
> >> > >> > > > > > >
> >> > >> > > > > >
> >> > >> > > > >
> >> > >> > > >
> >> > >> > >
> >> > >> >
> >> > >>
> >> >
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >> > >> > > > > > > >
> >> > >> > > > > > >
> >> > >> > > > > > >
> >> > >> > > > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <
> >> > >> > danielsoro@apache.org>
> >> > >> > > a
> >> > >> > > > > > > écrit :
> >> > >> > > > > > >
> >> > >> > > > > > > > Hi,
> >> > >> > > > > > > >
> >> > >> > > > > > > > I updated the PR with tests!
> >> > >> > > > > > > >
> >> > >> > > > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan
> Gallimore
> >> <
> >> > >> > > > > > > > jonathan.gallimore@gmail.com> escreveu:
> >> > >> > > > > > > >
> >> > >> > > > > > > > > I saw your note on the PR regarding Romain's
> >> feedback.
> >> > >> > > > > > > > >
> >> > >> > > > > > > > > For visibility here, Romain is querying whether
> this
> >> > patch
> >> > >> > will
> >> > >> > > > > cause
> >> > >> > > > > > > an
> >> > >> > > > > > > > > issue with folks calling the CLI with a custom
> >> classpath
> >> > >> > using
> >> > >> > > > > `java
> >> > >> > > > > > > -cp
> >> > >> > > > > > > > > ...` under Java 8. I'd suggest we add a test for
> it,
> >> > which
> >> > >> > > looks
> >> > >> > > > to
> >> > >> > > > > > be
> >> > >> > > > > > > > what
> >> > >> > > > > > > > > Daniel is doing.
> >> > >> > > > > > > > >
> >> > >> > > > > > > > > Thanks guys.
> >> > >> > > > > > > > >
> >> > >> > > > > > > > > Jon
> >> > >> > > > > > > > >
> >> > >> > > > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
> >> > >> > > > > danielsoro@apache.org
> >> > >> > > > > > >
> >> > >> > > > > > > > > wrote:
> >> > >> > > > > > > > >
> >> > >> > > > > > > > > > Hi Folks,
> >> > >> > > > > > > > > >
> >> > >> > > > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I
> got a
> >> > lot
> >> > >> of
> >> > >> > > > > feedback
> >> > >> > > > > > > > from
> >> > >> > > > > > > > > > Romain (thank you Romain for all comments and
> help)
> >> > >> > > > > > > > > >
> >> > >> > > > > > > > > > I believe which we are in good shape to merge.
> >> > >> > > > > > > > > > The changes cover from Java 8 till Java 11 which
> >> not
> >> > >> > provide
> >> > >> > > > any
> >> > >> > > > > > > impact
> >> > >> > > > > > > > > for
> >> > >> > > > > > > > > > TomEE or application deployed on it.
> >> > >> > > > > > > > > >
> >> > >> > > > > > > > > > Please, look at the PR and let me know what you
> >> think.
> >> > >> > > > > > > > > >
> >> > >> > > > > > > > > > PR: https://github.com/apache/tomee/pull/176
> >> > >> > > > > > > > > >
> >> > >> > > > > > > > > > --
> >> > >> > > > > > > > > > Daniel "soro" Cunha
> >> > >> > > > > > > > > > https://twitter.com/dvlc_
> >> > >> > > > > > > > > >
> >> > >> > > > > > > > >
> >> > >> > > > > > > >
> >> > >> > > > > > > >
> >> > >> > > > > > > > --
> >> > >> > > > > > > > Daniel "soro" Cunha
> >> > >> > > > > > > > https://twitter.com/dvlc_
> >> > >> > > > > > > >
> >> > >> > > > > > >
> >> > >> > > > > >
> >> > >> > > > >
> >> > >> > > > > Thank you! :)
> >> > >> > > > > --
> >> > >> > > > > Daniel "soro" Cunha
> >> > >> > > > > https://twitter.com/dvlc_
> >> > >> > > > >
> >> > >> > > >
> >> > >> > >
> >> > >> >
> >> > >> >
> >> > >> > --
> >> > >> > Daniel "soro" Cunha
> >> > >> > https://twitter.com/dvlc_
> >> > >> >
> >> > >>
> >> > >
> >> > >
> >> > > --
> >> > > Daniel "soro" Cunha
> >> > > https://twitter.com/dvlc_
> >> > >
> >> >
> >> >
> >> > --
> >> > Daniel "soro" Cunha
> >> > https://twitter.com/dvlc_
> >> >
> >>
> >
> >
> > --
> > Daniel "soro" Cunha
> > https://twitter.com/dvlc_
> >
>
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Daniel Cunha <da...@gmail.com>.
Hi Romain,
I get your point now...
Question: Why do we need to do it[1]?
if (loader != null) {
Thread.currentThread().setContextClassLoader(loader);
if (loader != ClassLoader.getSystemClassLoader()) {
System.setProperty("openejb.classloader.first.disallow-system-loading",
"true");
}
}
Do we really need to change it? Why?
I believe which the CLI has you owner classpath and we don't need to change
the default on to execute our commands. Just try understand why we are
doing it.
[1]
https://github.com/apache/tomee/blob/master/container/openejb-core/src/main/java/org/apache/openejb/cli/Bootstrap.java#L151-L156
Em ter, 20 de nov de 2018 às 17:32, Daniel Cunha <da...@gmail.com>
escreveu:
> Hi Romain,
>
> Ok, I'll create a scenario for it.
>
> Thank you.
>
> Em ter, 20 de nov de 2018 às 17:29, Romain Manni-Bucau <
> rmannibucau@gmail.com> escreveu:
>
>> Hi Daniel
>>
>> Do you think you can assert the classloader in the execution thread didnt
>> change after the call? Was one concern and i didnt find the matching
>> assert
>>
>> Le mar. 20 nov. 2018 20:00, Daniel Cunha <da...@apache.org> a écrit
>> :
>>
>> > Hi folks,
>> >
>> > I've added a new test with JAXRS, calling a command in my endpoint from
>> > Bootstrap CLI.
>> > Please review the changes on my PR[1] and let me know what do you think
>> > about it.
>> >
>> > Thank you. :)
>> >
>> > [1] https://github.com/apache/tomee/pull/176
>> >
>> > Em seg, 19 de nov de 2018 às 13:57, Daniel Cunha <da...@gmail.com>
>> > escreveu:
>> >
>> > > Hi Jon,
>> > >
>> > > ok. I got it! :)
>> > >
>> > > Em seg, 19 de nov de 2018 às 13:49, Jonathan Gallimore <
>> > > jonathan.gallimore@gmail.com> escreveu:
>> > >
>> > >> On Mon, Nov 19, 2018 at 4:30 PM Daniel Cunha <da...@gmail.com>
>> > >> wrote:
>> > >>
>> > >> > Hi guys,
>> > >> >
>> > >> > I updated the PR with some chages:
>> > >> > 1 - Move BasicURLClassPath.CustomizableURLClassLoader to be public.
>> > >> > 2 - I'm using BasicURLClassPath.CustomizableURLClassLoader instead
>> of
>> > >> > DynamicClassLoader (class that I created)
>> > >> > 3- DynamicClassLoader is not needed anymore. Was removed from the
>> PR.
>> > >> > 3 - Added more tests:
>> > >> > - Standalone client that has its own main and delegates to the
>> CLI
>> > >> main
>> > >> > - As above, but perhaps with the standalone client creating a
>> > >> > classloader
>> > >> > with the CLI jars and then launching it
>> > >> >
>> > >> > Question:
>> > >> >
>> > >> > @Jon,
>> > >> > what do you want me mean with: A webapp calling commands in the
>> CLI?
>> > >> >
>> > >>
>> > >> Hopefully I can answer that one in a straightforward way :). Try
>> > creating
>> > >> a
>> > >> servlet that loads the Bootstrap CLI class, and invokes some methods.
>> > >> Effectively you'd be calling the TomEE command line commands from a
>> > >> servlet. That use case hadn't occurred to me, but it sounds like one
>> > worth
>> > >> trying.
>> > >>
>> > >> That would probably need to sit
>> > >> under
>> > arquillian/arquillian-tomee-tests/arquillian-tomee-webprofile-tests.
>> > >>
>> > >> Jon
>> > >>
>> > >>
>> > >> >
>> > >> > Command is a class with main method, if I've it deployed in my
>> library
>> > >> > folder, it should works fine, I just need to create a instance and
>> > call
>> > >> the
>> > >> > main.
>> > >> > Same will be if I try execute it with Process (that is exactly
>> what we
>> > >> are
>> > >> > testing).
>> > >> >
>> > >> > Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
>> > >> > jonathan.gallimore@gmail.com> escreveu:
>> > >> >
>> > >> > > I'm following this thread with some interest. What would be some
>> > good
>> > >> > tests
>> > >> > > to make sure these cases are covered off? Some suggestions in
>> > >> addition to
>> > >> > > the existing test:
>> > >> > >
>> > >> > > * Standalone client that has its own main and delegates to the
>> CLI
>> > >> main
>> > >> > > * As above, but perhaps with the standalone client creating a
>> > >> classloader
>> > >> > > with the CLI jars and then launching it
>> > >> > > * A webapp calling commands in the CLI
>> > >> > >
>> > >> > > Any others? These are interesting use cases that I hadn't
>> > considered -
>> > >> > this
>> > >> > > is interesting.
>> > >> > >
>> > >> > > Jon
>> > >> > >
>> > >> > >
>> > >> > >
>> > >> > > On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <
>> > >> > rmannibucau@gmail.com>
>> > >> > > wrote:
>> > >> > >
>> > >> > > > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <
>> danielsoro@gmail.com>
>> > a
>> > >> > > écrit
>> > >> > > > :
>> > >> > > >
>> > >> > > > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
>> > >> > > > > jonathan.gallimore@gmail.com> escreveu:
>> > >> > > > >
>> > >> > > > > > We should write a test case for both of these, and agree on
>> > >> them.
>> > >> > I'd
>> > >> > > > > like
>> > >> > > > > > to be able to merge this in with confidence, and without
>> the
>> > >> tests
>> > >> > I
>> > >> > > > > don't
>> > >> > > > > > see how we can do that.
>> > >> > > > > >
>> > >> > > > > > Jon
>> > >> > > > > >
>> > >> > > > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
>> > >> > > > > rmannibucau@gmail.com>
>> > >> > > > > > wrote:
>> > >> > > > > >
>> > >> > > > > > > Hi Daniel,
>> > >> > > > > > >
>> > >> > > > > > > the point is that this code is reused in several places
>> so
>> > we
>> > >> > must
>> > >> > > be
>> > >> > > > > as
>> > >> > > > > > > clean as we can, here the cases I had in mind:
>> > >> > > > > > >
>> > >> > > > > > > 1. reused main in another main (so an app calls the
>> main):
>> > >> here
>> > >> > the
>> > >> > > > > > pitfall
>> > >> > > > > > > is that the property
>> > >> > openejb.classloader.first.disallow-system-load
>> > >> > > > is
>> > >> > > > > > not
>> > >> > > > > > > resetted and the TCLL neither so after the main you use a
>> > >> closed
>> > >> > > > > > > classloader (so will likely easily fail)
>> > >> > > > > >
>> > >> > > > >
>> > >> > > > > I really didn't get your point. Side note we are create a
>> > >> classloader
>> > >> > > > only
>> > >> > > > > for tomee cli execution, which keep alive only for a short
>> time
>> > >> > during
>> > >> > > > > command execution.
>> > >> > > > >
>> > >> > > > > That part of code is not reused in another place, if yes,
>> just
>> > >> point
>> > >> > me
>> > >> > > > > where, because I can't find it.
>> > >> > > > >
>> > >> > > > > It will close only after the program finish, since it is
>> inside
>> > a
>> > >> > > > try-catch
>> > >> > > > > block, no?
>> > >> > > > >
>> > >> > > >
>> > >> > > > Assuming the main is launch but this main is sometimes launched
>> > >> > > > programmatically (like in a webapp)
>> > >> > > > When not you are right
>> > >> > > >
>> > >> > > >
>> > >> > > > >
>> > >> > > > >
>> > >> > > > > > > 2. if a user relies in java 8 on the classloader being
>> the
>> > >> system
>> > >> > > one
>> > >> > > > > > then
>> > >> > > > > > > your new classloader level breaks it so we shouldnt
>> create
>> > >> this
>> > >> > new
>> > >> > > > > > > classloader when not needed and the old ClassPath impl is
>> > >> working
>> > >> > > > IMO.
>> > >> > > > > We
>> > >> > > > > > > can enrich it with a warn log saying "it will fail in
>> next
>> > >> > version"
>> > >> > > > > (and
>> > >> > > > > > we
>> > >> > > > > > > drop that code after 1 or 2 releases). Idea is to not
>> break
>> > >> > > directly
>> > >> > > > > > users
>> > >> > > > > > > who can not even know some of the libs they added in the
>> > >> > classpath
>> > >> > > > for
>> > >> > > > > > > logging purposes rely on that classloader setup - random
>> > >> example
>> > >> > > > indeed
>> > >> > > > > > ;).
>> > >> > > > > > >
>> > >> > > > > >
>> > >> > > > >
>> > >> > > > > Yeah, I probably could use the SystemClassPath, but it is
>> doing
>> > >> much
>> > >> > > more
>> > >> > > > > than I need to CLI. I don't think that I need to change the
>> > >> > > > java.class.path
>> > >> > > > > property to make it works or other property like
>> > >> > > > > openejb.classloader.first.disallow-system-load.
>> > >> > > > >
>> > >> > > >
>> > >> > > > We should likely be cleaner here but not your PR, fair enough
>> > >> > > >
>> > >> > > >
>> > >> > > > >
>> > >> > > > > And to be honest, I didn't understand what is
>> > >> > > > > openejb.classloader.first.disallow-system-load and why do we
>> > have
>> > >> it.
>> > >> > > :)
>> > >> > > > >
>> > >> > > >
>> > >> > > > It is used in the "webapp" loader to check if we should load
>> some
>> > >> well
>> > >> > > > known classes from the system classloader or not.
>> > >> > > > It is likely the kind of property you rely on when you want to
>> be
>> > >> > > > embeddable but also standalone.
>> > >> > > >
>> > >> > > >
>> > >> > > > >
>> > >> > > > >
>> > >> > > > > > > Romain Manni-Bucau
>> > >> > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> > >> > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
>> > >> > > > > > > <http://rmannibucau.wordpress.com> | Github <
>> > >> > > > > > > https://github.com/rmannibucau> |
>> > >> > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> |
>> Book
>> > >> > > > > > > <
>> > >> > > > > > >
>> > >> > > > > >
>> > >> > > > >
>> > >> > > >
>> > >> > >
>> > >> >
>> > >>
>> >
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>> > >> > > > > > > >
>> > >> > > > > > >
>> > >> > > > > > >
>> > >> > > > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <
>> > >> > danielsoro@apache.org>
>> > >> > > a
>> > >> > > > > > > écrit :
>> > >> > > > > > >
>> > >> > > > > > > > Hi,
>> > >> > > > > > > >
>> > >> > > > > > > > I updated the PR with tests!
>> > >> > > > > > > >
>> > >> > > > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore
>> <
>> > >> > > > > > > > jonathan.gallimore@gmail.com> escreveu:
>> > >> > > > > > > >
>> > >> > > > > > > > > I saw your note on the PR regarding Romain's
>> feedback.
>> > >> > > > > > > > >
>> > >> > > > > > > > > For visibility here, Romain is querying whether this
>> > patch
>> > >> > will
>> > >> > > > > cause
>> > >> > > > > > > an
>> > >> > > > > > > > > issue with folks calling the CLI with a custom
>> classpath
>> > >> > using
>> > >> > > > > `java
>> > >> > > > > > > -cp
>> > >> > > > > > > > > ...` under Java 8. I'd suggest we add a test for it,
>> > which
>> > >> > > looks
>> > >> > > > to
>> > >> > > > > > be
>> > >> > > > > > > > what
>> > >> > > > > > > > > Daniel is doing.
>> > >> > > > > > > > >
>> > >> > > > > > > > > Thanks guys.
>> > >> > > > > > > > >
>> > >> > > > > > > > > Jon
>> > >> > > > > > > > >
>> > >> > > > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
>> > >> > > > > danielsoro@apache.org
>> > >> > > > > > >
>> > >> > > > > > > > > wrote:
>> > >> > > > > > > > >
>> > >> > > > > > > > > > Hi Folks,
>> > >> > > > > > > > > >
>> > >> > > > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a
>> > lot
>> > >> of
>> > >> > > > > feedback
>> > >> > > > > > > > from
>> > >> > > > > > > > > > Romain (thank you Romain for all comments and help)
>> > >> > > > > > > > > >
>> > >> > > > > > > > > > I believe which we are in good shape to merge.
>> > >> > > > > > > > > > The changes cover from Java 8 till Java 11 which
>> not
>> > >> > provide
>> > >> > > > any
>> > >> > > > > > > impact
>> > >> > > > > > > > > for
>> > >> > > > > > > > > > TomEE or application deployed on it.
>> > >> > > > > > > > > >
>> > >> > > > > > > > > > Please, look at the PR and let me know what you
>> think.
>> > >> > > > > > > > > >
>> > >> > > > > > > > > > PR: https://github.com/apache/tomee/pull/176
>> > >> > > > > > > > > >
>> > >> > > > > > > > > > --
>> > >> > > > > > > > > > Daniel "soro" Cunha
>> > >> > > > > > > > > > https://twitter.com/dvlc_
>> > >> > > > > > > > > >
>> > >> > > > > > > > >
>> > >> > > > > > > >
>> > >> > > > > > > >
>> > >> > > > > > > > --
>> > >> > > > > > > > Daniel "soro" Cunha
>> > >> > > > > > > > https://twitter.com/dvlc_
>> > >> > > > > > > >
>> > >> > > > > > >
>> > >> > > > > >
>> > >> > > > >
>> > >> > > > > Thank you! :)
>> > >> > > > > --
>> > >> > > > > Daniel "soro" Cunha
>> > >> > > > > https://twitter.com/dvlc_
>> > >> > > > >
>> > >> > > >
>> > >> > >
>> > >> >
>> > >> >
>> > >> > --
>> > >> > Daniel "soro" Cunha
>> > >> > https://twitter.com/dvlc_
>> > >> >
>> > >>
>> > >
>> > >
>> > > --
>> > > Daniel "soro" Cunha
>> > > https://twitter.com/dvlc_
>> > >
>> >
>> >
>> > --
>> > Daniel "soro" Cunha
>> > https://twitter.com/dvlc_
>> >
>>
>
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>
--
Daniel "soro" Cunha
https://twitter.com/dvlc_
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Daniel Cunha <da...@gmail.com>.
Hi Romain,
Ok, I'll create a scenario for it.
Thank you.
Em ter, 20 de nov de 2018 às 17:29, Romain Manni-Bucau <
rmannibucau@gmail.com> escreveu:
> Hi Daniel
>
> Do you think you can assert the classloader in the execution thread didnt
> change after the call? Was one concern and i didnt find the matching assert
>
> Le mar. 20 nov. 2018 20:00, Daniel Cunha <da...@apache.org> a écrit :
>
> > Hi folks,
> >
> > I've added a new test with JAXRS, calling a command in my endpoint from
> > Bootstrap CLI.
> > Please review the changes on my PR[1] and let me know what do you think
> > about it.
> >
> > Thank you. :)
> >
> > [1] https://github.com/apache/tomee/pull/176
> >
> > Em seg, 19 de nov de 2018 às 13:57, Daniel Cunha <da...@gmail.com>
> > escreveu:
> >
> > > Hi Jon,
> > >
> > > ok. I got it! :)
> > >
> > > Em seg, 19 de nov de 2018 às 13:49, Jonathan Gallimore <
> > > jonathan.gallimore@gmail.com> escreveu:
> > >
> > >> On Mon, Nov 19, 2018 at 4:30 PM Daniel Cunha <da...@gmail.com>
> > >> wrote:
> > >>
> > >> > Hi guys,
> > >> >
> > >> > I updated the PR with some chages:
> > >> > 1 - Move BasicURLClassPath.CustomizableURLClassLoader to be public.
> > >> > 2 - I'm using BasicURLClassPath.CustomizableURLClassLoader instead
> of
> > >> > DynamicClassLoader (class that I created)
> > >> > 3- DynamicClassLoader is not needed anymore. Was removed from the
> PR.
> > >> > 3 - Added more tests:
> > >> > - Standalone client that has its own main and delegates to the
> CLI
> > >> main
> > >> > - As above, but perhaps with the standalone client creating a
> > >> > classloader
> > >> > with the CLI jars and then launching it
> > >> >
> > >> > Question:
> > >> >
> > >> > @Jon,
> > >> > what do you want me mean with: A webapp calling commands in the CLI?
> > >> >
> > >>
> > >> Hopefully I can answer that one in a straightforward way :). Try
> > creating
> > >> a
> > >> servlet that loads the Bootstrap CLI class, and invokes some methods.
> > >> Effectively you'd be calling the TomEE command line commands from a
> > >> servlet. That use case hadn't occurred to me, but it sounds like one
> > worth
> > >> trying.
> > >>
> > >> That would probably need to sit
> > >> under
> > arquillian/arquillian-tomee-tests/arquillian-tomee-webprofile-tests.
> > >>
> > >> Jon
> > >>
> > >>
> > >> >
> > >> > Command is a class with main method, if I've it deployed in my
> library
> > >> > folder, it should works fine, I just need to create a instance and
> > call
> > >> the
> > >> > main.
> > >> > Same will be if I try execute it with Process (that is exactly what
> we
> > >> are
> > >> > testing).
> > >> >
> > >> > Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
> > >> > jonathan.gallimore@gmail.com> escreveu:
> > >> >
> > >> > > I'm following this thread with some interest. What would be some
> > good
> > >> > tests
> > >> > > to make sure these cases are covered off? Some suggestions in
> > >> addition to
> > >> > > the existing test:
> > >> > >
> > >> > > * Standalone client that has its own main and delegates to the CLI
> > >> main
> > >> > > * As above, but perhaps with the standalone client creating a
> > >> classloader
> > >> > > with the CLI jars and then launching it
> > >> > > * A webapp calling commands in the CLI
> > >> > >
> > >> > > Any others? These are interesting use cases that I hadn't
> > considered -
> > >> > this
> > >> > > is interesting.
> > >> > >
> > >> > > Jon
> > >> > >
> > >> > >
> > >> > >
> > >> > > On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <
> > >> > rmannibucau@gmail.com>
> > >> > > wrote:
> > >> > >
> > >> > > > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <
> danielsoro@gmail.com>
> > a
> > >> > > écrit
> > >> > > > :
> > >> > > >
> > >> > > > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
> > >> > > > > jonathan.gallimore@gmail.com> escreveu:
> > >> > > > >
> > >> > > > > > We should write a test case for both of these, and agree on
> > >> them.
> > >> > I'd
> > >> > > > > like
> > >> > > > > > to be able to merge this in with confidence, and without the
> > >> tests
> > >> > I
> > >> > > > > don't
> > >> > > > > > see how we can do that.
> > >> > > > > >
> > >> > > > > > Jon
> > >> > > > > >
> > >> > > > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
> > >> > > > > rmannibucau@gmail.com>
> > >> > > > > > wrote:
> > >> > > > > >
> > >> > > > > > > Hi Daniel,
> > >> > > > > > >
> > >> > > > > > > the point is that this code is reused in several places so
> > we
> > >> > must
> > >> > > be
> > >> > > > > as
> > >> > > > > > > clean as we can, here the cases I had in mind:
> > >> > > > > > >
> > >> > > > > > > 1. reused main in another main (so an app calls the main):
> > >> here
> > >> > the
> > >> > > > > > pitfall
> > >> > > > > > > is that the property
> > >> > openejb.classloader.first.disallow-system-load
> > >> > > > is
> > >> > > > > > not
> > >> > > > > > > resetted and the TCLL neither so after the main you use a
> > >> closed
> > >> > > > > > > classloader (so will likely easily fail)
> > >> > > > > >
> > >> > > > >
> > >> > > > > I really didn't get your point. Side note we are create a
> > >> classloader
> > >> > > > only
> > >> > > > > for tomee cli execution, which keep alive only for a short
> time
> > >> > during
> > >> > > > > command execution.
> > >> > > > >
> > >> > > > > That part of code is not reused in another place, if yes, just
> > >> point
> > >> > me
> > >> > > > > where, because I can't find it.
> > >> > > > >
> > >> > > > > It will close only after the program finish, since it is
> inside
> > a
> > >> > > > try-catch
> > >> > > > > block, no?
> > >> > > > >
> > >> > > >
> > >> > > > Assuming the main is launch but this main is sometimes launched
> > >> > > > programmatically (like in a webapp)
> > >> > > > When not you are right
> > >> > > >
> > >> > > >
> > >> > > > >
> > >> > > > >
> > >> > > > > > > 2. if a user relies in java 8 on the classloader being the
> > >> system
> > >> > > one
> > >> > > > > > then
> > >> > > > > > > your new classloader level breaks it so we shouldnt create
> > >> this
> > >> > new
> > >> > > > > > > classloader when not needed and the old ClassPath impl is
> > >> working
> > >> > > > IMO.
> > >> > > > > We
> > >> > > > > > > can enrich it with a warn log saying "it will fail in next
> > >> > version"
> > >> > > > > (and
> > >> > > > > > we
> > >> > > > > > > drop that code after 1 or 2 releases). Idea is to not
> break
> > >> > > directly
> > >> > > > > > users
> > >> > > > > > > who can not even know some of the libs they added in the
> > >> > classpath
> > >> > > > for
> > >> > > > > > > logging purposes rely on that classloader setup - random
> > >> example
> > >> > > > indeed
> > >> > > > > > ;).
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > > > Yeah, I probably could use the SystemClassPath, but it is
> doing
> > >> much
> > >> > > more
> > >> > > > > than I need to CLI. I don't think that I need to change the
> > >> > > > java.class.path
> > >> > > > > property to make it works or other property like
> > >> > > > > openejb.classloader.first.disallow-system-load.
> > >> > > > >
> > >> > > >
> > >> > > > We should likely be cleaner here but not your PR, fair enough
> > >> > > >
> > >> > > >
> > >> > > > >
> > >> > > > > And to be honest, I didn't understand what is
> > >> > > > > openejb.classloader.first.disallow-system-load and why do we
> > have
> > >> it.
> > >> > > :)
> > >> > > > >
> > >> > > >
> > >> > > > It is used in the "webapp" loader to check if we should load
> some
> > >> well
> > >> > > > known classes from the system classloader or not.
> > >> > > > It is likely the kind of property you rely on when you want to
> be
> > >> > > > embeddable but also standalone.
> > >> > > >
> > >> > > >
> > >> > > > >
> > >> > > > >
> > >> > > > > > > Romain Manni-Bucau
> > >> > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > >> > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > >> > > > > > > <http://rmannibucau.wordpress.com> | Github <
> > >> > > > > > > https://github.com/rmannibucau> |
> > >> > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > >> > > > > > > <
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <
> > >> > danielsoro@apache.org>
> > >> > > a
> > >> > > > > > > écrit :
> > >> > > > > > >
> > >> > > > > > > > Hi,
> > >> > > > > > > >
> > >> > > > > > > > I updated the PR with tests!
> > >> > > > > > > >
> > >> > > > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> > >> > > > > > > > jonathan.gallimore@gmail.com> escreveu:
> > >> > > > > > > >
> > >> > > > > > > > > I saw your note on the PR regarding Romain's feedback.
> > >> > > > > > > > >
> > >> > > > > > > > > For visibility here, Romain is querying whether this
> > patch
> > >> > will
> > >> > > > > cause
> > >> > > > > > > an
> > >> > > > > > > > > issue with folks calling the CLI with a custom
> classpath
> > >> > using
> > >> > > > > `java
> > >> > > > > > > -cp
> > >> > > > > > > > > ...` under Java 8. I'd suggest we add a test for it,
> > which
> > >> > > looks
> > >> > > > to
> > >> > > > > > be
> > >> > > > > > > > what
> > >> > > > > > > > > Daniel is doing.
> > >> > > > > > > > >
> > >> > > > > > > > > Thanks guys.
> > >> > > > > > > > >
> > >> > > > > > > > > Jon
> > >> > > > > > > > >
> > >> > > > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
> > >> > > > > danielsoro@apache.org
> > >> > > > > > >
> > >> > > > > > > > > wrote:
> > >> > > > > > > > >
> > >> > > > > > > > > > Hi Folks,
> > >> > > > > > > > > >
> > >> > > > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a
> > lot
> > >> of
> > >> > > > > feedback
> > >> > > > > > > > from
> > >> > > > > > > > > > Romain (thank you Romain for all comments and help)
> > >> > > > > > > > > >
> > >> > > > > > > > > > I believe which we are in good shape to merge.
> > >> > > > > > > > > > The changes cover from Java 8 till Java 11 which not
> > >> > provide
> > >> > > > any
> > >> > > > > > > impact
> > >> > > > > > > > > for
> > >> > > > > > > > > > TomEE or application deployed on it.
> > >> > > > > > > > > >
> > >> > > > > > > > > > Please, look at the PR and let me know what you
> think.
> > >> > > > > > > > > >
> > >> > > > > > > > > > PR: https://github.com/apache/tomee/pull/176
> > >> > > > > > > > > >
> > >> > > > > > > > > > --
> > >> > > > > > > > > > Daniel "soro" Cunha
> > >> > > > > > > > > > https://twitter.com/dvlc_
> > >> > > > > > > > > >
> > >> > > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > --
> > >> > > > > > > > Daniel "soro" Cunha
> > >> > > > > > > > https://twitter.com/dvlc_
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > > > Thank you! :)
> > >> > > > > --
> > >> > > > > Daniel "soro" Cunha
> > >> > > > > https://twitter.com/dvlc_
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >> >
> > >> > --
> > >> > Daniel "soro" Cunha
> > >> > https://twitter.com/dvlc_
> > >> >
> > >>
> > >
> > >
> > > --
> > > Daniel "soro" Cunha
> > > https://twitter.com/dvlc_
> > >
> >
> >
> > --
> > Daniel "soro" Cunha
> > https://twitter.com/dvlc_
> >
>
--
Daniel "soro" Cunha
https://twitter.com/dvlc_
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi Daniel
Do you think you can assert the classloader in the execution thread didnt
change after the call? Was one concern and i didnt find the matching assert
Le mar. 20 nov. 2018 20:00, Daniel Cunha <da...@apache.org> a écrit :
> Hi folks,
>
> I've added a new test with JAXRS, calling a command in my endpoint from
> Bootstrap CLI.
> Please review the changes on my PR[1] and let me know what do you think
> about it.
>
> Thank you. :)
>
> [1] https://github.com/apache/tomee/pull/176
>
> Em seg, 19 de nov de 2018 às 13:57, Daniel Cunha <da...@gmail.com>
> escreveu:
>
> > Hi Jon,
> >
> > ok. I got it! :)
> >
> > Em seg, 19 de nov de 2018 às 13:49, Jonathan Gallimore <
> > jonathan.gallimore@gmail.com> escreveu:
> >
> >> On Mon, Nov 19, 2018 at 4:30 PM Daniel Cunha <da...@gmail.com>
> >> wrote:
> >>
> >> > Hi guys,
> >> >
> >> > I updated the PR with some chages:
> >> > 1 - Move BasicURLClassPath.CustomizableURLClassLoader to be public.
> >> > 2 - I'm using BasicURLClassPath.CustomizableURLClassLoader instead of
> >> > DynamicClassLoader (class that I created)
> >> > 3- DynamicClassLoader is not needed anymore. Was removed from the PR.
> >> > 3 - Added more tests:
> >> > - Standalone client that has its own main and delegates to the CLI
> >> main
> >> > - As above, but perhaps with the standalone client creating a
> >> > classloader
> >> > with the CLI jars and then launching it
> >> >
> >> > Question:
> >> >
> >> > @Jon,
> >> > what do you want me mean with: A webapp calling commands in the CLI?
> >> >
> >>
> >> Hopefully I can answer that one in a straightforward way :). Try
> creating
> >> a
> >> servlet that loads the Bootstrap CLI class, and invokes some methods.
> >> Effectively you'd be calling the TomEE command line commands from a
> >> servlet. That use case hadn't occurred to me, but it sounds like one
> worth
> >> trying.
> >>
> >> That would probably need to sit
> >> under
> arquillian/arquillian-tomee-tests/arquillian-tomee-webprofile-tests.
> >>
> >> Jon
> >>
> >>
> >> >
> >> > Command is a class with main method, if I've it deployed in my library
> >> > folder, it should works fine, I just need to create a instance and
> call
> >> the
> >> > main.
> >> > Same will be if I try execute it with Process (that is exactly what we
> >> are
> >> > testing).
> >> >
> >> > Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
> >> > jonathan.gallimore@gmail.com> escreveu:
> >> >
> >> > > I'm following this thread with some interest. What would be some
> good
> >> > tests
> >> > > to make sure these cases are covered off? Some suggestions in
> >> addition to
> >> > > the existing test:
> >> > >
> >> > > * Standalone client that has its own main and delegates to the CLI
> >> main
> >> > > * As above, but perhaps with the standalone client creating a
> >> classloader
> >> > > with the CLI jars and then launching it
> >> > > * A webapp calling commands in the CLI
> >> > >
> >> > > Any others? These are interesting use cases that I hadn't
> considered -
> >> > this
> >> > > is interesting.
> >> > >
> >> > > Jon
> >> > >
> >> > >
> >> > >
> >> > > On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <
> >> > rmannibucau@gmail.com>
> >> > > wrote:
> >> > >
> >> > > > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <da...@gmail.com>
> a
> >> > > écrit
> >> > > > :
> >> > > >
> >> > > > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
> >> > > > > jonathan.gallimore@gmail.com> escreveu:
> >> > > > >
> >> > > > > > We should write a test case for both of these, and agree on
> >> them.
> >> > I'd
> >> > > > > like
> >> > > > > > to be able to merge this in with confidence, and without the
> >> tests
> >> > I
> >> > > > > don't
> >> > > > > > see how we can do that.
> >> > > > > >
> >> > > > > > Jon
> >> > > > > >
> >> > > > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
> >> > > > > rmannibucau@gmail.com>
> >> > > > > > wrote:
> >> > > > > >
> >> > > > > > > Hi Daniel,
> >> > > > > > >
> >> > > > > > > the point is that this code is reused in several places so
> we
> >> > must
> >> > > be
> >> > > > > as
> >> > > > > > > clean as we can, here the cases I had in mind:
> >> > > > > > >
> >> > > > > > > 1. reused main in another main (so an app calls the main):
> >> here
> >> > the
> >> > > > > > pitfall
> >> > > > > > > is that the property
> >> > openejb.classloader.first.disallow-system-load
> >> > > > is
> >> > > > > > not
> >> > > > > > > resetted and the TCLL neither so after the main you use a
> >> closed
> >> > > > > > > classloader (so will likely easily fail)
> >> > > > > >
> >> > > > >
> >> > > > > I really didn't get your point. Side note we are create a
> >> classloader
> >> > > > only
> >> > > > > for tomee cli execution, which keep alive only for a short time
> >> > during
> >> > > > > command execution.
> >> > > > >
> >> > > > > That part of code is not reused in another place, if yes, just
> >> point
> >> > me
> >> > > > > where, because I can't find it.
> >> > > > >
> >> > > > > It will close only after the program finish, since it is inside
> a
> >> > > > try-catch
> >> > > > > block, no?
> >> > > > >
> >> > > >
> >> > > > Assuming the main is launch but this main is sometimes launched
> >> > > > programmatically (like in a webapp)
> >> > > > When not you are right
> >> > > >
> >> > > >
> >> > > > >
> >> > > > >
> >> > > > > > > 2. if a user relies in java 8 on the classloader being the
> >> system
> >> > > one
> >> > > > > > then
> >> > > > > > > your new classloader level breaks it so we shouldnt create
> >> this
> >> > new
> >> > > > > > > classloader when not needed and the old ClassPath impl is
> >> working
> >> > > > IMO.
> >> > > > > We
> >> > > > > > > can enrich it with a warn log saying "it will fail in next
> >> > version"
> >> > > > > (and
> >> > > > > > we
> >> > > > > > > drop that code after 1 or 2 releases). Idea is to not break
> >> > > directly
> >> > > > > > users
> >> > > > > > > who can not even know some of the libs they added in the
> >> > classpath
> >> > > > for
> >> > > > > > > logging purposes rely on that classloader setup - random
> >> example
> >> > > > indeed
> >> > > > > > ;).
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > > > Yeah, I probably could use the SystemClassPath, but it is doing
> >> much
> >> > > more
> >> > > > > than I need to CLI. I don't think that I need to change the
> >> > > > java.class.path
> >> > > > > property to make it works or other property like
> >> > > > > openejb.classloader.first.disallow-system-load.
> >> > > > >
> >> > > >
> >> > > > We should likely be cleaner here but not your PR, fair enough
> >> > > >
> >> > > >
> >> > > > >
> >> > > > > And to be honest, I didn't understand what is
> >> > > > > openejb.classloader.first.disallow-system-load and why do we
> have
> >> it.
> >> > > :)
> >> > > > >
> >> > > >
> >> > > > It is used in the "webapp" loader to check if we should load some
> >> well
> >> > > > known classes from the system classloader or not.
> >> > > > It is likely the kind of property you rely on when you want to be
> >> > > > embeddable but also standalone.
> >> > > >
> >> > > >
> >> > > > >
> >> > > > >
> >> > > > > > > Romain Manni-Bucau
> >> > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >> > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> >> > > > > > > <http://rmannibucau.wordpress.com> | Github <
> >> > > > > > > https://github.com/rmannibucau> |
> >> > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >> > > > > > > <
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >> > > > > > > >
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <
> >> > danielsoro@apache.org>
> >> > > a
> >> > > > > > > écrit :
> >> > > > > > >
> >> > > > > > > > Hi,
> >> > > > > > > >
> >> > > > > > > > I updated the PR with tests!
> >> > > > > > > >
> >> > > > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> >> > > > > > > > jonathan.gallimore@gmail.com> escreveu:
> >> > > > > > > >
> >> > > > > > > > > I saw your note on the PR regarding Romain's feedback.
> >> > > > > > > > >
> >> > > > > > > > > For visibility here, Romain is querying whether this
> patch
> >> > will
> >> > > > > cause
> >> > > > > > > an
> >> > > > > > > > > issue with folks calling the CLI with a custom classpath
> >> > using
> >> > > > > `java
> >> > > > > > > -cp
> >> > > > > > > > > ...` under Java 8. I'd suggest we add a test for it,
> which
> >> > > looks
> >> > > > to
> >> > > > > > be
> >> > > > > > > > what
> >> > > > > > > > > Daniel is doing.
> >> > > > > > > > >
> >> > > > > > > > > Thanks guys.
> >> > > > > > > > >
> >> > > > > > > > > Jon
> >> > > > > > > > >
> >> > > > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
> >> > > > > danielsoro@apache.org
> >> > > > > > >
> >> > > > > > > > > wrote:
> >> > > > > > > > >
> >> > > > > > > > > > Hi Folks,
> >> > > > > > > > > >
> >> > > > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a
> lot
> >> of
> >> > > > > feedback
> >> > > > > > > > from
> >> > > > > > > > > > Romain (thank you Romain for all comments and help)
> >> > > > > > > > > >
> >> > > > > > > > > > I believe which we are in good shape to merge.
> >> > > > > > > > > > The changes cover from Java 8 till Java 11 which not
> >> > provide
> >> > > > any
> >> > > > > > > impact
> >> > > > > > > > > for
> >> > > > > > > > > > TomEE or application deployed on it.
> >> > > > > > > > > >
> >> > > > > > > > > > Please, look at the PR and let me know what you think.
> >> > > > > > > > > >
> >> > > > > > > > > > PR: https://github.com/apache/tomee/pull/176
> >> > > > > > > > > >
> >> > > > > > > > > > --
> >> > > > > > > > > > Daniel "soro" Cunha
> >> > > > > > > > > > https://twitter.com/dvlc_
> >> > > > > > > > > >
> >> > > > > > > > >
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > --
> >> > > > > > > > Daniel "soro" Cunha
> >> > > > > > > > https://twitter.com/dvlc_
> >> > > > > > > >
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > > > Thank you! :)
> >> > > > > --
> >> > > > > Daniel "soro" Cunha
> >> > > > > https://twitter.com/dvlc_
> >> > > > >
> >> > > >
> >> > >
> >> >
> >> >
> >> > --
> >> > Daniel "soro" Cunha
> >> > https://twitter.com/dvlc_
> >> >
> >>
> >
> >
> > --
> > Daniel "soro" Cunha
> > https://twitter.com/dvlc_
> >
>
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Daniel Cunha <da...@apache.org>.
Hi folks,
I've added a new test with JAXRS, calling a command in my endpoint from
Bootstrap CLI.
Please review the changes on my PR[1] and let me know what do you think
about it.
Thank you. :)
[1] https://github.com/apache/tomee/pull/176
Em seg, 19 de nov de 2018 às 13:57, Daniel Cunha <da...@gmail.com>
escreveu:
> Hi Jon,
>
> ok. I got it! :)
>
> Em seg, 19 de nov de 2018 às 13:49, Jonathan Gallimore <
> jonathan.gallimore@gmail.com> escreveu:
>
>> On Mon, Nov 19, 2018 at 4:30 PM Daniel Cunha <da...@gmail.com>
>> wrote:
>>
>> > Hi guys,
>> >
>> > I updated the PR with some chages:
>> > 1 - Move BasicURLClassPath.CustomizableURLClassLoader to be public.
>> > 2 - I'm using BasicURLClassPath.CustomizableURLClassLoader instead of
>> > DynamicClassLoader (class that I created)
>> > 3- DynamicClassLoader is not needed anymore. Was removed from the PR.
>> > 3 - Added more tests:
>> > - Standalone client that has its own main and delegates to the CLI
>> main
>> > - As above, but perhaps with the standalone client creating a
>> > classloader
>> > with the CLI jars and then launching it
>> >
>> > Question:
>> >
>> > @Jon,
>> > what do you want me mean with: A webapp calling commands in the CLI?
>> >
>>
>> Hopefully I can answer that one in a straightforward way :). Try creating
>> a
>> servlet that loads the Bootstrap CLI class, and invokes some methods.
>> Effectively you'd be calling the TomEE command line commands from a
>> servlet. That use case hadn't occurred to me, but it sounds like one worth
>> trying.
>>
>> That would probably need to sit
>> under arquillian/arquillian-tomee-tests/arquillian-tomee-webprofile-tests.
>>
>> Jon
>>
>>
>> >
>> > Command is a class with main method, if I've it deployed in my library
>> > folder, it should works fine, I just need to create a instance and call
>> the
>> > main.
>> > Same will be if I try execute it with Process (that is exactly what we
>> are
>> > testing).
>> >
>> > Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
>> > jonathan.gallimore@gmail.com> escreveu:
>> >
>> > > I'm following this thread with some interest. What would be some good
>> > tests
>> > > to make sure these cases are covered off? Some suggestions in
>> addition to
>> > > the existing test:
>> > >
>> > > * Standalone client that has its own main and delegates to the CLI
>> main
>> > > * As above, but perhaps with the standalone client creating a
>> classloader
>> > > with the CLI jars and then launching it
>> > > * A webapp calling commands in the CLI
>> > >
>> > > Any others? These are interesting use cases that I hadn't considered -
>> > this
>> > > is interesting.
>> > >
>> > > Jon
>> > >
>> > >
>> > >
>> > > On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <
>> > rmannibucau@gmail.com>
>> > > wrote:
>> > >
>> > > > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <da...@gmail.com> a
>> > > écrit
>> > > > :
>> > > >
>> > > > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
>> > > > > jonathan.gallimore@gmail.com> escreveu:
>> > > > >
>> > > > > > We should write a test case for both of these, and agree on
>> them.
>> > I'd
>> > > > > like
>> > > > > > to be able to merge this in with confidence, and without the
>> tests
>> > I
>> > > > > don't
>> > > > > > see how we can do that.
>> > > > > >
>> > > > > > Jon
>> > > > > >
>> > > > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
>> > > > > rmannibucau@gmail.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Hi Daniel,
>> > > > > > >
>> > > > > > > the point is that this code is reused in several places so we
>> > must
>> > > be
>> > > > > as
>> > > > > > > clean as we can, here the cases I had in mind:
>> > > > > > >
>> > > > > > > 1. reused main in another main (so an app calls the main):
>> here
>> > the
>> > > > > > pitfall
>> > > > > > > is that the property
>> > openejb.classloader.first.disallow-system-load
>> > > > is
>> > > > > > not
>> > > > > > > resetted and the TCLL neither so after the main you use a
>> closed
>> > > > > > > classloader (so will likely easily fail)
>> > > > > >
>> > > > >
>> > > > > I really didn't get your point. Side note we are create a
>> classloader
>> > > > only
>> > > > > for tomee cli execution, which keep alive only for a short time
>> > during
>> > > > > command execution.
>> > > > >
>> > > > > That part of code is not reused in another place, if yes, just
>> point
>> > me
>> > > > > where, because I can't find it.
>> > > > >
>> > > > > It will close only after the program finish, since it is inside a
>> > > > try-catch
>> > > > > block, no?
>> > > > >
>> > > >
>> > > > Assuming the main is launch but this main is sometimes launched
>> > > > programmatically (like in a webapp)
>> > > > When not you are right
>> > > >
>> > > >
>> > > > >
>> > > > >
>> > > > > > > 2. if a user relies in java 8 on the classloader being the
>> system
>> > > one
>> > > > > > then
>> > > > > > > your new classloader level breaks it so we shouldnt create
>> this
>> > new
>> > > > > > > classloader when not needed and the old ClassPath impl is
>> working
>> > > > IMO.
>> > > > > We
>> > > > > > > can enrich it with a warn log saying "it will fail in next
>> > version"
>> > > > > (and
>> > > > > > we
>> > > > > > > drop that code after 1 or 2 releases). Idea is to not break
>> > > directly
>> > > > > > users
>> > > > > > > who can not even know some of the libs they added in the
>> > classpath
>> > > > for
>> > > > > > > logging purposes rely on that classloader setup - random
>> example
>> > > > indeed
>> > > > > > ;).
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > > Yeah, I probably could use the SystemClassPath, but it is doing
>> much
>> > > more
>> > > > > than I need to CLI. I don't think that I need to change the
>> > > > java.class.path
>> > > > > property to make it works or other property like
>> > > > > openejb.classloader.first.disallow-system-load.
>> > > > >
>> > > >
>> > > > We should likely be cleaner here but not your PR, fair enough
>> > > >
>> > > >
>> > > > >
>> > > > > And to be honest, I didn't understand what is
>> > > > > openejb.classloader.first.disallow-system-load and why do we have
>> it.
>> > > :)
>> > > > >
>> > > >
>> > > > It is used in the "webapp" loader to check if we should load some
>> well
>> > > > known classes from the system classloader or not.
>> > > > It is likely the kind of property you rely on when you want to be
>> > > > embeddable but also standalone.
>> > > >
>> > > >
>> > > > >
>> > > > >
>> > > > > > > Romain Manni-Bucau
>> > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
>> > > > > > > <http://rmannibucau.wordpress.com> | Github <
>> > > > > > > https://github.com/rmannibucau> |
>> > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>> > > > > > > <
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> https://www.packtpub.com/application-development/java-ee-8-high-performance
>> > > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <
>> > danielsoro@apache.org>
>> > > a
>> > > > > > > écrit :
>> > > > > > >
>> > > > > > > > Hi,
>> > > > > > > >
>> > > > > > > > I updated the PR with tests!
>> > > > > > > >
>> > > > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
>> > > > > > > > jonathan.gallimore@gmail.com> escreveu:
>> > > > > > > >
>> > > > > > > > > I saw your note on the PR regarding Romain's feedback.
>> > > > > > > > >
>> > > > > > > > > For visibility here, Romain is querying whether this patch
>> > will
>> > > > > cause
>> > > > > > > an
>> > > > > > > > > issue with folks calling the CLI with a custom classpath
>> > using
>> > > > > `java
>> > > > > > > -cp
>> > > > > > > > > ...` under Java 8. I'd suggest we add a test for it, which
>> > > looks
>> > > > to
>> > > > > > be
>> > > > > > > > what
>> > > > > > > > > Daniel is doing.
>> > > > > > > > >
>> > > > > > > > > Thanks guys.
>> > > > > > > > >
>> > > > > > > > > Jon
>> > > > > > > > >
>> > > > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
>> > > > > danielsoro@apache.org
>> > > > > > >
>> > > > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > > Hi Folks,
>> > > > > > > > > >
>> > > > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot
>> of
>> > > > > feedback
>> > > > > > > > from
>> > > > > > > > > > Romain (thank you Romain for all comments and help)
>> > > > > > > > > >
>> > > > > > > > > > I believe which we are in good shape to merge.
>> > > > > > > > > > The changes cover from Java 8 till Java 11 which not
>> > provide
>> > > > any
>> > > > > > > impact
>> > > > > > > > > for
>> > > > > > > > > > TomEE or application deployed on it.
>> > > > > > > > > >
>> > > > > > > > > > Please, look at the PR and let me know what you think.
>> > > > > > > > > >
>> > > > > > > > > > PR: https://github.com/apache/tomee/pull/176
>> > > > > > > > > >
>> > > > > > > > > > --
>> > > > > > > > > > Daniel "soro" Cunha
>> > > > > > > > > > https://twitter.com/dvlc_
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > --
>> > > > > > > > Daniel "soro" Cunha
>> > > > > > > > https://twitter.com/dvlc_
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > > > Thank you! :)
>> > > > > --
>> > > > > Daniel "soro" Cunha
>> > > > > https://twitter.com/dvlc_
>> > > > >
>> > > >
>> > >
>> >
>> >
>> > --
>> > Daniel "soro" Cunha
>> > https://twitter.com/dvlc_
>> >
>>
>
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>
--
Daniel "soro" Cunha
https://twitter.com/dvlc_
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Daniel Cunha <da...@gmail.com>.
Hi Jon,
ok. I got it! :)
Em seg, 19 de nov de 2018 às 13:49, Jonathan Gallimore <
jonathan.gallimore@gmail.com> escreveu:
> On Mon, Nov 19, 2018 at 4:30 PM Daniel Cunha <da...@gmail.com> wrote:
>
> > Hi guys,
> >
> > I updated the PR with some chages:
> > 1 - Move BasicURLClassPath.CustomizableURLClassLoader to be public.
> > 2 - I'm using BasicURLClassPath.CustomizableURLClassLoader instead of
> > DynamicClassLoader (class that I created)
> > 3- DynamicClassLoader is not needed anymore. Was removed from the PR.
> > 3 - Added more tests:
> > - Standalone client that has its own main and delegates to the CLI
> main
> > - As above, but perhaps with the standalone client creating a
> > classloader
> > with the CLI jars and then launching it
> >
> > Question:
> >
> > @Jon,
> > what do you want me mean with: A webapp calling commands in the CLI?
> >
>
> Hopefully I can answer that one in a straightforward way :). Try creating a
> servlet that loads the Bootstrap CLI class, and invokes some methods.
> Effectively you'd be calling the TomEE command line commands from a
> servlet. That use case hadn't occurred to me, but it sounds like one worth
> trying.
>
> That would probably need to sit
> under arquillian/arquillian-tomee-tests/arquillian-tomee-webprofile-tests.
>
> Jon
>
>
> >
> > Command is a class with main method, if I've it deployed in my library
> > folder, it should works fine, I just need to create a instance and call
> the
> > main.
> > Same will be if I try execute it with Process (that is exactly what we
> are
> > testing).
> >
> > Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
> > jonathan.gallimore@gmail.com> escreveu:
> >
> > > I'm following this thread with some interest. What would be some good
> > tests
> > > to make sure these cases are covered off? Some suggestions in addition
> to
> > > the existing test:
> > >
> > > * Standalone client that has its own main and delegates to the CLI main
> > > * As above, but perhaps with the standalone client creating a
> classloader
> > > with the CLI jars and then launching it
> > > * A webapp calling commands in the CLI
> > >
> > > Any others? These are interesting use cases that I hadn't considered -
> > this
> > > is interesting.
> > >
> > > Jon
> > >
> > >
> > >
> > > On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <
> > rmannibucau@gmail.com>
> > > wrote:
> > >
> > > > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <da...@gmail.com> a
> > > écrit
> > > > :
> > > >
> > > > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
> > > > > jonathan.gallimore@gmail.com> escreveu:
> > > > >
> > > > > > We should write a test case for both of these, and agree on them.
> > I'd
> > > > > like
> > > > > > to be able to merge this in with confidence, and without the
> tests
> > I
> > > > > don't
> > > > > > see how we can do that.
> > > > > >
> > > > > > Jon
> > > > > >
> > > > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
> > > > > rmannibucau@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Daniel,
> > > > > > >
> > > > > > > the point is that this code is reused in several places so we
> > must
> > > be
> > > > > as
> > > > > > > clean as we can, here the cases I had in mind:
> > > > > > >
> > > > > > > 1. reused main in another main (so an app calls the main): here
> > the
> > > > > > pitfall
> > > > > > > is that the property
> > openejb.classloader.first.disallow-system-load
> > > > is
> > > > > > not
> > > > > > > resetted and the TCLL neither so after the main you use a
> closed
> > > > > > > classloader (so will likely easily fail)
> > > > > >
> > > > >
> > > > > I really didn't get your point. Side note we are create a
> classloader
> > > > only
> > > > > for tomee cli execution, which keep alive only for a short time
> > during
> > > > > command execution.
> > > > >
> > > > > That part of code is not reused in another place, if yes, just
> point
> > me
> > > > > where, because I can't find it.
> > > > >
> > > > > It will close only after the program finish, since it is inside a
> > > > try-catch
> > > > > block, no?
> > > > >
> > > >
> > > > Assuming the main is launch but this main is sometimes launched
> > > > programmatically (like in a webapp)
> > > > When not you are right
> > > >
> > > >
> > > > >
> > > > >
> > > > > > > 2. if a user relies in java 8 on the classloader being the
> system
> > > one
> > > > > > then
> > > > > > > your new classloader level breaks it so we shouldnt create this
> > new
> > > > > > > classloader when not needed and the old ClassPath impl is
> working
> > > > IMO.
> > > > > We
> > > > > > > can enrich it with a warn log saying "it will fail in next
> > version"
> > > > > (and
> > > > > > we
> > > > > > > drop that code after 1 or 2 releases). Idea is to not break
> > > directly
> > > > > > users
> > > > > > > who can not even know some of the libs they added in the
> > classpath
> > > > for
> > > > > > > logging purposes rely on that classloader setup - random
> example
> > > > indeed
> > > > > > ;).
> > > > > > >
> > > > > >
> > > > >
> > > > > Yeah, I probably could use the SystemClassPath, but it is doing
> much
> > > more
> > > > > than I need to CLI. I don't think that I need to change the
> > > > java.class.path
> > > > > property to make it works or other property like
> > > > > openejb.classloader.first.disallow-system-load.
> > > > >
> > > >
> > > > We should likely be cleaner here but not your PR, fair enough
> > > >
> > > >
> > > > >
> > > > > And to be honest, I didn't understand what is
> > > > > openejb.classloader.first.disallow-system-load and why do we have
> it.
> > > :)
> > > > >
> > > >
> > > > It is used in the "webapp" loader to check if we should load some
> well
> > > > known classes from the system classloader or not.
> > > > It is likely the kind of property you rely on when you want to be
> > > > embeddable but also standalone.
> > > >
> > > >
> > > > >
> > > > >
> > > > > > > Romain Manni-Bucau
> > > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > > <http://rmannibucau.wordpress.com> | Github <
> > > > > > > https://github.com/rmannibucau> |
> > > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > > > > > > <
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <
> > danielsoro@apache.org>
> > > a
> > > > > > > écrit :
> > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > I updated the PR with tests!
> > > > > > > >
> > > > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> > > > > > > > jonathan.gallimore@gmail.com> escreveu:
> > > > > > > >
> > > > > > > > > I saw your note on the PR regarding Romain's feedback.
> > > > > > > > >
> > > > > > > > > For visibility here, Romain is querying whether this patch
> > will
> > > > > cause
> > > > > > > an
> > > > > > > > > issue with folks calling the CLI with a custom classpath
> > using
> > > > > `java
> > > > > > > -cp
> > > > > > > > > ...` under Java 8. I'd suggest we add a test for it, which
> > > looks
> > > > to
> > > > > > be
> > > > > > > > what
> > > > > > > > > Daniel is doing.
> > > > > > > > >
> > > > > > > > > Thanks guys.
> > > > > > > > >
> > > > > > > > > Jon
> > > > > > > > >
> > > > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
> > > > > danielsoro@apache.org
> > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Folks,
> > > > > > > > > >
> > > > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot
> of
> > > > > feedback
> > > > > > > > from
> > > > > > > > > > Romain (thank you Romain for all comments and help)
> > > > > > > > > >
> > > > > > > > > > I believe which we are in good shape to merge.
> > > > > > > > > > The changes cover from Java 8 till Java 11 which not
> > provide
> > > > any
> > > > > > > impact
> > > > > > > > > for
> > > > > > > > > > TomEE or application deployed on it.
> > > > > > > > > >
> > > > > > > > > > Please, look at the PR and let me know what you think.
> > > > > > > > > >
> > > > > > > > > > PR: https://github.com/apache/tomee/pull/176
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Daniel "soro" Cunha
> > > > > > > > > > https://twitter.com/dvlc_
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Daniel "soro" Cunha
> > > > > > > > https://twitter.com/dvlc_
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > > Thank you! :)
> > > > > --
> > > > > Daniel "soro" Cunha
> > > > > https://twitter.com/dvlc_
> > > > >
> > > >
> > >
> >
> >
> > --
> > Daniel "soro" Cunha
> > https://twitter.com/dvlc_
> >
>
--
Daniel "soro" Cunha
https://twitter.com/dvlc_
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Jonathan Gallimore <jo...@gmail.com>.
On Mon, Nov 19, 2018 at 4:30 PM Daniel Cunha <da...@gmail.com> wrote:
> Hi guys,
>
> I updated the PR with some chages:
> 1 - Move BasicURLClassPath.CustomizableURLClassLoader to be public.
> 2 - I'm using BasicURLClassPath.CustomizableURLClassLoader instead of
> DynamicClassLoader (class that I created)
> 3- DynamicClassLoader is not needed anymore. Was removed from the PR.
> 3 - Added more tests:
> - Standalone client that has its own main and delegates to the CLI main
> - As above, but perhaps with the standalone client creating a
> classloader
> with the CLI jars and then launching it
>
> Question:
>
> @Jon,
> what do you want me mean with: A webapp calling commands in the CLI?
>
Hopefully I can answer that one in a straightforward way :). Try creating a
servlet that loads the Bootstrap CLI class, and invokes some methods.
Effectively you'd be calling the TomEE command line commands from a
servlet. That use case hadn't occurred to me, but it sounds like one worth
trying.
That would probably need to sit
under arquillian/arquillian-tomee-tests/arquillian-tomee-webprofile-tests.
Jon
>
> Command is a class with main method, if I've it deployed in my library
> folder, it should works fine, I just need to create a instance and call the
> main.
> Same will be if I try execute it with Process (that is exactly what we are
> testing).
>
> Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
> jonathan.gallimore@gmail.com> escreveu:
>
> > I'm following this thread with some interest. What would be some good
> tests
> > to make sure these cases are covered off? Some suggestions in addition to
> > the existing test:
> >
> > * Standalone client that has its own main and delegates to the CLI main
> > * As above, but perhaps with the standalone client creating a classloader
> > with the CLI jars and then launching it
> > * A webapp calling commands in the CLI
> >
> > Any others? These are interesting use cases that I hadn't considered -
> this
> > is interesting.
> >
> > Jon
> >
> >
> >
> > On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <
> rmannibucau@gmail.com>
> > wrote:
> >
> > > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <da...@gmail.com> a
> > écrit
> > > :
> > >
> > > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
> > > > jonathan.gallimore@gmail.com> escreveu:
> > > >
> > > > > We should write a test case for both of these, and agree on them.
> I'd
> > > > like
> > > > > to be able to merge this in with confidence, and without the tests
> I
> > > > don't
> > > > > see how we can do that.
> > > > >
> > > > > Jon
> > > > >
> > > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
> > > > rmannibucau@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Daniel,
> > > > > >
> > > > > > the point is that this code is reused in several places so we
> must
> > be
> > > > as
> > > > > > clean as we can, here the cases I had in mind:
> > > > > >
> > > > > > 1. reused main in another main (so an app calls the main): here
> the
> > > > > pitfall
> > > > > > is that the property
> openejb.classloader.first.disallow-system-load
> > > is
> > > > > not
> > > > > > resetted and the TCLL neither so after the main you use a closed
> > > > > > classloader (so will likely easily fail)
> > > > >
> > > >
> > > > I really didn't get your point. Side note we are create a classloader
> > > only
> > > > for tomee cli execution, which keep alive only for a short time
> during
> > > > command execution.
> > > >
> > > > That part of code is not reused in another place, if yes, just point
> me
> > > > where, because I can't find it.
> > > >
> > > > It will close only after the program finish, since it is inside a
> > > try-catch
> > > > block, no?
> > > >
> > >
> > > Assuming the main is launch but this main is sometimes launched
> > > programmatically (like in a webapp)
> > > When not you are right
> > >
> > >
> > > >
> > > >
> > > > > > 2. if a user relies in java 8 on the classloader being the system
> > one
> > > > > then
> > > > > > your new classloader level breaks it so we shouldnt create this
> new
> > > > > > classloader when not needed and the old ClassPath impl is working
> > > IMO.
> > > > We
> > > > > > can enrich it with a warn log saying "it will fail in next
> version"
> > > > (and
> > > > > we
> > > > > > drop that code after 1 or 2 releases). Idea is to not break
> > directly
> > > > > users
> > > > > > who can not even know some of the libs they added in the
> classpath
> > > for
> > > > > > logging purposes rely on that classloader setup - random example
> > > indeed
> > > > > ;).
> > > > > >
> > > > >
> > > >
> > > > Yeah, I probably could use the SystemClassPath, but it is doing much
> > more
> > > > than I need to CLI. I don't think that I need to change the
> > > java.class.path
> > > > property to make it works or other property like
> > > > openejb.classloader.first.disallow-system-load.
> > > >
> > >
> > > We should likely be cleaner here but not your PR, fair enough
> > >
> > >
> > > >
> > > > And to be honest, I didn't understand what is
> > > > openejb.classloader.first.disallow-system-load and why do we have it.
> > :)
> > > >
> > >
> > > It is used in the "webapp" loader to check if we should load some well
> > > known classes from the system classloader or not.
> > > It is likely the kind of property you rely on when you want to be
> > > embeddable but also standalone.
> > >
> > >
> > > >
> > > >
> > > > > > Romain Manni-Bucau
> > > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > > <http://rmannibucau.wordpress.com> | Github <
> > > > > > https://github.com/rmannibucau> |
> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > > > > > <
> > > > > >
> > > > >
> > > >
> > >
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > > > > > >
> > > > > >
> > > > > >
> > > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <
> danielsoro@apache.org>
> > a
> > > > > > écrit :
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I updated the PR with tests!
> > > > > > >
> > > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> > > > > > > jonathan.gallimore@gmail.com> escreveu:
> > > > > > >
> > > > > > > > I saw your note on the PR regarding Romain's feedback.
> > > > > > > >
> > > > > > > > For visibility here, Romain is querying whether this patch
> will
> > > > cause
> > > > > > an
> > > > > > > > issue with folks calling the CLI with a custom classpath
> using
> > > > `java
> > > > > > -cp
> > > > > > > > ...` under Java 8. I'd suggest we add a test for it, which
> > looks
> > > to
> > > > > be
> > > > > > > what
> > > > > > > > Daniel is doing.
> > > > > > > >
> > > > > > > > Thanks guys.
> > > > > > > >
> > > > > > > > Jon
> > > > > > > >
> > > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
> > > > danielsoro@apache.org
> > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Folks,
> > > > > > > > >
> > > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot of
> > > > feedback
> > > > > > > from
> > > > > > > > > Romain (thank you Romain for all comments and help)
> > > > > > > > >
> > > > > > > > > I believe which we are in good shape to merge.
> > > > > > > > > The changes cover from Java 8 till Java 11 which not
> provide
> > > any
> > > > > > impact
> > > > > > > > for
> > > > > > > > > TomEE or application deployed on it.
> > > > > > > > >
> > > > > > > > > Please, look at the PR and let me know what you think.
> > > > > > > > >
> > > > > > > > > PR: https://github.com/apache/tomee/pull/176
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Daniel "soro" Cunha
> > > > > > > > > https://twitter.com/dvlc_
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Daniel "soro" Cunha
> > > > > > > https://twitter.com/dvlc_
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > > Thank you! :)
> > > > --
> > > > Daniel "soro" Cunha
> > > > https://twitter.com/dvlc_
> > > >
> > >
> >
>
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>
Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11
Posted by Daniel Cunha <da...@gmail.com>.
Hi guys,
I updated the PR with some chages:
1 - Move BasicURLClassPath.CustomizableURLClassLoader to be public.
2 - I'm using BasicURLClassPath.CustomizableURLClassLoader instead of
DynamicClassLoader (class that I created)
3- DynamicClassLoader is not needed anymore. Was removed from the PR.
3 - Added more tests:
- Standalone client that has its own main and delegates to the CLI main
- As above, but perhaps with the standalone client creating a
classloader
with the CLI jars and then launching it
Question:
@Jon,
what do you want me mean with: A webapp calling commands in the CLI?
Command is a class with main method, if I've it deployed in my library
folder, it should works fine, I just need to create a instance and call the
main.
Same will be if I try execute it with Process (that is exactly what we are
testing).
Em sex, 2 de nov de 2018 às 07:53, Jonathan Gallimore <
jonathan.gallimore@gmail.com> escreveu:
> I'm following this thread with some interest. What would be some good tests
> to make sure these cases are covered off? Some suggestions in addition to
> the existing test:
>
> * Standalone client that has its own main and delegates to the CLI main
> * As above, but perhaps with the standalone client creating a classloader
> with the CLI jars and then launching it
> * A webapp calling commands in the CLI
>
> Any others? These are interesting use cases that I hadn't considered - this
> is interesting.
>
> Jon
>
>
>
> On Wed, Oct 31, 2018 at 2:30 PM Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
> > Le mer. 31 oct. 2018 à 14:52, Daniel Cunha <da...@gmail.com> a
> écrit
> > :
> >
> > > Em seg, 29 de out de 2018 às 14:01, Jonathan Gallimore <
> > > jonathan.gallimore@gmail.com> escreveu:
> > >
> > > > We should write a test case for both of these, and agree on them. I'd
> > > like
> > > > to be able to merge this in with confidence, and without the tests I
> > > don't
> > > > see how we can do that.
> > > >
> > > > Jon
> > > >
> > > > On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau <
> > > rmannibucau@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Daniel,
> > > > >
> > > > > the point is that this code is reused in several places so we must
> be
> > > as
> > > > > clean as we can, here the cases I had in mind:
> > > > >
> > > > > 1. reused main in another main (so an app calls the main): here the
> > > > pitfall
> > > > > is that the property openejb.classloader.first.disallow-system-load
> > is
> > > > not
> > > > > resetted and the TCLL neither so after the main you use a closed
> > > > > classloader (so will likely easily fail)
> > > >
> > >
> > > I really didn't get your point. Side note we are create a classloader
> > only
> > > for tomee cli execution, which keep alive only for a short time during
> > > command execution.
> > >
> > > That part of code is not reused in another place, if yes, just point me
> > > where, because I can't find it.
> > >
> > > It will close only after the program finish, since it is inside a
> > try-catch
> > > block, no?
> > >
> >
> > Assuming the main is launch but this main is sometimes launched
> > programmatically (like in a webapp)
> > When not you are right
> >
> >
> > >
> > >
> > > > > 2. if a user relies in java 8 on the classloader being the system
> one
> > > > then
> > > > > your new classloader level breaks it so we shouldnt create this new
> > > > > classloader when not needed and the old ClassPath impl is working
> > IMO.
> > > We
> > > > > can enrich it with a warn log saying "it will fail in next version"
> > > (and
> > > > we
> > > > > drop that code after 1 or 2 releases). Idea is to not break
> directly
> > > > users
> > > > > who can not even know some of the libs they added in the classpath
> > for
> > > > > logging purposes rely on that classloader setup - random example
> > indeed
> > > > ;).
> > > > >
> > > >
> > >
> > > Yeah, I probably could use the SystemClassPath, but it is doing much
> more
> > > than I need to CLI. I don't think that I need to change the
> > java.class.path
> > > property to make it works or other property like
> > > openejb.classloader.first.disallow-system-load.
> > >
> >
> > We should likely be cleaner here but not your PR, fair enough
> >
> >
> > >
> > > And to be honest, I didn't understand what is
> > > openejb.classloader.first.disallow-system-load and why do we have it.
> :)
> > >
> >
> > It is used in the "webapp" loader to check if we should load some well
> > known classes from the system classloader or not.
> > It is likely the kind of property you rely on when you want to be
> > embeddable but also standalone.
> >
> >
> > >
> > >
> > > > > Romain Manni-Bucau
> > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > > > > <https://rmannibucau.metawerx.net/> | Old Blog
> > > > > <http://rmannibucau.wordpress.com> | Github <
> > > > > https://github.com/rmannibucau> |
> > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > > > > <
> > > > >
> > > >
> > >
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > > > > >
> > > > >
> > > > >
> > > > > Le lun. 29 oct. 2018 à 16:14, Daniel Cunha <da...@apache.org>
> a
> > > > > écrit :
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I updated the PR with tests!
> > > > > >
> > > > > > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> > > > > > jonathan.gallimore@gmail.com> escreveu:
> > > > > >
> > > > > > > I saw your note on the PR regarding Romain's feedback.
> > > > > > >
> > > > > > > For visibility here, Romain is querying whether this patch will
> > > cause
> > > > > an
> > > > > > > issue with folks calling the CLI with a custom classpath using
> > > `java
> > > > > -cp
> > > > > > > ...` under Java 8. I'd suggest we add a test for it, which
> looks
> > to
> > > > be
> > > > > > what
> > > > > > > Daniel is doing.
> > > > > > >
> > > > > > > Thanks guys.
> > > > > > >
> > > > > > > Jon
> > > > > > >
> > > > > > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha <
> > > danielsoro@apache.org
> > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Folks,
> > > > > > > >
> > > > > > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot of
> > > feedback
> > > > > > from
> > > > > > > > Romain (thank you Romain for all comments and help)
> > > > > > > >
> > > > > > > > I believe which we are in good shape to merge.
> > > > > > > > The changes cover from Java 8 till Java 11 which not provide
> > any
> > > > > impact
> > > > > > > for
> > > > > > > > TomEE or application deployed on it.
> > > > > > > >
> > > > > > > > Please, look at the PR and let me know what you think.
> > > > > > > >
> > > > > > > > PR: https://github.com/apache/tomee/pull/176
> > > > > > > >
> > > > > > > > --
> > > > > > > > Daniel "soro" Cunha
> > > > > > > > https://twitter.com/dvlc_
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Daniel "soro" Cunha
> > > > > > https://twitter.com/dvlc_
> > > > > >
> > > > >
> > > >
> > >
> > > Thank you! :)
> > > --
> > > Daniel "soro" Cunha
> > > https://twitter.com/dvlc_
> > >
> >
>
--
Daniel "soro" Cunha
https://twitter.com/dvlc_