wkhtmltopdf is not necessarily a file

[ylecuyer]

WKHTMLTOPDF is not necessarily a file:

WickedPdf.config = {
   :wkhtmltopdf => 'xvfb-run wkhtmltopdf'
}

[unixmonkey]

Won't this executable check also raise in your case?

File.executable?('/bin/ls') # => true 
File.executable?('/bin/ls -a') => false

[ylecuyer]

I tested and you're right, I will change the PR

[unixmonkey]

What do you think about putting the original raise statements back, but only checking if xvfb isn't in the EXE_NAME?

Something like this:

unless EXE_PATH.include?('xvfb')
  raise "Bad #{EXE_NAME}'s path: #{@exe_path}" unless File.exist?(@exe_path)
  raise "#{EXE_NAME} is not executable" unless File.executable?(@exe_path)
end

I worry that removing these raise statements will make it harder for people to figure out what's wrong when first configuring wicked_pdf.

[zorbash]

What about introducing a :wkhtmltopdf_command like:

WickedPdf.config = {
   :wkhtmltopdf_command => 'xvfb-run -- wkhtmltopdf'
}

Which would have no validations?

---

I also think that the current :wkhtmltopdf option should be renamed to :wkhtmltopdf_path, so that it's clear that the validations are performed against an executable file.

This issue probably affects lots of users, since the official Ruby Docker image is based on Debian jessie and the Debian wkhtmltopdf package clearly states that for environments which don't have a running X server, a virtual X11 server (like Xvfb) can be used:

Example:

xvfb-run -- wkhtmltopdf [options]

see: https://sources.debian.net/src/wkhtmltopdf/0.12.3.2-3/debian/README.Debian/

[unixmonkey]

@zorbash That sounds like a good compromise, though I am hesitant to rename the existing config option until it gets properly deprecated.

There's also some confusion around the wkhtmltopdf and exe_path config options, since they essentially do the same thing, maybe some cleanup here would be good.