Review Request 111954: proper use filter_<app> products flag to enable/disable whole app filters

Friedrich W. H. Kossebau kossebau at kde.org
Fri Aug 9 09:20:16 BST 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111954/#review37396
-----------------------------------------------------------


Hi Sebastian,

In the design of these product definition macros currently all the rules are whitelisting rules, not blacklisting rules.
The FILTER_<appname>  products are kind of meta-products (or rather productsets) which are used to spare people to have to list each and every filter in their custom productset (and also the premade ones). I am unhappy with that they are also called products, plan to turn them into rather "productsets" as well, because they have a different purpose/semantic. Sets will be listed in sets then as well.
Support for BUILD_xxx=FALSE in the cmake call was rather meant as minimal legacy support for some existing scripts out there, which had xxx as some appname. But it never was done to properly support (sub-)products, like the filters (or filter groups).

Given the recent feedback I plan to add this finally (wanted to do last WE already, so hopefully will do at the upcoming).

So if you do not want to have filters build, create your own productset file in which they are not listed (or for the porting turn all related lines into comments in the predefined productssets, though that will have no effect on private ones).
If filters are broken in general, then with the current system you might need to add them one-by-one in the disable-because-broken area (yes, porting with mass-breaking states was not considered, bah :) ).

But the proposed patch would not match the current logic, so I have to reject it. Will see to solve this in the system design ASAP.

- Friedrich W. H. Kossebau


On Aug. 8, 2013, 8:26 p.m., Sebastian Sauer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111954/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2013, 8:26 p.m.)
> 
> 
> Review request for Calligra and Friedrich W. H. Kossebau.
> 
> 
> Description
> -------
> 
> Those new product flags define for each top-level application a matching FILTER_<appname> flag. That flag allows to disable all filters for a certain app(s) but it (seems?) they are not evaluated and applied. Following patch works for me in that if I now add something like
> 
> calligra_disable_product(FILTER_WORDS "Qt5 TODO")
> calligra_disable_product(FILTER_SHEETS "Qt5 TODO")
> calligra_disable_product(FILTER_STAGE "Qt5 TODO")
> 
> to the top-level CMakeLists.txt then all filters for Words, Sheets and Stage are disabled means not build.
> 
> p.s. an alternate way to solve that could be to e.g.
> in calligra/CMakeLists.txt
> - calligra_define_product(FILTER_APPLIXSPREAD_TO_KSPREAD "Applix Spreadsheet to KSpread filter" NEEDS SHEETS_PART)
> + calligra_define_product(FILTER_APPLIXSPREAD_TO_KSPREAD "Applix Spreadsheet to KSpread filter" NEEDS SHEETS_PART FILTER_SHEETS)
> 
> What's the prefered way?
> 
> 
> Diffs
> -----
> 
>   filters/CMakeLists.txt 79a42a2 
> 
> Diff: http://git.reviewboard.kde.org/r/111954/diff/
> 
> 
> Testing
> -------
> 
> Note that testing was done with the Qt5-branch. I ask for review (and merge to master) so I get feedback I didn't got something wrong here how those product-flags are supposed to work. Since there are way more such switches I may have to add more such cases here and there and so it would make sense to at least have the initial patch reviewed asap before investing time into solving a not-existing problem and/or a solving it in a wrong way :)
> 
> 
> Thanks,
> 
> Sebastian Sauer
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130809/b88fb65f/attachment.htm>


More information about the calligra-devel mailing list