-
-
Notifications
You must be signed in to change notification settings - Fork 898
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
Fix windows native build with RubyInstaller and DevKit #1310
Fix windows native build with RubyInstaller and DevKit #1310
Conversation
Thanks for submitting this. I need to dig in an understand what's going on here, and why the builds are failing. |
I'll check why ruby-1.9.3 on OSX failed and update the PR if necessary. |
@flavorjones Bummer! Travis-CI currently doesn't accept OSX requests and I don't have a Mac at hand. So I currently can only find out why ruby-1.9.3 on OSX fails, by temporary adding another commit to this PR, to debug this. |
3124400
to
b168653
Compare
This depends on a fixed version of mini_portile in regards to the handling of configure_options. I therefore assigned a git link in the Gemfile, temporary. Rubys stdlib shellwords is generelly an issue on Windows. It's sadly, but Windows uses several different ways to escape command line options. While shellwords mostly works in Msys bash (and therefore in a Makefile), it is completely wrong in cmd.exe or msvcrt based programs. It's annoying, but escape rules in mkmf.rb functions (like try_compile etc) are different to that of $CPPFLAGS etc. which run in a Makefile. I made an attempt to codify the different escape rules in: https://github.com/larskanis/shellwords/tree/master/lib/shellwords As a compromise I used String#inspect for the lists of patches. This works well enough on all platforms.
b168653
to
205dfa6
Compare
Some configure_options were still quoted. This is fixed now. |
$CPPFLAGS << ' ' << "-DNOKOGIRI_#{recipe.name.upcase}_PATH=\"#{recipe.path}\"".shellescape | ||
$CPPFLAGS << ' ' << "-DNOKOGIRI_#{recipe.name.upcase}_PATCHES=\"#{recipe.patch_files.map { |path| File.basename(path) }.join(' ')}\"".shellescape | ||
$CPPFLAGS << ' ' << "-DNOKOGIRI_#{recipe.name.upcase}_PATH=\"#{recipe.path}\"".inspect | ||
$CPPFLAGS << ' ' << "-DNOKOGIRI_#{recipe.name.upcase}_PATCHES=\"#{recipe.patch_files.map { |path| File.basename(path) }.join(' ')}\"".inspect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the .inspect
intentional? seems a little odd to me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is. This is what I refer in the PR description "As a compromise I used String#inspect for the lists of patches. This works well enough on all platforms."
What is status of this PR? |
Hi all, I just emailed nokogiri-talk about a Windows release. If you don't subscribe, you should, but here's the link in any case:
TL;DR we're going to get this out the door this week, honest. Thanks for your patience. |
Nice! Let me know if you need anything from RubyInstaller. |
@Azolo Good to know! For now, there's everything mighty fine with RubyInstaller in respect to nokogiri. |
Merging. Will be on master shortly. |
This depends on flavorjones/mini_portile#51 in regards to the handling of configure_options. I therefore assigned a git link in the Gemfile, temporary.
Rubys stdlib shellwords is generelly an issue on Windows. It's sadly, but Windows uses several different ways to escape command line options. While shellwords mostly works in Msys bash (and therefore in a Makefile), it is completely wrong in cmd.exe or msvcrt based programs. It's annoying, but escape rules in mkmf.rb functions (like try_compile etc) are different to that of $CPPFLAGS etc. which run in a Makefile. I made an attempt to codify the different escape rules in, but don't think this is necessary for nokogiri:
https://github.com/larskanis/shellwords/tree/master/lib/shellwords
As a compromise I used String#inspect for the lists of patches. This works well enough on all platforms.
This fixes #1190 and already includes #1308 , to make sure, that the native build is actively checked.