State of Velocity 2.0 upgrade

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

State of Velocity 2.0 upgrade

Thomas Mortagne
Administrator
Hi devs,

Some of you may know that I started working on upgrading our scripting
engine to Velocity 2.0.

The corresponding Jira issue is https://jira.xwiki.org/browse/XCOMMONS-1296.

Those of you who want to take a look at breakages with standard
Velocity 2.0 upgrade (if you directly use Velocity API) can take a
look at http://velocity.apache.org/engine/2.0/upgrading.html.

The following details are about XWiki Velocity "engine":

= The code

You can see the current state of the upgrade on the following branches:

* https://github.com/xwiki/xwiki-commons/tree/feature-velocity2
* https://github.com/xwiki/xwiki-rendering/tree/feature-velocity2
* https://github.com/xwiki/xwiki-platform/tree/feature-velocity2

= Bad news

== Definitive (probably) breakages

* it was not easy in Velocity 1.7 but it's now impossible in Velocity
2.0 to use the hack we currently use to make macro "return" something.
I mitigated this a bit by working on a #setVariable directive (the
name of the helper we currently have in macros.vm) but it's not yet
working in all situations and any code that was not going trough
#setVariable will be broken anyway
* the hypen ( - ) cannot be used in variable names anymore

Those changes make impossible to upgrade to Velocity 2.0 in 10.x
branch IMO, to big to be done in the middle of a cycle.

== Temporary (hopefully) breakages

* impossible to have $ or # at the end of a " based string literal (we
have that a lot in various regex for example). This looks like an
unplanned regression (since it's not documented) so I'm waiting for
some kind reaction in the issues I reported
(https://issues.apache.org/jira/browse/VELOCITY-897 and
https://issues.apache.org/jira/browse/VELOCITY-896)

== Not too impacting (hopefully) breakages

The Velocity API changed quite a lot, fortunately not in classes we
expose publicly (basically all we expose is VelocityContext). Also
reminder: directly manipulating even XWiki Velocity API is considered
legacy things (better use templates or wiki content and
ScriptContext).

= Good news

== Macros handling

The way to manage namespaces and macros has been completely changed
(it does mean that lots of API has been broken in the process but we
don't expose this API) and the good news is the new design will allow
us to (finally) do proper caching of velocity scripts.

I also removed a few hacks we had to do to not end up with endless
memory leaks and milti-thread issues caused by the old namespace
handling system.

== Less stuff on our side

I was able to move the following to legacy:
* an equivalent of our chainable uberspector system has been
implemented in Velocity standard
* a DeprecatedCheckUberspector has been implemented in Velocity standard

== Retro compatibility I was able to add

* XWiki will keep supporting $velocityCount and $velocityHasNext (with
a warning). It imply using XWikiVelocityContext instead of
VelocityContext if you create it directly (which is a very bad idea)
but you don't need to worry about that if you get your context from
VelocityManager
* a new #setVariable directive help supporting a few use case of macro
"return" but unfortunately not all yet (Velocity AST is not super
obvious to navigate...)

== Performances

Other than the caching on our side I already mentioned (huge boost
planned when implemented) there is some memory and performance
improvements claims in Velocity 2.0 release note (but I did not had
time to do any testing myself yet).

= TODO

a) waiting for answer about the regressions I reported
b) try find a way to make #setVariable directive works in a macro
which is itself called in another macro (but it may not be possible)
c) send a mail to discuss when to merge the Velocity 2 branch when at
least a) is resolved

Thanks,
--
Thomas Mortagne
Reply | Threaded
Open this post in threaded view
|

Re: State of Velocity 2.0 upgrade

Thomas Mortagne
Administrator
So good news: Claude Brisson fixed all the blockers and they will be
part of Velocity 2.1. As a bonus we even got configuration flags to
allow back dash in variables names and to put back old behavior
regarding variable name when passed as parameter so I deleted my
crappy #setVariable directive.

I think we should still wait for 11.0 before doing the upgrade but it
will be much smoother than I previous anticipated.
On Fri, Jun 15, 2018 at 11:19 AM Thomas Mortagne
<[hidden email]> wrote:

>
> Hi devs,
>
> Some of you may know that I started working on upgrading our scripting
> engine to Velocity 2.0.
>
> The corresponding Jira issue is https://jira.xwiki.org/browse/XCOMMONS-1296.
>
> Those of you who want to take a look at breakages with standard
> Velocity 2.0 upgrade (if you directly use Velocity API) can take a
> look at http://velocity.apache.org/engine/2.0/upgrading.html.
>
> The following details are about XWiki Velocity "engine":
>
> = The code
>
> You can see the current state of the upgrade on the following branches:
>
> * https://github.com/xwiki/xwiki-commons/tree/feature-velocity2
> * https://github.com/xwiki/xwiki-rendering/tree/feature-velocity2
> * https://github.com/xwiki/xwiki-platform/tree/feature-velocity2
>
> = Bad news
>
> == Definitive (probably) breakages
>
> * it was not easy in Velocity 1.7 but it's now impossible in Velocity
> 2.0 to use the hack we currently use to make macro "return" something.
> I mitigated this a bit by working on a #setVariable directive (the
> name of the helper we currently have in macros.vm) but it's not yet
> working in all situations and any code that was not going trough
> #setVariable will be broken anyway
> * the hypen ( - ) cannot be used in variable names anymore
>
> Those changes make impossible to upgrade to Velocity 2.0 in 10.x
> branch IMO, to big to be done in the middle of a cycle.
>
> == Temporary (hopefully) breakages
>
> * impossible to have $ or # at the end of a " based string literal (we
> have that a lot in various regex for example). This looks like an
> unplanned regression (since it's not documented) so I'm waiting for
> some kind reaction in the issues I reported
> (https://issues.apache.org/jira/browse/VELOCITY-897 and
> https://issues.apache.org/jira/browse/VELOCITY-896)
>
> == Not too impacting (hopefully) breakages
>
> The Velocity API changed quite a lot, fortunately not in classes we
> expose publicly (basically all we expose is VelocityContext). Also
> reminder: directly manipulating even XWiki Velocity API is considered
> legacy things (better use templates or wiki content and
> ScriptContext).
>
> = Good news
>
> == Macros handling
>
> The way to manage namespaces and macros has been completely changed
> (it does mean that lots of API has been broken in the process but we
> don't expose this API) and the good news is the new design will allow
> us to (finally) do proper caching of velocity scripts.
>
> I also removed a few hacks we had to do to not end up with endless
> memory leaks and milti-thread issues caused by the old namespace
> handling system.
>
> == Less stuff on our side
>
> I was able to move the following to legacy:
> * an equivalent of our chainable uberspector system has been
> implemented in Velocity standard
> * a DeprecatedCheckUberspector has been implemented in Velocity standard
>
> == Retro compatibility I was able to add
>
> * XWiki will keep supporting $velocityCount and $velocityHasNext (with
> a warning). It imply using XWikiVelocityContext instead of
> VelocityContext if you create it directly (which is a very bad idea)
> but you don't need to worry about that if you get your context from
> VelocityManager
> * a new #setVariable directive help supporting a few use case of macro
> "return" but unfortunately not all yet (Velocity AST is not super
> obvious to navigate...)
>
> == Performances
>
> Other than the caching on our side I already mentioned (huge boost
> planned when implemented) there is some memory and performance
> improvements claims in Velocity 2.0 release note (but I did not had
> time to do any testing myself yet).
>
> = TODO
>
> a) waiting for answer about the regressions I reported
> b) try find a way to make #setVariable directive works in a macro
> which is itself called in another macro (but it may not be possible)
> c) send a mail to discuss when to merge the Velocity 2 branch when at
> least a) is resolved
>
> Thanks,
> --
> Thomas Mortagne



--
Thomas Mortagne