2017-04-19 10:05 GMT+02:00 Luca Toscano <toscano.l...@gmail.com>: > > > 2017-03-27 19:09 GMT+02:00 Luca Toscano <toscano.l...@gmail.com>: > >> Hi Yann, >> >> 2017-03-27 8:56 GMT+02:00 Yann Ylavic <ylavic....@gmail.com>: >> >>> Hi Luca, >>> >>> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <toscano.l...@gmail.com> >>> wrote: >>> > >>> > Documentation updated with the current status, plus the following patch >>> > seems to allow nested if blocks (probably not the best one but it is a >>> pof): >>> > >>> > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch >>> >>> LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a >>> local helper only). >>> >> >> Thanks a lot for the feedback, I modified the patch to include your >> suggestion and another one from Jim (checking whether or not the >> ap_if_walk_sub calls return something different than OK, hope that the >> implementation is what was expected). Ran also the test suite in trunk, no >> failure registered (Jacob: make check is really awesome). >> >> I'd like to perform some performance tests before proceeding, this code >> adds a (probably negligible in 90% of the use case) overhead to each >> ap_if_walk call (running twice for each request afaics). >> >> Any other comment/review will be really appreciated :) >> >> > Updates: > > - the original patch evolved up to http://home.apache.org/~elukey > /httpd-trunk-core-nested_if_blocks.patch but it was a bit too hacky. > > - Jacob reviewed the code and suggested to investigate the use of > now_merged to clean up the code, and a new patch came to life: > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks_v2.patch > > - Added some tests to the suite in http://home.apache.org/~elukey > /httpd-framework-nested-ifs.patch, everything seems to work fine. My > understanding is that the tests are leveraging the "Header merge" feature > to simulate the order of merging with different <If> condition evaluations. > I added the same block of nested <If>/ElseIf/Else to > Directory/Location/Files and the result looks consistent. > > Comments welcome, I think the work is almost done but I might have missed > something.. >
For the records the trunk patch has been committed in r1792589 and the related tests in r1793187. Luca
