Throw an informative error if Qt is not present #591

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
@kgrz

kgrz commented Nov 14, 2013

Right now, running "gem install capybara-webkit" on a machine that
doesn't have Qt binaries installed fails with the
message "'qmake -spec macs-g++' not available". This change will check
if a binary is installed and throw a descriptive error.

Fixes #590

Throw an informative error if Qt is not present
Right now, running "gem install capybara-webkit" on a machine that
doesn't have Qt binaries installed, the installation will fail with the
message "'qmake -spec macs-g++' not available". This change will check
if a binary is installed and throw a descriptive error.
@jferris

This comment has been minimized.

Show comment Hide comment
@jferris

jferris Nov 15, 2013

Member

Thanks for the pull request. A couple of small things:

  • We've tried to keep the code in CapybaraWebkitBuilder. If you could move the check into the first step that looks for qmake, that would be awesome.
  • We look for several binaries like qmake-qt4 and allow users to override the binary with the QMAKE environment variable. This change would cause the build to crash on those configurations.

Do you want to make those changes?

Member

jferris commented Nov 15, 2013

Thanks for the pull request. A couple of small things:

  • We've tried to keep the code in CapybaraWebkitBuilder. If you could move the check into the first step that looks for qmake, that would be awesome.
  • We look for several binaries like qmake-qt4 and allow users to override the binary with the QMAKE environment variable. This change would cause the build to crash on those configurations.

Do you want to make those changes?

@kgrz

This comment has been minimized.

Show comment Hide comment
@kgrz

kgrz Nov 24, 2013

@jferris Thank you so much for your patience :) I made some changes so that the binary-availability checking is not limited to just qmake. I was thinking to extract the raise part into a separate error but the errors inside the lib directory doesn't seem to be the best place to add it and so, I wrote that inline.

kgrz commented Nov 24, 2013

@jferris Thank you so much for your patience :) I made some changes so that the binary-availability checking is not limited to just qmake. I was thinking to extract the raise part into a separate error but the errors inside the lib directory doesn't seem to be the best place to add it and so, I wrote that inline.

@jferris

This comment has been minimized.

Show comment Hide comment
@jferris

jferris Dec 20, 2013

Member

@kgrz sorry for the delay here. Looking over your most recent changes, it seems like this will now unconditionally throw an error whenever attempting to build?

Member

jferris commented Dec 20, 2013

@kgrz sorry for the delay here. Looking over your most recent changes, it seems like this will now unconditionally throw an error whenever attempting to build?

@kgrz

This comment has been minimized.

Show comment Hide comment
@kgrz

kgrz Dec 21, 2013

@jferris Apologies :/ I will close this PR and will open a new one with proper code. I don't know why I removed the if conditional at the end and pushed the code.

kgrz commented Dec 21, 2013

@jferris Apologies :/ I will close this PR and will open a new one with proper code. I don't know why I removed the if conditional at the end and pushed the code.

@kgrz kgrz closed this Dec 21, 2013

@kgrz kgrz deleted the kgrz:explicit_qt_error branch Dec 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment