Hi, Leo, I really get your point that we can't change this stuff. Changing SPI stuff (even just renaming packages) will break application server integation code, we all got it now..
That's why I suggested (a few mails ago, but unfortunately too vague) keeping the package names *.spi, *.spi.impl and *.config.element as is, but still moving *.spi and *.config.element to a new module called myfaces-spi. This alone will let us be able to remove shaded-impl, no code change is actually required, just moving some classes from myfaces-impl into myfaces-spi. In the end (--> the myfaces-impl jar), it will all be back in myfaces-impl again, because of shade. I will provide a new patch by tomorrow and then start a vote for it. There really is no technical reason for not committing such a solution at this point. Regards, Jakob 2011/7/11 Leonardo Uribe <lu4...@gmail.com>: > Hi > > I'll be clear and direct. What makes statements true or false are the > reasons behind it. Until this moment, you are not question the reasons > behind the reasoning proposed. > > To be short, my argumentation is we can't change for now: > > 1. Everything inside org.apache.myfaces.spi > 2. LifecycleProvided > 3. Everything inside org.apache.myfaces.config.element > > because JSF 2.1 is a maintenance release (not a mayor release) which > already has it first version (but even without a version). Do that > will create bugs on server integration code. The problem is there is > no way to detect such changes or create a proper patch from the server > side point of view without use some ugly reflection code and that > really s...cks!. > > Let it to JSF 2.2 (which is another JSR) sounds better because in that > time I think we can get rid of implee6 too in one move!. In that > version, just change the poms, and move all code to impl and that's > it. > > In conclusion, here we have a technical restriction that doesn't allow > us to move further, so we can't really make a vote because there is no > decision to make, we just can't change the code!. The idea of an API > in impl module is precisely make the "promise" that we will be nice > and do not change that stuff until the next major version. > > Unfortunately there is nothing we can do in 2.1.x branch. > > regards, > > Leonardo Uribe > > 2011/7/11 Jakob Korherr <jakob.korh...@gmail.com>: >> Hi, >> >> No, sorry Leo, they are not enough. Frankly, I don't understand why >> you are blocking this solution. It is easy, it does not break anything >> (if we do not change the package names), it is a lot more clean and we >> can get rid of the shaded-impl module. If we don't do this now, we >> will maybe have to use this ugly module for a long time.. >> >> And yes: in my opinion it is an epic fail. If you think otherwise, >> that's ok, but just because you say so and I don't does not make your >> statement true. >> >> I think we have to start a vote for this one just like we did with the >> resource-handler stuff. >> >> Regards, >> Jakob >> >> 2011/7/11 Leonardo Uribe <lu4...@gmail.com>: >>> Hi >>> >>> 2011/7/11 Jakob Korherr <jakob.korh...@gmail.com>: >>>> Hi, >>>> >>>>> 1. All classes in org.apache.myfaces.spi. >>>> >>>> I did not change anything here, just internal imports (from *.spi.impl >>>> to *.spi.util)! >>> >>> It is questionable to create .spi.util. After all, it is not supposed >>> that package be consumed by container integration code, so it should >>> be on spi.impl. >>> >>>> >>>>> 2. All classes that has to be with LifecycleProvider (@PostConstruct >>>>> annotation). Such classes should be on spi package, but note spi work >>>>> started after 2.0 release, so this should wait to a major version. >>>> >>>> Geronimo (for which we did the SPI stuff) includes MyFaces 2.0.x. Here >>>> we are talking about 2.1.x. Furthermore, one call to organize imports >>>> does the trick, so I do not see a problem here. >>> >>> Look the page of JSR-314 spec http://www.jcp.org/en/jsr/summary?id=314 >>> >>> You will notice 2.1.x is not a new JSR like the future JSR-344 (JSF >>> 2.2), and instead is tagged as "Maintenance Release 2". So, to be >>> consistent, it should be possible to change between 2.0 and 2.1 on the >>> same server. That's the most important reason to be so conservative or >>> strict with 2.1. >>> >>>> >>>>> 3. All classes on org.apache.myfaces.config.element. These classes are >>>>> an interface to manipulate faces-config.xml files with java code, and >>>>> spi interfaces provide the hooks to get them, so in theory we can add >>>>> methods there, but relocate this package to >>>>> org.apache.myfaces.spi.data is not necessary. I think the package name >>>>> is correct. >>>> >>>> OK, fine. I thought the relocation to org.apache.myfaces.spi.* would >>>> fit better, since it is the myfaces-spi module and, again, since we're >>>> talking about 2.1.x, not 2.0.x here. But if you want to keep the >>>> package-name, I have no problem with that. >>>> org.apache.myfaces.config.element is fine too, however, you may not >>>> expect it to be in the myfaces-spi module. >>>> >>>>> [...] Considering this, I think the costs >>>>> involved on the changes proposed are too big compared with the >>>>> benefits, which are very limited and almost inexistent from user point >>>>> of view. >>>> >>>> From a user point of view: maybe yes. But from a developer point of >>>> view myfaces-shaded-impl is an epic fail. I know it was an easy an >>>> fast solution at the time we got 2.1.0 out, but for the long term this >>>> has to be changed. Please think about it, you use 2.0.5 (or any other >>>> _previous_ version) in myfaces-impl-ee6 as if it was 2.1.x-SNAPSHOT. >>>> Thus you use internals of previous versions which might not even be >>>> there anymore in the current 2.1.x-SNAPSHOT. And the worst part of it: >>>> you don't even see it at build time, b/c it's a separate module and >>>> myfaces-impl-ee6 is shaded into myfaces-impl (and shade does not warn >>>> about this kind of stuff, of course). >>>> >>> >>> I know the hack done to compile 2.1 is not very clean, but it works. >>> The idea is replace 2.0.5 ref with 2.1.0 in future versions. Note >>> myfaces-shaded-impl is a module that is just for allow compile >>> myfaces-impl-ee6, and nobody else. It is not "an epic fail", because >>> it does not cause any changes on the code that cause problems. >>> >>>> Considering this, it was ok to create shaded-impl in order to get the >>>> 2.1.0 release out, but for future releases (maybe also towards 2.2.0), >>>> this must be done in another way. >>> >>> In fact, the idea is do something in 2.2.x, but that will take some >>> time, so maybe we can keep in mind those changes until get there. >>> >>>> I have to say that I won't support a >>>> 2.1.2 release including the shaded-impl module. >>> >>> I hope my arguments could be enough to convince you about the opposite. >>> >>> regards , >>> >>> Leonardo Uribe >>> >>>> >>>> Regards, >>>> Jakob >>>> >>>> 2011/7/10 Leonardo Uribe <lu4...@gmail.com>: >>>>> Hi Gerhard >>>>> >>>>> Ok, in theory the parts that we should not change, or otherwise that >>>>> will trigger a change on JEE containers are: >>>>> >>>>> 1. All classes in org.apache.myfaces.spi. >>>>> 2. All classes that has to be with LifecycleProvider (@PostConstruct >>>>> annotation). Such classes should be on spi package, but note spi work >>>>> started after 2.0 release, so this should wait to a major version. >>>>> 3. All classes on org.apache.myfaces.config.element. These classes are >>>>> an interface to manipulate faces-config.xml files with java code, and >>>>> spi interfaces provide the hooks to get them, so in theory we can add >>>>> methods there, but relocate this package to >>>>> org.apache.myfaces.spi.data is not necessary. I think the package name >>>>> is correct. >>>>> >>>>> regards, >>>>> >>>>> Leonardo Uribe >>>>> >>>>> 2011/7/9 Gerhard Petracek <gerhard.petra...@gmail.com>: >>>>>> hi leo, >>>>>> thx for checking it. >>>>>> it would be great, if you can post a list of parts (not classes) which >>>>>> would >>>>>> break. so we can discuss this topic in a more concrete manner. >>>>>> regards, >>>>>> gerhard >>>>>> http://www.irian.at >>>>>> >>>>>> Your JSF powerhouse - >>>>>> JSF Consulting, Development and >>>>>> Courses in English and German >>>>>> >>>>>> Professional Support for Apache MyFaces >>>>>> >>>>>> >>>>>> 2011/7/10 Leonardo Uribe <lu4...@gmail.com> >>>>>>> >>>>>>> Hi >>>>>>> >>>>>>> Good to know that. Unfortunately, do a change on the spi related >>>>>>> classes will break existing integration code with containers like >>>>>>> Geronimo, JBoss and others too. Considering this, I think the costs >>>>>>> involved on the changes proposed are too big compared with the >>>>>>> benefits, which are very limited and almost inexistent from user point >>>>>>> of view. >>>>>>> >>>>>>> regards, >>>>>>> >>>>>>> Leonardo Uribe >>>>>>> >>>>>>> 2011/7/8 Werner Punz <werner.p...@gmail.com>: >>>>>>> > I have no problem if you break Ext-Script, after all I program >>>>>>> > against >>>>>>> > the >>>>>>> > impl, so whatever change you do I have to patch it in my codebase >>>>>>> > anyway. >>>>>>> > Don´t feel stopped by it. >>>>>>> > >>>>>>> > Werner >>>>>>> > >>>>>>> > >>>>>>> > Am 08.07.11 18:27, schrieb Gerhard Petracek: >>>>>>> >> >>>>>>> >> hi, >>>>>>> >> >>>>>>> >> thx for reviewing the changes. >>>>>>> >> for sure we can discuss when we are going to do such changes. >>>>>>> >> however, >>>>>>> >> if ext-script is the only reason against it, we can do it right now >>>>>>> >> imo. >>>>>>> >> there is no problem with shipping a small update of ext-script and >>>>>>> >> the >>>>>>> >> user base is currently small enough to communicate this change >>>>>>> >> easily. >>>>>>> >> >>>>>>> >> regards, >>>>>>> >> gerhard >>>>>>> >> >>>>>>> >> http://www.irian.at >>>>>>> >> >>>>>>> >> Your JSF powerhouse - >>>>>>> >> JSF Consulting, Development and >>>>>>> >> Courses in English and German >>>>>>> >> >>>>>>> >> Professional Support for Apache MyFaces >>>>>>> >> >>>>>>> >> >>>>>>> >> >>>>>>> >> 2011/7/8 Leonardo Uribe <lu4...@gmail.com <mailto:lu4...@gmail.com>> >>>>>>> >> >>>>>>> >> Hi >>>>>>> >> >>>>>>> >> In principle I agree with this change, but after a quick patch >>>>>>> >> review >>>>>>> >> I saw some package relocation for some classes (from >>>>>>> >> org.apache.myfaces.config.element to org.apache.myfaces.spi.data). >>>>>>> >> Those changes will break extensions scripting code. Such changes >>>>>>> >> should be done between major versions (2.2.0). Please do not >>>>>>> >> change >>>>>>> >> the package names. >>>>>>> >> >>>>>>> >> regards, >>>>>>> >> >>>>>>> >> Leonardo Uribe >>>>>>> >> >>>>>>> >> 2011/7/8 Gerhard Petracek <gerhard.petra...@gmail.com >>>>>>> >> <mailto:gerhard.petra...@gmail.com>>: >>>>>>> >> > +1 >>>>>>> >> > regards, >>>>>>> >> > gerhard >>>>>>> >> > >>>>>>> >> > http://www.irian.at >>>>>>> >> > >>>>>>> >> > Your JSF powerhouse - >>>>>>> >> > JSF Consulting, Development and >>>>>>> >> > Courses in English and German >>>>>>> >> > >>>>>>> >> > Professional Support for Apache MyFaces >>>>>>> >> > >>>>>>> >> > >>>>>>> >> > >>>>>>> >> > 2011/7/8 Jakob Korherr <jakob.korh...@gmail.com >>>>>>> >> <mailto:jakob.korh...@gmail.com>> >>>>>>> >> >> >>>>>>> >> >> Hi guys, >>>>>>> >> >> >>>>>>> >> >> We currently use the shaded-impl module in core 2.1.x to >>>>>>> >> solve a >>>>>>> >> >> cyclic dependency problem. However, this introduces redundancy >>>>>>> >> and >>>>>>> >> >> complexity and may lead to some strange issues. This can be >>>>>>> >> solved by >>>>>>> >> >> the introduction of a myfaces-spi module (which is then packed >>>>>>> >> >> together with myfaces-impl, just like myfaces-impl-ee6 is). >>>>>>> >> >> >>>>>>> >> >> Please see MYFACES-3211 [1] for a detailed description of the >>>>>>> >> solution >>>>>>> >> >> and a patch for it. >>>>>>> >> >> >>>>>>> >> >> If there are no objections, I will commit this patch soon! >>>>>>> >> >> >>>>>>> >> >> Regards, >>>>>>> >> >> Jakob >>>>>>> >> >> >>>>>>> >> >> [1] https://issues.apache.org/jira/browse/MYFACES-3211 >>>>>>> >> >> >>>>>>> >> >> -- >>>>>>> >> >> Jakob Korherr >>>>>>> >> >> >>>>>>> >> >> blog: http://www.jakobk.com >>>>>>> >> >> twitter: http://twitter.com/jakobkorherr >>>>>>> >> >> work: http://www.irian.at >>>>>>> >> > >>>>>>> >> > >>>>>>> >> >>>>>>> >> >>>>>>> > >>>>>>> > >>>>>>> > >>>>>> >>>>>> >>>>> >>>> >>>> >>>> >>>> -- >>>> Jakob Korherr >>>> >>>> blog: http://www.jakobk.com >>>> twitter: http://twitter.com/jakobkorherr >>>> work: http://www.irian.at >>>> >>> >> >> >> >> -- >> Jakob Korherr >> >> blog: http://www.jakobk.com >> twitter: http://twitter.com/jakobkorherr >> work: http://www.irian.at >> > -- Jakob Korherr blog: http://www.jakobk.com twitter: http://twitter.com/jakobkorherr work: http://www.irian.at
