Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using the shell to locate wkhtmltopdf in a Bundler environment #728

Merged

Conversation

glossawy
Copy link

@glossawy glossawy commented Mar 16, 2018

Currently, wicked_pdf suffers from the same issue the pdfkit does (according to pdfkit/pdfkit#388) where it picks up warning information introduced in bundle 1.4.x when the HOME environment variable points to a non-writable directory. When the information is not specified by configuration and the wicked_pdf is being used in a bundler environment, wicked_pdf attempts to use bundle exec which wkhtmltopdf which will output something like:

`/not/a/writable/directory` is not a directory.
 Bundler will use `/tmp/bundler/home/travis' as your home directory temporarily.
 /usr/bin/wkhtmltopdf

Which will then trigger a failure in [WickedPdf#initialize] caused by getting the full text above from the call to the shell in WickedPdf#find_wkhtmltopdf_binary_path.

Calling out to the shell itself seems odd when calling the shell through bundler. Instead, this change uses the long-present Bundler.which method which does the same work, but will give back a consistent result.

Note on Workaround

Similar to the pdfkit issue, this can be worked around by explicitly setting exe_path when configuring wicked_pdf, per the readme.

Matthew Crocco added 2 commits March 16, 2018 19:05
Using the shell to get this information leads to the risk of capturing
warning information from bundler. Using the which method (which has been
around for years) will do the same thing, but will only return the path
and nothing else.
@glossawy glossawy force-pushed the find-binary-using-which-interface branch from 8b48385 to 7768be7 Compare March 16, 2018 23:05
@glossawy glossawy changed the title Add tests around erroneous behavior from using the shell Avoid using the shell to locate wkhtmltopdf in a Bundler environment Mar 16, 2018
@unixmonkey unixmonkey merged commit f06f00e into mileszs:master Jul 9, 2018
@unixmonkey
Copy link
Collaborator

Great! Thanks for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants