Review Request 125399: Enable loading alternative plugins and add executable check

[Elvis Angelaccio]

> On Sept. 26, 2015, 2:39 p.m., Elvis Angelaccio wrote:
> > kerfuffle/cliinterface.cpp, line 89
> > <https://git.reviewboard.kde.org/r/125399/diff/1/?file=408397#file408397line89>
> >
> >     This function can be slightly optimized/simplified. You are checking at the end whether all the programs are found. Instead, if something is not found, there is no point in searching the remaining executables; you can just return false and you're done.
> 
> Ragnar Thomsen wrote:
>     I am uncertain how this should be handled. At the time of loading the plugin, we don't know if the user intends to only list/extract the archive or modify it. That's why I initially required all executable types to be found. However, the list/extract and add/delete programs are often found in different packages (zip/unzip, rar/unrar), so there is a possibility that e.g. unzip is installed while zip is not. If we refuse to load the plugin due to a missing zip executable, the user will not be able to use this plugin at all. Any ideas how this should be solved?

Probably with yet another boolean property in `ReadOnlyArchiveInterface`, which tells Ark if the archive is read-only "for this session only", due to a missing zip/rar executable. Btw, this is stuff for another patch. My suggestion above can still be applied without changing the original logic of the function.


- Elvis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125399/#review85971
-----------------------------------------------------------


On Sept. 26, 2015, 3:40 p.m., Ragnar Thomsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125399/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2015, 3:40 p.m.)
> 
> 
> Review request for KDE Utils, Elvis Angelaccio and Raphael Kubo da Costa.
> 
> 
> Repository: ark
> 
> 
> Description
> -------
> 
> Plugin loading code is reworked so Ark attempts other plugins, if first priority plugin fails. For CliPlugins, Ark now also searches for executables (as specified in ParameterList in the individual plugins) in standard path after the plugin has been loaded and if they are not found moves on to next plugin in list. It requires all four type of executables to be found (ListProgram, ExtractProgram, AddProgram and DeleteProgram). However, if the plugin is readonly (as specified by the X-KDE-Kerfuffle-ReadWrite property in the desktop file), it only requires ListProgram and ExtractProgram.
> 
> This paves the way for setting cli7z priority higher than clizip to improve unicode support. If p7zip is not installed by distributions, Ark will now automatically fallback to using the more widely-installed clizip.
> 
> 
> Diffs
> -----
> 
>   kerfuffle/archiveinterface.h 1aac2a4 
>   kerfuffle/archiveinterface.cpp d8a6725 
>   kerfuffle/cliinterface.h e4bc6f3 
>   kerfuffle/cliinterface.cpp b537f95 
>   kerfuffle/archive_kerfuffle.cpp f17155c 
> 
> Diff: https://git.reviewboard.kde.org/r/125399/diff/
> 
> 
> Testing
> -------
> 
> 1. Tried moving one of the infozip executables out of path -> Ark doesn't error out, but uses cli7z plugin instead. 
> 2. Tried changing X-KDE-Kerfuffle-ReadWrite to false for some plugin -> Ark only searches for ListProgram and ExtractProgram.
> 
> 
> Thanks,
> 
> Ragnar Thomsen
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20150927/51ceac19/attachment.html>