[6.0] report_webkit can't find the wkhtmltopdf binary automatically

This patch allow report_webkit to automatically look for the wkhtmltopdf binary in the PATH environment variable, respecting it's ordre . It still keep the old manual configuration method, which take precedence when it's set.

Olivier,

It is believed that you should not search in the whole system for the bin file of wkhtmltopdf.

Reasons:
1. I could have downloaded it and I am using it placed somewhere else.
2. We cannot waste system resources searching for any third party lib. It seems it will happen each time report is printed!
3. I could rename too.
4. Its a third party library. Logically, similar to addons-path. So better to configure rather than code.
5. It might be possible that there is no enough access for the user to access the PATH you wrote.

Hope this justifies, but final decision can still be of C2C.

Regards,
Serpent Consulting Services.

Hi,

Thanks for your answer.

> 1. I could have downloaded it and I am using it placed somewhere else.
This patch still allow to manually set the path for wkhtmltopdf.

> 2. We cannot waste system resources searching for any third party lib. It seems it will happen each time report is printed!
This patch will only search the system if no path is defined but even then this is a extremly fast operation and it's done each time you launch a program on *nix.

> 3. I could rename too.

Current code doesn't allow you to have an executable by any other name than 'wkhtmltopdf' anyway

> 4. Its a third party library. Logically, similar to addons-path. So better to configure rather than code.

It's still configuration but done via the PATH environment settings, which is generally the preferred way under *nix. It also allow to configure it once for the whole server than each time you create a new database.

> 5. It might be possible that there is no enough access for the user to access the PATH you wrote.

The PATH is configured in the system, if an user doesn't have access right for one of the directory in your system path it probably mean that there is an error in your system configuration.

I hope I have answered all yours concerns. I don't know the rules for the C2C so I hope I don't break any by re-puting this patch up for consideration.

Olivier,

Thanks for your answers.

Your points are quite understandable, but we can only discuss as being a part of community.

Nicolas, would you please take a look?

Thanks.

Hello,

Well this point is all a story.

Summary of the long debate in short:

-official addons can not embed compiled binary it may make anti viruses angry
-some people want to use a different version of binary than the one in the system for example the QT patched one
-walking path can be costly etc.

So in 6.1 we decided to put it as a server ir_config_parameter. If this data is available it is used else it will look in the path.

"""
    def get_lib(self, cursor, uid):
        """Return the lib wkhtml path"""
        proxy = self.pool.get('ir.config_parameter')
        webkit_path = proxy.get_param(cursor, uid, 'webkit_path')

        if not webkit_path:
            try:
                defpath = os.environ.get('PATH', os.defpath).split(os.pathsep)
                if hasattr(sys, 'frozen'):
                    defpath.append(os.getcwd())
                    if tools.config['root_path']:
                        defpath.append(os.path.dirname(tools.config['root_path']))
                webkit_path = tools.which('wkhtmltopdf', path=os.pathsep.join(defpath))
            except IOError:
                webkit_path = None

        if webkit_path:
            return webkit_path
"""

Your patch is follows the flow that has been taken in 6.1. But I'm not sure it fits 6.0 commit policy.

To offer an alternate way we have coded the webkit_lib addons that embed binary and use the good one depending on OS and architecture (32, 64).

Regards

Nicolas

Hey,

 thanks both for your answers, I understand your rationales now.