Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

revert to 6e2d550 (wkhtmltopdf 0.9.9) #12171

Closed
wants to merge 1 commit into from
Closed

revert to 6e2d550 (wkhtmltopdf 0.9.9) #12171

wants to merge 1 commit into from

Conversation

kerrizor
Copy link

@kerrizor kerrizor commented May 9, 2012

0.11.0_rc1 is a release candidate and has outstanding bugs that are rendering it non-functional for many Mac users.

…idate and has outstanding bugs that are rendering it non-functional for many Mac users.
@2bits
Copy link
Contributor

2bits commented May 10, 2012

cf #9265 and #11400 Please explain how reverting fixes 9265. Are you aware of this command?

$ brew versions wkhtmltopdf
0.11.0_rc1 git checkout aa80fbc /usr/local/Library/Formula/wkhtmltopdf.rb
0.9.9    git checkout 6e2d550 /usr/local/Library/Formula/wkhtmltopdf.rb

@kerrizor
Copy link
Author

Good morning!

Yes, I'm quite familiar with the "versions" command, thanks :)

Obviously this doesn't address #9265. You've done a lot of great work in your code to fix those issues, and its great to see continued development and effort, and this PR isn't meant to devalue that, or toss it all aside.

The issue for me is that 0.11.0_rc1 is, after all, just a release candidate. Yes, its working with simple HTML documents, but myself and others are seeing massive performance problems with it when using it for more complex documents; across 6 computers (Snow Leopard and Lion) I'm seeing render times ranging from 4 minutes to "it never finishes", random crashes, runaway processes.. fixing #9265 addresses one user's issue, but is making a buggy release candidate the default homebrew install the right path for everyone?

You obviously have more experience working with the wkhtmltopdf tool - do you think there's a solution that satisfies the one user in #9265 and provides a stable, working tool for the rest of us? How can I help make that happen?

@2bits
Copy link
Contributor

2bits commented May 10, 2012

fixing 9265 addresses one user's issue, but is making a buggy release candidate the default homebrew install the right path for everyone?

That's a good question. In general the answer is use the latest version and get the issues fixed upstream. In the case of this software, though, it's not likely to happen. So the easiest thing would be to rework the formula so that it contains both stable and devel versions, 0.9.9 and 0.11.0_rc1. That should elicit a joyful noise :-)

You can do that here in this pull request if you'd like, by amending your commit and doing a force push. It will be a lot of work because things changed between those two versions, but it's just logic. So if you have the time, go for it. Just be sure to include the qmake -spec macx-g++ code to fix 9265. That's all it needed. If you're too busy, I'd be happy to author the changes. That's no problem.

To you and any other people who got burned by this update, I'm sorry for taking up your time and increasing your workload. My experience with this software is limited to authoring the formula and testing it on two html pages. So like I said, I'm happy to fix what I broke.

@2bits
Copy link
Contributor

2bits commented May 16, 2012

@kerrizor Could you link a couple of web pages that convert slowly and the wkhtmltopdf command line you use? Thanks.

@kerrizor
Copy link
Author

@2bits - Sorry, I've been swamped. I'll dig up some URLs and post back today.

@shazron
Copy link
Contributor

shazron commented May 24, 2012

Hi,
I can confirm 0.11 is abnormally slow (the process doesn't seem to end), when I revert to 0.99 it is very speedy. I maintain this repo below, and we have several Markdown files that we convert to HTML then run wkhtmltopdf on (markdown downloaded using brew as well).

The repo: https://github.com/apache/incubator-cordova-ios
The markdown files can be found in the root, and the "guides" subfolder - all with the suffix of .md

You can run the Makefile to see it in action.

@shazron
Copy link
Contributor

shazron commented May 24, 2012

(sorry the Github repo is a mirror of the Apache repo - my latest changes might not be reflected yet). The main repo is at:
http://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=summary

@2bits
Copy link
Contributor

2bits commented May 24, 2012

You're saying when that README.md is in html it can't convert it?

@shazron
Copy link
Contributor

shazron commented May 24, 2012

Possibly the README or the other 5 markdown files in the guides subfolder, I haven't narrowed it down (it just hangs the whole make flow), I'll narrow down one troublesome file for you tomorrow when I can get back to this.

@2bits
Copy link
Contributor

2bits commented May 24, 2012

I posted a formula in this gist https://gist.github.com/2724019 that will build 0.9.9 or 0.11.0_rc1 as the devel version. It will also build their patched Qt into the devel version only, if you use the option, --devel --build-patched-qt. By viewing those md files on GitHub, they are converted to html, and I am now testing them by converting the GitHub web pages to pdf.

@2bits
Copy link
Contributor

2bits commented May 24, 2012

I saved each of those .md files to my computer with filenames that contain no spaces. I used markdown C1.md > C1.html and then time -p wkhtmltopdf C1.html C1.pdf. After testing 4 of those files, I stopped because they all took less than a second to convert. The README.md spit out this error when running markdown on it:

Complex regular subexpression recursion limit (32766) exceeded at /usr/local/bin/markdown line 396, <> line 1.

But it still worked. So I'm not sure why you have problem, but I think it's fast, wkhtmltopdf-0.11.0_rc1 build-patched-qt.

@shazron
Copy link
Contributor

shazron commented May 24, 2012

Thanks @2bits, I'll try to repro again tomorrow, and report the results.

@shazron
Copy link
Contributor

shazron commented May 24, 2012

Did some re-tests. I have no doubt that your version with the devel patched qt works (from your gist code I couldn't get it to compile, but that's another story), but I'm thinking of the "default experience" of users downloading wkhtmltopdf.

From the version downloaded from homebrew, which will be 0.11.0_rc, it hangs after "Done" is output to stdout. I quit the process after 2 mins, when using 0.9.9, it only took 3s for it to convert the README.md/.html I linked to. I'll have to make my Makefile default to downloading 0.9.9 for now until I figure this out.

@2bits
Copy link
Contributor

2bits commented May 24, 2012

Why couldn't you get https://gist.github.com/2724019 to work? Can you gist your error please?
So you are saying you built 0.11.0 with Homebrew and Homebrew's Qt4. Did you convert README.html from a file on your local hard drive or was it a remote url? If it was a url, can you give me a link to try it.
The gist I made, https://gist.github.com/2724019, defaults to building 0.9.9.

@adamv
Copy link
Contributor

adamv commented Aug 29, 2012

What do we do here?

There's an rc2 but only as a git sha1: wkhtmltopdf/wkhtmltopdf@1a24c17

@adamv
Copy link
Contributor

adamv commented Aug 29, 2012

Tentatively marking for removal.

@2bits
Copy link
Contributor

2bits commented Aug 29, 2012

Let's keep this. It is still in development and the tarballs are on googlecode. In early June he started two new mailing lists that he responds to which include a discussion of his recent work on qt-4.8.2 where he recommends a new Qt branch than the one I have in the subformula https://gist.github.com/2724019

Is there something else wrong?

@adamv
Copy link
Contributor

adamv commented Aug 29, 2012

Passing on reverting this to an earlier version; will accept pull requests to move to newer rcs.

@adamv adamv closed this Aug 29, 2012
@2bits
Copy link
Contributor

2bits commented Aug 31, 2012

This is the new branch. It needs our workarounds we applied to 4.8.2, but it should work then.
https://qt.gitorious.org/~antialize/qt/antializes-qt/commits/4.8.2

@mhfs
Copy link

mhfs commented Dec 10, 2012

Reverting seems like a good idea. Installed current formula and ran into this: http://code.google.com/p/wkhtmltopdf/issues/detail?id=141

Installing 0.9.9 via previous formula worked great.

@Homebrew Homebrew locked and limited conversation to collaborators Feb 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants